Convert ProtocolHandlerRegistry to be a ProfileKeyedService.

chrome/browser/custom_handlers/protocol_handler_registry.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 25 matching lines @@
   // We don't do this on Linux as the OS registration there is not reliable,
   // and Chrome OS doesn't have any notion of OS registration.
   // TODO(benwells): When Linux support is more reliable remove this
   // difference (http://crbug.com/88255).
   return false;
 #else
   return ShellIntegration::CanSetAsDefaultProtocolClient();
 #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(
     const ProtocolHandlerRegistry::ProtocolHandlerMap& handler_map,
     const std::string& scheme) {
   ProtocolHandlerRegistry::ProtocolHandlerMap::const_iterator p =
       handler_map.find(scheme);
   if (p != handler_map.end()) {
     return p->second;
   }
@@ skipping 82 matching lines @@
   // and it will be automatically freed once all its tasks have finished.
   scoped_refptr<ShellIntegration::DefaultProtocolClientWorker> worker;
   worker = CreateShellWorker(observer, protocol);
   observer->SetWorker(worker);
   registry->default_client_observers_.push_back(observer);
   worker->StartSetAsDefault();
 }
 
 // ProtocolHandlerRegistry -----------------------------------------------------
 
-ProtocolHandlerRegistry::ProtocolHandlerRegistry(Profile* profile,
+ProtocolHandlerRegistry::ProtocolHandlerRegistry(
+    Profile* profile,
     Delegate* delegate)
     : profile_(profile),
       delegate_(delegate),
       enabled_(true),
       enabled_io_(enabled_),
       is_loading_(false),
       is_loaded_(false) {
 }
 
 bool ProtocolHandlerRegistry::SilentlyHandleRegisterHandlerRequest(
@@ skipping 66 matching lines @@
        p != handlers->end(); p++) {
     if (handler.IsSameOrigin(*p)) {
       replaced_handlers.push_back(*p);
     }
   }
   return replaced_handlers;
 }
 
 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();
 }
 
 bool ProtocolHandlerRegistry::IsDefault(
     const ProtocolHandler& handler) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   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;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_loading_ = true;
   PrefService* prefs = profile_->GetPrefs();
   if (prefs->HasPrefPath(prefs::kCustomHandlersEnabled)) {
     enabled_ = prefs->GetBoolean(prefs::kCustomHandlersEnabled);
     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);
   for (std::vector<const DictionaryValue*>::const_iterator p =
        registered_handlers.begin();
        p != registered_handlers.end(); ++p) {
     ProtocolHandler handler = ProtocolHandler::CreateProtocolHandler(*p);
     RegisterProtocolHandler(handler);
     bool is_default = false;
     if ((*p)->GetBoolean("default", &is_default) && is_default) {
@@ skipping 171 matching lines @@
   }
   ProtocolHandlerMap::iterator q = default_handlers_.find(handler.protocol());
   if (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.
       SetDefault(ProtocolHandler(handlers[0]));
     } 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);
     }
   }
 
   if (!IsHandledProtocol(handler.protocol())) {
     delegate_->DeregisterExternalHandler(handler.protocol());
   }
   Save();
   NotifyChanged();
@@ skipping 29 matching lines @@
 
 void ProtocolHandlerRegistry::Enable() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (enabled_) {
     return;
   }
   enabled_ = true;
   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);
   }
   Save();
   NotifyChanged();
 }
 
 void ProtocolHandlerRegistry::Disable() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!enabled_) {
     return;
   }
   enabled_ = false;
   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);
   }
   Save();
   NotifyChanged();
 }
 
 void ProtocolHandlerRegistry::Finalize() {

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

Rename Finalize() to Shutdown(), which is part of the PKS interface. Finalize()
was called during ~ProfileImpl(), but before the destruction phase starts.
Renaming this to Shutdown() will have this happen at the equivalent time, when
the rest of the object graph is still live.

(Unless you're really sure that you want to change this so that it runs during
destruction instead.)

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

Done.

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   delegate_.reset(NULL);
   // We free these now in case there are any outstanding workers running. If
   // we didn't free them they could respond to workers and try to update the
   // protocol handler registry after it was deleted.
   // Observers remove themselves from this list when they are deleted; so
   // we delete the last item until none are left in the list.
   while (!default_client_observers_.empty()) {
     delete default_client_observers_.back();
   }
 }
 
 // static
 void ProtocolHandlerRegistry::RegisterPrefs(PrefService* pref_service) {
   pref_service->RegisterListPref(prefs::kRegisteredProtocolHandlers,
                                  PrefService::UNSYNCABLE_PREF);
   pref_service->RegisterListPref(prefs::kIgnoredProtocolHandlers,
                                  PrefService::UNSYNCABLE_PREF);
   pref_service->RegisterBooleanPref(prefs::kCustomHandlersEnabled, true,
                                     PrefService::UNSYNCABLE_PREF);
 }
 
 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());
 }
 
 void ProtocolHandlerRegistry::PromoteHandler(const ProtocolHandler& handler) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(IsRegistered(handler));
   ProtocolHandlerMultiMap::iterator p =
       protocol_handlers_.find(handler.protocol());
   ProtocolHandlerList& list = p->second;
   list.erase(std::find(list.begin(), list.end(), handler));
@@ skipping 44 matching lines @@
   // If we're not loading, and we are setting a default for a new protocol,
   // register with the OS.
   if (!is_loading_ && p == default_handlers_.end())
       delegate_->RegisterWithOSAsDefaultClient(handler.protocol(), this);
   default_handlers_.erase(handler.protocol());
   default_handlers_.insert(std::make_pair(handler.protocol(), handler));
   PromoteHandler(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) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ProtocolHandlerMultiMap::iterator p =
       protocol_handlers_.find(handler.protocol());
 
   if (p != protocol_handlers_.end()) {
     p->second.push_back(handler);
     return;
@@ skipping 81 matching lines @@
   ignored_protocol_handlers_.push_back(handler);
 }
 
 void ProtocolHandlerRegistry::AddPredefinedHandler(
     const ProtocolHandler& handler) {
   // If called after the load command was issued this function will fail.
   DCHECK(!is_loaded_);
   RegisterProtocolHandler(handler);
   SetDefault(handler);
 }
-
-