Convert ProtocolHandlerRegistry to be a ProfileKeyedService.

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 0300e6d7fa894d6c0af8c6466a690f7a6e6c083d..82bcad2ca5083e8169a92721af238f16c32b7791 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry.cc
@@ -43,6 +43,22 @@ bool ShouldRemoveHandlersNotInOS() {
 #endif
 }
 
+void InstallDefaultProtocolHandlers(ProtocolHandlerRegistry* registry) {
+  // only chromeos has default protocol handlers at this point.
+  #if defined(OS_CHROMEOS)
+    registry->AddPredefinedHandler(
+        ProtocolHandler::CreateProtocolHandler(
+            "mailto",
+            GURL(l10n_util::GetStringUTF8(IDS_GOOGLE_MAILTO_HANDLER_URL)),
+            l10n_util::GetStringUTF16(IDS_GOOGLE_MAILTO_HANDLER_NAME)));
+    registry->AddPredefinedHandler(
+        ProtocolHandler::CreateProtocolHandler(
+            "webcal",
+            GURL(l10n_util::GetStringUTF8(IDS_GOOGLE_WEBCAL_HANDLER_URL)),
+            l10n_util::GetStringUTF16(IDS_GOOGLE_WEBCAL_HANDLER_NAME)));
+  #endif
+}
+
 } // namespace
 
 static const ProtocolHandler& LookupHandler(
@@ -145,7 +161,8 @@ void ProtocolHandlerRegistry::Delegate::RegisterWithOSAsDefaultClient(
 
 // ProtocolHandlerRegistry -----------------------------------------------------
 
-ProtocolHandlerRegistry::ProtocolHandlerRegistry(Profile* profile,
+ProtocolHandlerRegistry::ProtocolHandlerRegistry(
+    Profile* profile,
     Delegate* delegate)
     : profile_(profile),
       delegate_(delegate),
@@ -232,11 +249,14 @@ ProtocolHandlerRegistry::GetReplacedHandlers(
 
 void ProtocolHandlerRegistry::ClearDefault(const std::string& scheme) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   default_handlers_.erase(scheme);
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&ProtocolHandlerRegistry::ClearDefaultIO, this, scheme));
+      base::Bind(&ProtocolHandlerRegistry::ClearDefaultIO,
+                 base::Unretained(this),
+                 scheme));
   Save();
   NotifyChanged();
 }
@@ -247,6 +267,15 @@ bool ProtocolHandlerRegistry::IsDefault(
   return GetHandlerFor(handler.protocol()) == handler;
 }
 
+void ProtocolHandlerRegistry::InitProtocolSettings() {
+
+  // Install predefined protocol handlers.
+  InstallDefaultProtocolHandlers(this);
+  Load();
+}
+
+// TODO(smckay): Can this be made anonymous and accessed
+// via InitProtocolSettings?
 void ProtocolHandlerRegistry::Load() {
   // Any further default additions to the table will get rejected from now on.
   is_loaded_ = true;
@@ -258,8 +287,10 @@ void ProtocolHandlerRegistry::Load() {
     BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
-        base::Bind(enabled_ ? &ProtocolHandlerRegistry::EnableIO :
-                   &ProtocolHandlerRegistry::DisableIO, this));
+        base::Bind(enabled_ ?
+                       &ProtocolHandlerRegistry::EnableIO :
+                       &ProtocolHandlerRegistry::DisableIO,
+                   base::Unretained(this)));
   }
   std::vector<const DictionaryValue*> registered_handlers =
       GetHandlersFromPref(prefs::kRegisteredProtocolHandlers);
@@ -451,7 +482,8 @@ void ProtocolHandlerRegistry::RemoveHandler(
     } else {
       BrowserThread::PostTask(
           BrowserThread::IO, FROM_HERE,
-          base::Bind(&ProtocolHandlerRegistry::ClearDefaultIO, this,
+          base::Bind(&ProtocolHandlerRegistry::ClearDefaultIO,
+                     base::Unretained(this),
                      q->second.protocol()));
       default_handlers_.erase(q);
     }
@@ -501,7 +533,7 @@ void ProtocolHandlerRegistry::Enable() {
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&ProtocolHandlerRegistry::EnableIO, this));
+      base::Bind(&ProtocolHandlerRegistry::EnableIO, base::Unretained(this)));
   ProtocolHandlerMap::const_iterator p;
   for (p = default_handlers_.begin(); p != default_handlers_.end(); ++p) {
     delegate_->RegisterExternalHandler(p->first);
@@ -519,7 +551,8 @@ void ProtocolHandlerRegistry::Disable() {
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&ProtocolHandlerRegistry::DisableIO, this));
+      base::Bind(&ProtocolHandlerRegistry::DisableIO,
+                 base::Unretained(this)));
   ProtocolHandlerMap::const_iterator p;
   for (p = default_handlers_.begin(); p != default_handlers_.end(); ++p) {
     delegate_->DeregisterExternalHandler(p->first);
@@ -552,7 +585,8 @@ void ProtocolHandlerRegistry::RegisterPrefs(PrefService* pref_service) {
 }
 
 ProtocolHandlerRegistry::~ProtocolHandlerRegistry() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  Finalize();

[Elliot Glaysher, 2012/06/11 19:43:16]

(And remove this call)

[Steve McKay, 2012/06/11 22:09:38]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[willchan no longer on Chromium, 2012/06/11 21:17:22]

I think this class is used on both the UI and IO thread. Are you doing anything
to make sure it's not concurrently being used on the IO thread when this is
being deleted? If not, it'll probably lead to race conditions.

[Steve McKay, 2012/06/11 22:09:38]

So what I need to tease apart here, for my understanding and because
WebIntentsRegistry uses a ProfileKeyedService as well, is the distinction
between the overall design of the class and it's consumers and the changes I'm
making (switching to a PKS). I'm not doing anything new in this CL to
orchestrate thread access to this class. If there's a race condition, can you
help me understand if it is intrinsic to the design or if emerges from the
refactoring in this CL.

In either case if there is a race condition I would appreciate your advice on
how I can remedy this (again, I'm new to Chrome and even C++ and I might require
a little more hand holding than a seasoned Chrome engineer :)

Thanks for your help.

[willchan no longer on Chromium, 2012/06/12 02:09:56]

The full answer is complicated :) But in short, this object is being used on two
threads, and may be getting deleted on the UI thread while it's being used on
the IO thread, which would be problematic. With an object that is accessed on
multiple threads, you need to do some sort of coordination across threads to
destroy the object without race conditions.

I don't have time right now to explain further...and in my own selfish interest
of saving my own time, can I ask you to ask your teammates about it first? Or
Elliot (erg) or someone. And they can punt to me if they want.

   DCHECK(default_client_observers_.empty());
 }
 
@@ -617,7 +651,9 @@ void ProtocolHandlerRegistry::SetDefault(const ProtocolHandler& handler) {
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&ProtocolHandlerRegistry::SetDefaultIO, this, handler));
+      base::Bind(&ProtocolHandlerRegistry::SetDefaultIO,
+                 base::Unretained(this),
+                 handler));
 }
 
 void ProtocolHandlerRegistry::InsertHandler(const ProtocolHandler& handler) {
@@ -719,5 +755,3 @@ void ProtocolHandlerRegistry::AddPredefinedHandler(
   RegisterProtocolHandler(handler);
   SetDefault(handler);
 }
-
-