MD Settings: Always provide recommended meta-data
Currently we only provide recommended meta-data for prefs
when pref->IsRecommended() is true, which is only true when
the user has not set the value.
We should provide meta-data whenever a policy provides a
recommended value for a pref.
BUG=532958
Review-Url: https://codereview.chromium.org/2861393003
Cr-Commit-Position: refs/heads/master@{#472513}
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc
index f7f8a6a..99a57d58 100644
--- a/chrome/browser/extensions/api/settings_private/prefs_util.cc
+++ b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -483,13 +483,16 @@
return pref_object;
}
- if (pref && pref->IsRecommended()) {
+ // A pref is recommended if it has a recommended value, regardless of whether
+ // the current value is set by policy. The UI will test to see whether the
+ // current value matches the recommended value and inform the user.
+ const base::Value* recommended = pref ? pref->GetRecommendedValue() : nullptr;
+ if (recommended) {
pref_object->controlled_by =
settings_private::ControlledBy::CONTROLLED_BY_USER_POLICY;
pref_object->enforcement =
settings_private::Enforcement::ENFORCEMENT_RECOMMENDED;
- pref_object->recommended_value.reset(
- pref->GetRecommendedValue()->DeepCopy());
+ pref_object->recommended_value.reset(recommended->DeepCopy());
return pref_object;
}
diff --git a/chrome/browser/extensions/api/settings_private/settings_private_apitest.cc b/chrome/browser/extensions/api/settings_private/settings_private_apitest.cc
index 3e6d167..64a62c0 100644
--- a/chrome/browser/extensions/api/settings_private/settings_private_apitest.cc
+++ b/chrome/browser/extensions/api/settings_private/settings_private_apitest.cc
@@ -12,15 +12,26 @@
#include "chrome/browser/extensions/api/settings_private/settings_private_delegate_factory.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/common/extensions/api/settings_private.h"
+#include "chrome/common/pref_names.h"
#include "components/keyed_service/core/keyed_service.h"
+#include "components/policy/core/browser/browser_policy_connector.h"
+#include "components/policy/core/common/mock_configuration_policy_provider.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/test_utils.h"
#include "extensions/common/switches.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_CHROMEOS)
#include "chromeos/chromeos_switches.h"
#endif
+using testing::Mock;
+using testing::Return;
+using testing::_;
+
namespace extensions {
namespace {
@@ -37,14 +48,33 @@
#endif
}
+ void SetUpInProcessBrowserTestFixture() override {
+ EXPECT_CALL(provider_, IsInitializationComplete(_))
+ .WillRepeatedly(Return(true));
+ policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
+ ExtensionApiTest::SetUpInProcessBrowserTestFixture();
+ }
+
protected:
bool RunSettingsSubtest(const std::string& subtest) {
- return RunExtensionSubtest("settings_private",
- "main.html?" + subtest,
+ return RunExtensionSubtest("settings_private", "main.html?" + subtest,
kFlagLoadAsComponent);
}
+ void SetPrefPolicy(const std::string& key, policy::PolicyLevel level) {
+ policy::PolicyMap policies;
+ policies.Set(key, level, policy::POLICY_SCOPE_USER,
+ policy::POLICY_SOURCE_CLOUD,
+ base::WrapUnique(new base::Value(true)), nullptr);
+ provider_.UpdateChromePolicy(policies);
+ DCHECK(base::MessageLoop::current());
+ base::RunLoop loop;
+ loop.RunUntilIdle();
+ }
+
private:
+ policy::MockConfigurationPolicyProvider provider_;
+
DISALLOW_COPY_AND_ASSIGN(SettingsPrivateApiTest);
};
@@ -59,6 +89,18 @@
EXPECT_TRUE(RunSettingsSubtest("getPref")) << message_;
}
+IN_PROC_BROWSER_TEST_F(SettingsPrivateApiTest, GetEnforcedPref) {
+ SetPrefPolicy(policy::key::kHomepageIsNewTabPage,
+ policy::POLICY_LEVEL_MANDATORY);
+ EXPECT_TRUE(RunSettingsSubtest("getEnforcedPref")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(SettingsPrivateApiTest, GetRecommendedPref) {
+ SetPrefPolicy(policy::key::kHomepageIsNewTabPage,
+ policy::POLICY_LEVEL_RECOMMENDED);
+ EXPECT_TRUE(RunSettingsSubtest("getRecommendedPref")) << message_;
+}
+
IN_PROC_BROWSER_TEST_F(SettingsPrivateApiTest, GetAllPrefs) {
EXPECT_TRUE(RunSettingsSubtest("getAllPrefs")) << message_;
}
diff --git a/chrome/test/data/extensions/api_test/settings_private/test.js b/chrome/test/data/extensions/api_test/settings_private/test.js
index e611668..580971f 100644
--- a/chrome/test/data/extensions/api_test/settings_private/test.js
+++ b/chrome/test/data/extensions/api_test/settings_private/test.js
@@ -8,6 +8,11 @@
var kTestPrefName = 'download.default_directory';
var kTestPrefValue = '/Downloads';
+
+// This corresponds to policy key: kHomepageIsNewTabPage used in
+// settings_private_apitest.cc.
+var kTestEnforcedPrefName = 'homepage_is_newtabpage';
+
var kTestPageId = 'pageId';
function callbackResult(result) {
@@ -47,6 +52,46 @@
chrome.test.succeed();
});
},
+ function getEnforcedPref() {
+ chrome.settingsPrivate.getPref(kTestEnforcedPrefName, function(value) {
+ chrome.test.assertEq('object', typeof value);
+ callbackResult(true);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.ControlledBy.USER_POLICY, value.controlledBy);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.Enforcement.ENFORCED, value.enforcement);
+ chrome.test.succeed();
+ });
+ },
+ function getRecommendedPref() {
+ chrome.settingsPrivate.getPref(kTestEnforcedPrefName, function(value) {
+ chrome.test.assertEq('object', typeof value);
+ callbackResult(true);
+ chrome.test.assertEq(true, value.value);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.ControlledBy.USER_POLICY, value.controlledBy);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.Enforcement.RECOMMENDED, value.enforcement);
+ // Set the value to false, policy properties should still be set.
+ chrome.settingsPrivate.setPref(
+ kTestEnforcedPrefName, false, kTestPageId, function(success) {
+ callbackResult(success);
+ chrome.settingsPrivate.getPref(
+ kTestEnforcedPrefName, function(value) {
+ chrome.test.assertEq('object', typeof value);
+ callbackResult(true);
+ chrome.test.assertEq(false, value.value);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.ControlledBy.USER_POLICY,
+ value.controlledBy);
+ chrome.test.assertEq(
+ chrome.settingsPrivate.Enforcement.RECOMMENDED,
+ value.enforcement);
+ chrome.test.succeed();
+ });
+ });
+ });
+ },
function getPref_CrOSSetting() {
chrome.settingsPrivate.getPref(
'cros.accounts.allowBWSI',
@@ -100,4 +145,3 @@
chrome.test.runTests(availableTests.filter(function(op) {
return op.name == testToRun;
}));
-
