Policy: Ignore ForceSafeSearch if ForceGoogleSafeSearch or ForceYoutubeSafetyMode are enabled

chrome/browser/policy/configuration_policy_handler_list_factory.cc

Index: chrome/browser/policy/configuration_policy_handler_list_factory.cc
diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
index 0a26524ee762d201d67724d48bc8a76a9da42069..48dded2d367889996c47541de7fe6f878a46c29d 100644
--- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc
+++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/memory/scoped_vector.h"
+#include "base/prefs/pref_value_map.h"
 #include "base/values.h"
 #include "chrome/browser/net/proxy_policy_handler.h"
 #include "chrome/browser/policy/managed_bookmarks_policy_handler.h"
@@ -99,9 +100,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
   { key::kSafeBrowsingEnabled,
     prefs::kSafeBrowsingEnabled,
     base::Value::TYPE_BOOLEAN },
-  { key::kForceSafeSearch,
-    prefs::kForceSafeSearch,
-    base::Value::TYPE_BOOLEAN },
   { key::kForceGoogleSafeSearch,
     prefs::kForceGoogleSafeSearch,
     base::Value::TYPE_BOOLEAN },
@@ -503,6 +501,33 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
 #endif  // !defined(OS_CHROMEOS) && !defined(OS_ANDROID) && !defined(OS_IOS)
 };
 
+class ForceSafeSearchPolicyHandler : public TypeCheckingPolicyHandler {

[Mattias Nissler (ping if slow), 2015/04/16 13:10:47]

What you have works. However, I think an easier-to-follow solution would be to
drop the simple mappings declared above and move the responsibility for mapping
prefs::kFocreGoogleSafeSearch and prefs::kForceYouToubeSafetyMode into a single
place, namely this handler. That way, the reader has all the logic deciding the
pref values in one place and doesn't need to be aware of the simple mapping
declared in the table above.

[Marc Treib, 2015/04/16 13:18:23]

I considered that, and I actually prefer the way it is now: If you're only
interested in the non-legacy policies, you see the simple mapping above and know
all you need to know. Only the weirdly-behaving legacy policy has special code.
And if we can ever actually remove the legacy policy, it's as simple as removing
this class.
I've added a comment that refers to kSimplePolicyMap though :)

[Mattias Nissler (ping if slow), 2015/04/16 13:27:56]

I suppose somebody trying to understand how these policies work would prefer to
have the full story told in a single place. Your comment achieves that same goal
though, so fine with me.

+ public:
+  ForceSafeSearchPolicyHandler()
+      : TypeCheckingPolicyHandler(key::kForceSafeSearch,
+                                  base::Value::TYPE_BOOLEAN) {}
+  ~ForceSafeSearchPolicyHandler() override {}
+
+  // ConfigurationPolicyHandler implementation:
+  void ApplyPolicySettings(const PolicyMap& policies,
+                           PrefValueMap* prefs) override {
+    // If either of the new GoogleSafeSearch or YouTubeSafetyMode policies is
+    // defined, then this one should be ignored. crbug.com/476908
+    if (policies.GetValue(key::kForceGoogleSafeSearch) ||
+        policies.GetValue(key::kForceYouTubeSafetyMode)) {
+      return;
+    }
+    const base::Value* value = policies.GetValue(policy_name());
+    if (value) {
+      prefs->SetValue(prefs::kForceGoogleSafeSearch, value->DeepCopy());
+      prefs->SetValue(prefs::kForceYouTubeSafetyMode, value->DeepCopy());
+    }
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ForceSafeSearchPolicyHandler);
+};
+
 #if defined(ENABLE_EXTENSIONS)
 void GetExtensionAllowedTypesMap(
     ScopedVector<StringMappingListPolicyHandler::MappingEntry>* result) {
@@ -555,6 +580,7 @@ scoped_ptr<ConfigurationPolicyHandlerList> BuildHandlerList(
 
   handlers->AddHandler(make_scoped_ptr(new AutofillPolicyHandler()));
   handlers->AddHandler(make_scoped_ptr(new DefaultSearchPolicyHandler()));
+  handlers->AddHandler(make_scoped_ptr(new ForceSafeSearchPolicyHandler()));
   handlers->AddHandler(make_scoped_ptr(new IncognitoModePolicyHandler()));
   handlers->AddHandler(
       make_scoped_ptr(new ManagedBookmarksPolicyHandler(chrome_schema)));