Modify ProtocolHandlerRegistry to make room for policy

This CL modifies ProtocolHandlerRegistry to enable addition of policy for registering and ignoring protocol handlers. The current implementation does not allow it since it saves the registered handlers, which would result in policy leak when directly mapped, i.e., protocol handler registered via policy would be saved to disk and mixed with user preference.

BUG=116119

TBR=koz,jochen

Review URL: https://codereview.chromium.org/293983004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274223 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/custom_handlers/protocol_handler_registry.h

@@ -36,6 +36,10 @@ class PrefRegistrySyncable;
 class ProtocolHandlerRegistry : public KeyedService {
 
  public:
+  enum HandlerSource {
+    USER,    // The handler was installed by user
+    POLICY,  // The handler was installed by policy
+  };
   // Provides notification of when the OS level user agent settings
   // are changed.
   class DefaultClientObserver
@@ -287,7 +291,12 @@ class ProtocolHandlerRegistry : public KeyedService {
   void NotifyChanged();
 
   // Registers a new protocol handler.
-  void RegisterProtocolHandler(const ProtocolHandler& handler);
+  void RegisterProtocolHandler(const ProtocolHandler& handler,
+                               const HandlerSource source);
+
+  // Registers protocol handlers from the preference.
+  void RegisterProtocolHandlersFromPref(const char* pref_name,
+                                        const HandlerSource source);
 
   // Get the DictionaryValues stored under the given pref name that are valid
   // ProtocolHandler values.
@@ -295,7 +304,27 @@ class ProtocolHandlerRegistry : public KeyedService {
       const char* pref_name) const;
 
   // Ignores future requests to register the given protocol handler.
-  void IgnoreProtocolHandler(const ProtocolHandler& handler);
+  void IgnoreProtocolHandler(const ProtocolHandler& handler,
+                             const HandlerSource source);
+
+  // Ignores protocol handlers from the preference.
+  void IgnoreProtocolHandlersFromPref(const char* pref_name,
+                                      const HandlerSource source);
+
+  // Verifies if the handler exists in the map.
+  bool HandlerExists(const ProtocolHandler& handler,
+                     ProtocolHandlerMultiMap* map);
+
+  // Verifies if the handler exists in the list.
+  bool HandlerExists(const ProtocolHandler& handler,
+                     const ProtocolHandlerList& list);
+
+  // Erases the handler that is guaranteed to exist from the map.
+  void EraseHandler(const ProtocolHandler& handler,
+                    ProtocolHandlerMultiMap* map);
+
+  // Erases the handler that is guaranteed to exist from the list.
+  void EraseHandler(const ProtocolHandler& handler, ProtocolHandlerList* list);
 
   // Map from protocols (strings) to protocol handlers.
   ProtocolHandlerMultiMap protocol_handlers_;
@@ -303,6 +332,20 @@ class ProtocolHandlerRegistry : public KeyedService {
   // Protocol handlers that the user has told us to ignore.
   ProtocolHandlerList ignored_protocol_handlers_;
 
+  // These maps track the source of protocol handler registrations for the
+  // purposes of disallowing the removal of handlers that are registered by
+  // policy. Every entry in protocol_handlers_ should exist in at least one of
+  // the user or policy maps.
+  ProtocolHandlerMultiMap user_protocol_handlers_;
+  ProtocolHandlerMultiMap policy_protocol_handlers_;
+
+  // These lists track the source of protocol handlers that were ignored, for
+  // the purposes of disallowing the removal of handlers that are ignored by
+  // policy. Every entry in ignored_protocol_handlers_ should exist in at least
+  // one of the user or policy lists.
+  ProtocolHandlerList user_ignored_protocol_handlers_;
+  ProtocolHandlerList policy_ignored_protocol_handlers_;
+
   // Protocol handlers that are the defaults for a given protocol.
   ProtocolHandlerMap default_handlers_;
 

chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/common/custom_handlers/protocol_handler.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -103,6 +104,22 @@ void AssertWillHandle(
   base::MessageLoop::current()->RunUntilIdle();
 }
 
+base::DictionaryValue* GetProtocolHandlerValue(std::string protocol,
+                                               std::string url) {
+  base::DictionaryValue* value = new base::DictionaryValue();
+  value->SetString("protocol", protocol);
+  value->SetString("url", url);
+  return value;
+}
+
+base::DictionaryValue* GetProtocolHandlerValueWithDefault(std::string protocol,
+                                                          std::string url,
+                                                          bool is_default) {
+  base::DictionaryValue* value = GetProtocolHandlerValue(protocol, url);
+  value->SetBoolean("default", is_default);
+  return value;
+}
+
 class FakeDelegate : public ProtocolHandlerRegistry::Delegate {
  public:
   FakeDelegate() : force_os_failure_(false) {}
@@ -334,6 +351,36 @@ class ProtocolHandlerRegistryTest : public testing::Test {
     SetUpRegistry(initialize);
   }
 
+  int InPrefHandlerCount() {
+    const base::ListValue* in_pref_handlers =
+        profile()->GetPrefs()->GetList(prefs::kRegisteredProtocolHandlers);
+    return static_cast<int>(in_pref_handlers->GetSize());
+  }
+
+  int InMemoryHandlerCount() {
+    int in_memory_handler_count = 0;
+    ProtocolHandlerRegistry::ProtocolHandlerMultiMap::iterator it =
+        registry()->protocol_handlers_.begin();
+    for (; it != registry()->protocol_handlers_.end(); ++it)
+      in_memory_handler_count += it->second.size();
+    return in_memory_handler_count;
+  }
+
+  int InPrefIgnoredHandlerCount() {
+    const base::ListValue* in_pref_ignored_handlers =
+        profile()->GetPrefs()->GetList(prefs::kIgnoredProtocolHandlers);
+    return static_cast<int>(in_pref_ignored_handlers->GetSize());
+  }
+
+  int InMemoryIgnoredHandlerCount() {
+    int in_memory_ignored_handler_count = 0;
+    ProtocolHandlerRegistry::ProtocolHandlerList::iterator it =
+        registry()->ignored_protocol_handlers_.begin();
+    for (; it != registry()->ignored_protocol_handlers_.end(); ++it)
+      in_memory_ignored_handler_count++;
+    return in_memory_ignored_handler_count;
+  }
+
   // Returns a new registry, initializing it if |initialize| is true.
   // Caller assumes ownership for the object
   void SetUpRegistry(bool initialize) {
@@ -916,3 +963,166 @@ TEST_F(ProtocolHandlerRegistryTest, MAYBE_TestInstallDefaultHandler) {
   registry()->GetRegisteredProtocols(&protocols);
   ASSERT_EQ(static_cast<size_t>(1), protocols.size());
 }
+
+#define URL_p1u1 "http://p1u1.com/%s"
+#define URL_p1u2 "http://p1u2.com/%s"
+#define URL_p1u3 "http://p1u3.com/%s"
+#define URL_p2u1 "http://p2u1.com/%s"
+#define URL_p2u2 "http://p2u2.com/%s"
+#define URL_p3u1 "http://p3u1.com/%s"
+
+TEST_F(ProtocolHandlerRegistryTest, TestPrefPolicyOverlapRegister) {
+  base::ListValue* handlers_registered_by_pref = new base::ListValue();
+  base::ListValue* handlers_registered_by_policy = new base::ListValue();
+
+  handlers_registered_by_pref->Append(
+      GetProtocolHandlerValueWithDefault("p1", URL_p1u2, true));
+  handlers_registered_by_pref->Append(
+      GetProtocolHandlerValueWithDefault("p1", URL_p1u1, true));
+  handlers_registered_by_pref->Append(
+      GetProtocolHandlerValueWithDefault("p1", URL_p1u2, false));
+
+  handlers_registered_by_policy->Append(
+      GetProtocolHandlerValueWithDefault("p1", URL_p1u1, false));
+  handlers_registered_by_policy->Append(
+      GetProtocolHandlerValueWithDefault("p3", URL_p3u1, true));
+
+  profile()->GetPrefs()->Set(
+      prefs::kRegisteredProtocolHandlers,
+      *static_cast<base::Value*>(handlers_registered_by_pref));
+  profile()->GetPrefs()->Set(
+      prefs::kPolicyRegisteredProtocolHandlers,
+      *static_cast<base::Value*>(handlers_registered_by_policy));
+  registry()->InitProtocolSettings();
+
+  // Duplicate p1u2 eliminated in memory but not yet saved in pref
+  ProtocolHandler p1u1 = CreateProtocolHandler("p1", GURL(URL_p1u1));
+  ProtocolHandler p1u2 = CreateProtocolHandler("p1", GURL(URL_p1u2));
+  ASSERT_EQ(InPrefHandlerCount(), 3);
+  ASSERT_EQ(InMemoryHandlerCount(), 3);
+  ASSERT_TRUE(registry()->IsDefault(p1u1));
+  ASSERT_FALSE(registry()->IsDefault(p1u2));
+
+  ProtocolHandler p2u1 = CreateProtocolHandler("p2", GURL(URL_p2u1));
+  registry()->OnDenyRegisterProtocolHandler(p2u1);
+
+  // Duplicate p1u2 saved in pref and a new handler added to pref and memory
+  ASSERT_EQ(InPrefHandlerCount(), 3);
+  ASSERT_EQ(InMemoryHandlerCount(), 4);
+  ASSERT_FALSE(registry()->IsDefault(p2u1));
+
+  registry()->RemoveHandler(p1u1);
+
+  // p1u1 removed from user pref but not from memory due to policy.
+  ASSERT_EQ(InPrefHandlerCount(), 2);
+  ASSERT_EQ(InMemoryHandlerCount(), 4);
+  ASSERT_TRUE(registry()->IsDefault(p1u1));
+
+  ProtocolHandler p3u1 = CreateProtocolHandler("p3", GURL(URL_p3u1));
+  registry()->RemoveHandler(p3u1);
+
+  // p3u1 not removed from memory due to policy and it was never in pref.
+  ASSERT_EQ(InPrefHandlerCount(), 2);
+  ASSERT_EQ(InMemoryHandlerCount(), 4);
+  ASSERT_TRUE(registry()->IsDefault(p3u1));
+
+  registry()->RemoveHandler(p1u2);
+
+  // p1u2 removed from user pref and memory.
+  ASSERT_EQ(InPrefHandlerCount(), 1);
+  ASSERT_EQ(InMemoryHandlerCount(), 3);
+  ASSERT_TRUE(registry()->IsDefault(p1u1));
+
+  ProtocolHandler p1u3 = CreateProtocolHandler("p1", GURL(URL_p1u3));
+  registry()->OnAcceptRegisterProtocolHandler(p1u3);
+
+  // p1u3 added to pref and memory.
+  ASSERT_EQ(InPrefHandlerCount(), 2);
+  ASSERT_EQ(InMemoryHandlerCount(), 4);
+  ASSERT_FALSE(registry()->IsDefault(p1u1));
+  ASSERT_TRUE(registry()->IsDefault(p1u3));
+
+  registry()->RemoveHandler(p1u3);
+
+  // p1u3 the default handler for p1 removed from user pref and memory.
+  ASSERT_EQ(InPrefHandlerCount(), 1);
+  ASSERT_EQ(InMemoryHandlerCount(), 3);
+  ASSERT_FALSE(registry()->IsDefault(p1u3));
+  ASSERT_TRUE(registry()->IsDefault(p1u1));
+  ASSERT_TRUE(registry()->IsDefault(p3u1));
+  ASSERT_FALSE(registry()->IsDefault(p2u1));
+}
+
+TEST_F(ProtocolHandlerRegistryTest, TestPrefPolicyOverlapIgnore) {
+  base::ListValue* handlers_ignored_by_pref = new base::ListValue();
+  base::ListValue* handlers_ignored_by_policy = new base::ListValue();
+
+  handlers_ignored_by_pref->Append(GetProtocolHandlerValue("p1", URL_p1u1));
+  handlers_ignored_by_pref->Append(GetProtocolHandlerValue("p1", URL_p1u2));
+  handlers_ignored_by_pref->Append(GetProtocolHandlerValue("p1", URL_p1u2));
+  handlers_ignored_by_pref->Append(GetProtocolHandlerValue("p3", URL_p3u1));
+
+  handlers_ignored_by_policy->Append(GetProtocolHandlerValue("p1", URL_p1u2));
+  handlers_ignored_by_policy->Append(GetProtocolHandlerValue("p1", URL_p1u3));
+  handlers_ignored_by_policy->Append(GetProtocolHandlerValue("p2", URL_p2u1));
+
+  profile()->GetPrefs()->Set(
+      prefs::kIgnoredProtocolHandlers,
+      *static_cast<base::Value*>(handlers_ignored_by_pref));
+  profile()->GetPrefs()->Set(
+      prefs::kPolicyIgnoredProtocolHandlers,
+      *static_cast<base::Value*>(handlers_ignored_by_policy));
+  registry()->InitProtocolSettings();
+
+  // Duplicate p1u2 eliminated in memory but not yet saved in pref
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 4);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 5);
+
+  ProtocolHandler p2u2 = CreateProtocolHandler("p2", GURL(URL_p2u2));
+  registry()->OnIgnoreRegisterProtocolHandler(p2u2);
+
+  // Duplicate p1u2 eliminated in pref, p2u2 added to pref and memory.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 4);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 6);
+
+  ProtocolHandler p2u1 = CreateProtocolHandler("p2", GURL(URL_p2u1));
+  registry()->RemoveIgnoredHandler(p2u1);
+
+  // p2u1 installed by policy so cant be removed.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 4);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 6);
+
+  ProtocolHandler p1u2 = CreateProtocolHandler("p1", GURL(URL_p1u2));
+  registry()->RemoveIgnoredHandler(p1u2);
+
+  // p1u2 installed by policy and pref so it is removed from pref and not from
+  // memory.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 3);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 6);
+
+  ProtocolHandler p1u1 = CreateProtocolHandler("p1", GURL(URL_p1u1));
+  registry()->RemoveIgnoredHandler(p1u1);
+
+  // p1u1 installed by pref so it is removed from pref and memory.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 2);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 5);
+
+  registry()->RemoveIgnoredHandler(p2u2);
+
+  // p2u2 installed by user so it is removed from pref and memory.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 1);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 4);
+
+  registry()->OnIgnoreRegisterProtocolHandler(p2u1);
+
+  // p2u1 installed by user but it is already installed by policy, so it is
+  // added to pref.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 2);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 4);
+
+  registry()->RemoveIgnoredHandler(p2u1);
+
+  // p2u1 installed by user and policy, so it is removed from pref alone.
+  ASSERT_EQ(InPrefIgnoredHandlerCount(), 1);
+  ASSERT_EQ(InMemoryIgnoredHandlerCount(), 4);
+}

chrome/common/pref_names.cc

@@ -2256,6 +2256,14 @@ const char kRegisteredProtocolHandlers[] =
 const char kIgnoredProtocolHandlers[] =
   "custom_handlers.ignored_protocol_handlers";
 
+// List of protocol handlers registered by policy.
+const char kPolicyRegisteredProtocolHandlers[] =
+    "custom_handlers.policy.registered_protocol_handlers";
+
+// List of protocol handlers the policy has requested to be ignored.
+const char kPolicyIgnoredProtocolHandlers[] =
+    "custom_handlers.policy.ignored_protocol_handlers";
+
 // Whether user-specified handlers for protocols and content types can be
 // specified.
 const char kCustomHandlersEnabled[] = "custom_handlers.enabled";

chrome/common/pref_names.h

@@ -746,6 +746,8 @@ extern const char kBuiltInDnsClientEnabled[];
 
 extern const char kRegisteredProtocolHandlers[];
 extern const char kIgnoredProtocolHandlers[];
+extern const char kPolicyRegisteredProtocolHandlers[];
+extern const char kPolicyIgnoredProtocolHandlers[];
 extern const char kCustomHandlersEnabled[];
 
 #if defined(OS_MACOSX)