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

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"
 #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"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/custom_handlers/protocol_handler.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/notification_service.h"
 #include "net/base/network_delegate.h"
 #include "net/url_request/url_request_redirect_job.h"
 
 using content::BrowserThread;
 using content::ChildProcessSecurityPolicy;
 
 // ProtocolHandlerRegistry -----------------------------------------------------
 
 ProtocolHandlerRegistry::ProtocolHandlerRegistry(Profile* profile,
     Delegate* delegate)
     : 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() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(default_client_observers_.empty());
 }
 
 void ProtocolHandlerRegistry::Finalize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   delegate_.reset(NULL);
@@ skipping 139 matching lines @@
   // difference (http://crbug.com/88255).
   return false;
 #else
   return ShellIntegration::CanSetAsDefaultProtocolClient();
 #endif
 }
 
 } // 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();
   if (prefs->HasPrefPath(prefs::kCustomHandlersEnabled)) {
     enabled_ = prefs->GetBoolean(prefs::kCustomHandlersEnabled);
     BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
         base::Bind(enabled_ ? &ProtocolHandlerRegistry::EnableIO :
                    &ProtocolHandlerRegistry::DisableIO, this));
@@ skipping 496 matching lines @@
     }
   } else {
     NOTREACHED();
   }
 }
 
 void ProtocolHandlerRegistry::DefaultClientObserver::SetWorker(
     ShellIntegration::DefaultProtocolClientWorker* worker) {
   worker_ = worker;
 }