Preventing our default handlers for ChromeOS to show up or confuse the user

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 d81af988fc58af5d2e7f30e6b30679ff451d30bf..06627c6566a84a95eb1a77972a57d9be7e83efc2 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry.cc
@@ -10,6 +10,7 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -35,7 +36,30 @@ ProtocolHandlerRegistry::ProtocolHandlerRegistry(Profile* profile,
       delegate_(delegate),
       enabled_(true),
       enabled_io_(enabled_),
-      is_loading_(false) {
+      is_loading_(false),
+      fixed_handlers_loaded_(false) {
+}
+
+void ProtocolHandlerRegistry::InstallFixedHandlers() {
+#if defined(OS_CHROMEOS)
+  if (NULL == GetHandlerList(std::string("mailto"))) {
+    ProtocolHandler mail_handler = ProtocolHandler::CreateProtocolHandler(

[koz (OOO until 15th September), 2012/04/23 03:43:06]

nit: mail_handler -> mailto_handler

[Mr4D (OOO till 08-26), 2012/04/23 18:03:53]

Done.

+        std::string("mailto"),

[koz (OOO until 15th September), 2012/04/23 03:55:35]

Replace this with just "mailto"

[Mr4D (OOO till 08-26), 2012/04/23 18:03:53]

Not sure why this was done, but every other call of the function did it this
way.

+        GURL("https://mail.google.com/mail/?extsrc=mailto&url=%s"),
+        UTF8ToUTF16(std::string("Google.com Mail")));

[koz (OOO until 15th September), 2012/04/23 03:55:35]

Could you move the user-facing strings (ie: the title and the URL) into a GRD
file? Same with the webcal handler below.

[Mr4D (OOO till 08-26), 2012/04/23 18:06:21]

Done.

+    RegisterProtocolHandler(mail_handler);
+    SetDefault(mail_handler);
+  }
+  if (NULL == GetHandlerList(std::string("webcal"))) {
+    ProtocolHandler cal_handler = ProtocolHandler::CreateProtocolHandler(

[koz (OOO until 15th September), 2012/04/23 03:43:06]

nit: cal_handler -> webcal_handler

[Mr4D (OOO till 08-26), 2012/04/23 18:03:53]

Done.

+        std::string("webcal"),

[koz (OOO until 15th September), 2012/04/23 03:55:35]

Replace this with just "webcal"

[Mr4D (OOO till 08-26), 2012/04/23 18:03:53]

Ditto

+        GURL("https://www.google.com/calendar/render?cid=%s"),
+        UTF8ToUTF16(std::string("Google Calendar")));
+    RegisterProtocolHandler(cal_handler);
+    SetDefault(cal_handler);
+  }
+#endif
+  fixed_handlers_loaded_ = true;
 }
 
 ProtocolHandlerRegistry::~ProtocolHandlerRegistry() {
@@ -195,6 +219,11 @@ bool ShouldRemoveHandlersNotInOS() {
 } // namespace
 
 void ProtocolHandlerRegistry::Load() {
+  // We add our default handlers only the first time we come here.
+  // On the next call the defaults will be locked.

[koz (OOO until 15th September), 2012/04/23 03:43:06]

What does "locked" mean? Could you replace "will be locked" with "will be
already loaded"?

In fact, is this conditional even needed? Does Load() ever get called more than
once on a single instance of PHR? If it is why do we want the behaviour to be
different from the first time it is called? It seems okay to me to have the the
mailto and webcal handlers reinstated (if they have been removed) on every call
to Load().

[Mr4D (OOO till 08-26), 2012/04/23 18:03:53]

The "locked" comment was a remainder of some code which I had removed earlier.
Forgot to remove it.
I don't know if it ever gets called multiple times. When you say it only gets
called once (which would make sense) I believe you and remove the call blocker.
(Will test the default cases though).

+  if (!fixed_handlers_loaded_)
+    InstallFixedHandlers();
+
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_loading_ = true;
   PrefService* prefs = profile_->GetPrefs();