Modify ProtocolHandlerRegistry to make room for policy

chrome/browser/custom_handlers/protocol_handler_registry.cc

Index: chrome/browser/custom_handlers/protocol_handler_registry.cc
diff --git a/chrome/browser/custom_handlers/protocol_handler_registry.cc b/chrome/browser/custom_handlers/protocol_handler_registry.cc
index 7043681af2e1a037b6673c5cce3f46fc22425c4c..b8491f80e6448ca8af2d34efd7b88badec84b722 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry.cc
@@ -336,7 +336,7 @@ bool ProtocolHandlerRegistry::SilentlyHandleRegisterHandlerRequest(
 void ProtocolHandlerRegistry::OnAcceptRegisterProtocolHandler(
     const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  RegisterProtocolHandler(handler);
+  RegisterProtocolHandler(handler, false);
   SetDefault(handler);
   Save();
   NotifyChanged();
@@ -345,7 +345,7 @@ void ProtocolHandlerRegistry::OnAcceptRegisterProtocolHandler(
 void ProtocolHandlerRegistry::OnDenyRegisterProtocolHandler(
     const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  RegisterProtocolHandler(handler);
+  RegisterProtocolHandler(handler, false);
   Save();
   NotifyChanged();
 }
@@ -353,7 +353,7 @@ void ProtocolHandlerRegistry::OnDenyRegisterProtocolHandler(
 void ProtocolHandlerRegistry::OnIgnoreRegisterProtocolHandler(
     const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  IgnoreProtocolHandler(handler);
+  IgnoreProtocolHandler(handler, false);
   Save();
   NotifyChanged();
 }
@@ -444,23 +444,43 @@ void ProtocolHandlerRegistry::InitProtocolSettings() {
     }
   }
   std::vector<const base::DictionaryValue*> registered_handlers =

[koz (OOO until 15th September), 2014/05/22 00:51:31]

It might be worth introducing some helper functions here, e.g.

RegisterHandlersFromPref(prefs::kPolicyRegisteredProtocolHandlers, true);
IgnoreHandlersFromPref(prefs::kPolicyIgnoredProtocolHandlers, true);

RegisterHandlersFromPref(prefs::kRegisteredProtocolHandlers, false);
IgnoreHandlersFromPref(prefs::kIgnoredProtocolHandlers, false);

[kaliamoorthi, 2014/05/30 10:01:09]

Done.

-      GetHandlersFromPref(prefs::kRegisteredProtocolHandlers);
+      GetHandlersFromPref(prefs::kPolicyRegisteredProtocolHandlers);
   for (std::vector<const base::DictionaryValue*>::const_iterator p =
        registered_handlers.begin();
        p != registered_handlers.end(); ++p) {
     ProtocolHandler handler = ProtocolHandler::CreateProtocolHandler(*p);
-    RegisterProtocolHandler(handler);
+    RegisterProtocolHandler(handler, true);
     bool is_default = false;
     if ((*p)->GetBoolean("default", &is_default) && is_default) {
       SetDefault(handler);
     }
   }
   std::vector<const base::DictionaryValue*> ignored_handlers =
-    GetHandlersFromPref(prefs::kIgnoredProtocolHandlers);
+      GetHandlersFromPref(prefs::kPolicyIgnoredProtocolHandlers);
   for (std::vector<const base::DictionaryValue*>::const_iterator p =
        ignored_handlers.begin();
        p != ignored_handlers.end(); ++p) {
-    IgnoreProtocolHandler(ProtocolHandler::CreateProtocolHandler(*p));
+    IgnoreProtocolHandler(ProtocolHandler::CreateProtocolHandler(*p), true);
+  }
+
+  registered_handlers = GetHandlersFromPref(prefs::kRegisteredProtocolHandlers);
+  for (std::vector<const base::DictionaryValue*>::const_iterator p =
+           registered_handlers.begin();
+       p != registered_handlers.end();
+       ++p) {
+    ProtocolHandler handler = ProtocolHandler::CreateProtocolHandler(*p);
+    RegisterProtocolHandler(handler, false);
+    bool is_default = false;
+    if ((*p)->GetBoolean("default", &is_default) && is_default) {
+      SetDefault(handler);
+    }
+  }
+  ignored_handlers = GetHandlersFromPref(prefs::kIgnoredProtocolHandlers);
+  for (std::vector<const base::DictionaryValue*>::const_iterator p =
+           ignored_handlers.begin();
+       p != ignored_handlers.end();
+       ++p) {
+    IgnoreProtocolHandler(ProtocolHandler::CreateProtocolHandler(*p), false);
   }
   is_loading_ = false;
 
@@ -591,13 +611,15 @@ void ProtocolHandlerRegistry::RemoveIgnoredHandler(
     const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   bool should_notify = false;
-  ProtocolHandlerList::iterator p = std::find(
-      ignored_protocol_handlers_.begin(), ignored_protocol_handlers_.end(),
-      handler);
-  if (p != ignored_protocol_handlers_.end()) {
-    ignored_protocol_handlers_.erase(p);
-    Save();
-    should_notify = true;
+  if (HandlerExists(handler, ignored_protocol_handlers_)) {
+    if (HandlerExists(handler, user_ignored_protocol_handlers_)) {
+      EraseHandler(handler, &user_ignored_protocol_handlers_);
+      Save();

[koz (OOO until 15th September), 2014/05/22 00:51:31]

Save() should be called after removing handler from ignored_protocol_handlers_
below.

[kaliamoorthi, 2014/05/30 10:01:09]

Please note that only user_ignored_protocol_handlers are saved.
ignored_protocol_handlers which is the in memory merged representation is not
saved, since it includes handlers from policy.

[koz (OOO until 15th September), 2014/06/01 23:44:28]

Ah, my bad. In that case feel free to revert to how you had the code before.

+      if (!HandlerExists(handler, policy_ignored_protocol_handlers_)) {
+        EraseHandler(handler, &ignored_protocol_handlers_);
+        should_notify = true;
+      }
+    }
   }
   if (should_notify)
     NotifyChanged();
@@ -613,13 +635,18 @@ void ProtocolHandlerRegistry::RemoveHandler(
     const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ProtocolHandlerList& handlers = protocol_handlers_[handler.protocol()];
-  ProtocolHandlerList::iterator p =
-      std::find(handlers.begin(), handlers.end(), handler);
-  if (p != handlers.end()) {
-    handlers.erase(p);
+  bool erase_success = false;
+  if (HandlerExists(handler, handlers)) {
+    if (HandlerExists(handler, &user_protocol_handlers_)) {
+      EraseHandler(handler, &user_protocol_handlers_);
+      if (!HandlerExists(handler, &policy_protocol_handlers_)) {
+        erase_success = true;
+        EraseHandler(handler, &protocol_handlers_);
+      }
+    }
   }
   ProtocolHandlerMap::iterator q = default_handlers_.find(handler.protocol());
-  if (q != default_handlers_.end() && q->second == handler) {
+  if (erase_success && q != default_handlers_.end() && q->second == handler) {
     // Make the new top handler in the list the default.
     if (!handlers.empty()) {
       // NOTE We pass a copy because SetDefault() modifies handlers.
@@ -634,11 +661,12 @@ void ProtocolHandlerRegistry::RemoveHandler(
     }
   }
 
-  if (!IsHandledProtocol(handler.protocol())) {
+  if (erase_success && !IsHandledProtocol(handler.protocol())) {
     delegate_->DeregisterExternalHandler(handler.protocol());
   }
   Save();
-  NotifyChanged();
+  if (erase_success)
+    NotifyChanged();
 }
 
 void ProtocolHandlerRegistry::RemoveDefaultHandler(const std::string& scheme) {
@@ -795,8 +823,9 @@ void ProtocolHandlerRegistry::InsertHandler(const ProtocolHandler& handler) {
 base::Value* ProtocolHandlerRegistry::EncodeRegisteredHandlers() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::ListValue* protocol_handlers = new base::ListValue();
-  for (ProtocolHandlerMultiMap::iterator i = protocol_handlers_.begin();
-       i != protocol_handlers_.end(); ++i) {
+  for (ProtocolHandlerMultiMap::iterator i = user_protocol_handlers_.begin();
+       i != user_protocol_handlers_.end();
+       ++i) {
     for (ProtocolHandlerList::iterator j = i->second.begin();
          j != i->second.end(); ++j) {
       base::DictionaryValue* encoded = j->Encode();
@@ -812,8 +841,10 @@ base::Value* ProtocolHandlerRegistry::EncodeRegisteredHandlers() {
 base::Value* ProtocolHandlerRegistry::EncodeIgnoredHandlers() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::ListValue* handlers = new base::ListValue();
-  for (ProtocolHandlerList::iterator i = ignored_protocol_handlers_.begin();
-       i != ignored_protocol_handlers_.end(); ++i) {
+  for (ProtocolHandlerList::iterator i =
+           user_ignored_protocol_handlers_.begin();
+       i != user_ignored_protocol_handlers_.end();
+       ++i) {
     handlers->Append(i->Encode());
   }
   return handlers;
@@ -828,10 +859,16 @@ void ProtocolHandlerRegistry::NotifyChanged() {
 }
 
 void ProtocolHandlerRegistry::RegisterProtocolHandler(
-    const ProtocolHandler& handler) {
+    const ProtocolHandler& handler,
+    const bool policy_managed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(CanSchemeBeOverridden(handler.protocol()));
   DCHECK(!handler.IsEmpty());
+  ProtocolHandlerMultiMap& map =
+      policy_managed ? policy_protocol_handlers_ : user_protocol_handlers_;
+  ProtocolHandlerList& list = map[handler.protocol()];
+  if (!HandlerExists(handler, list))
+    list.push_back(handler);
   if (IsRegistered(handler)) {
     return;
   }
@@ -864,15 +901,42 @@ ProtocolHandlerRegistry::GetHandlersFromPref(const char* pref_name) const {
 }
 
 void ProtocolHandlerRegistry::IgnoreProtocolHandler(
-    const ProtocolHandler& handler) {
+    const ProtocolHandler& handler,
+    const bool policy_managed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ignored_protocol_handlers_.push_back(handler);
+  ProtocolHandlerList& list = policy_managed ? policy_ignored_protocol_handlers_
+                                             : user_ignored_protocol_handlers_;
+  if (!HandlerExists(handler, list))
+    list.push_back(handler);
+}
+
+bool ProtocolHandlerRegistry::HandlerExists(const ProtocolHandler& handler,
+                                            ProtocolHandlerMultiMap* map) {
+  return HandlerExists(handler, (*map)[handler.protocol()]);
+}
+
+bool ProtocolHandlerRegistry::HandlerExists(const ProtocolHandler& handler,
+                                            const ProtocolHandlerList& list) {
+  return (std::find(list.begin(), list.end(), handler) != list.end());

[koz (OOO until 15th September), 2014/05/22 00:51:31]

nit: no need for parens

[kaliamoorthi, 2014/05/30 10:01:09]

Done.

+}
+
+void ProtocolHandlerRegistry::EraseHandler(const ProtocolHandler& handler,
+                                           ProtocolHandlerMultiMap* map) {
+  EraseHandler(handler, &(*map)[handler.protocol()]);
+}
+
+void ProtocolHandlerRegistry::EraseHandler(const ProtocolHandler& handler,
+                                           ProtocolHandlerList* list) {
+  ProtocolHandlerList::iterator p =
+      std::find(list->begin(), list->end(), handler);
+  list->erase(p);
 }
 
 void ProtocolHandlerRegistry::AddPredefinedHandler(
     const ProtocolHandler& handler) {
   DCHECK(!is_loaded_);  // Must be called prior InitProtocolSettings.
-  RegisterProtocolHandler(handler);
+  RegisterProtocolHandler(handler, false);
   SetDefault(handler);
 }