Site Settings Desktop: Implement protocol handler section.

chrome/browser/ui/webui/settings/settings_handler_options_handler.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2016 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/ui/webui/options/handler_options_handler.h"
+#include "chrome/browser/ui/webui/settings/settings_handler_options_handler.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_ui.h"
 
-namespace options {
+namespace settings {
 
 HandlerOptionsHandler::HandlerOptionsHandler() {
 }
 
 HandlerOptionsHandler::~HandlerOptionsHandler() {
 }
 
-void HandlerOptionsHandler::GetLocalizedValues(
-    base::DictionaryValue* localized_strings) {
-  DCHECK(localized_strings);
-
-  static OptionsStringResource resources[] = {
-      { "handlersTabLabel", IDS_HANDLERS_TAB_LABEL },
-      { "handlersAllow", IDS_HANDLERS_ALLOW_RADIO },
-      { "handlersBlock", IDS_HANDLERS_DONOTALLOW_RADIO },
-      { "handlersTypeColumnHeader", IDS_HANDLERS_TYPE_COLUMN_HEADER },
-      { "handlersSiteColumnHeader", IDS_HANDLERS_SITE_COLUMN_HEADER },
-      { "handlersRemoveLink", IDS_HANDLERS_REMOVE_HANDLER_LINK },
-      { "handlersNoneHandler", IDS_HANDLERS_NONE_HANDLER },
-      { "handlersActiveHeading", IDS_HANDLERS_ACTIVE_HEADING },
-      { "handlersIgnoredHeading", IDS_HANDLERS_IGNORED_HEADING },
-  };
-  RegisterTitle(localized_strings, "handlersPage",
-                IDS_HANDLER_OPTIONS_WINDOW_TITLE);
-  RegisterStrings(localized_strings, resources, arraysize(resources));
-
-  localized_strings->SetString("handlersLearnMoreUrl",
-                               chrome::kLearnMoreRegisterProtocolHandlerURL);
-}
-
-void HandlerOptionsHandler::InitializeHandler() {
+void HandlerOptionsHandler::OnJavascriptAllowed() {
   notification_registrar_.Add(
       this, chrome::NOTIFICATION_PROTOCOL_HANDLER_REGISTRY_CHANGED,
       content::Source<Profile>(Profile::FromWebUI(web_ui())));
 }
 
-void HandlerOptionsHandler::InitializePage() {
-  UpdateHandlerList();
+void HandlerOptionsHandler::OnJavascriptDisallowed() {
+
 }
 
 void HandlerOptionsHandler::RegisterMessages() {
+  web_ui()->RegisterMessageCallback("initializeProtocolHandlerList",
+      base::Bind(&HandlerOptionsHandler::InitializeProtocolHandlerList,
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback("clearDefault",
       base::Bind(&HandlerOptionsHandler::ClearDefault,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback("removeHandler",
       base::Bind(&HandlerOptionsHandler::RemoveHandler,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback("setHandlersEnabled",
       base::Bind(&HandlerOptionsHandler::SetHandlersEnabled,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback("setDefault",
       base::Bind(&HandlerOptionsHandler::SetDefault,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback("removeIgnoredHandler",
       base::Bind(&HandlerOptionsHandler::RemoveIgnoredHandler,
                  base::Unretained(this)));
 }
 
 ProtocolHandlerRegistry* HandlerOptionsHandler::GetProtocolHandlerRegistry() {
   return ProtocolHandlerRegistryFactory::GetForBrowserContext(
       Profile::FromWebUI(web_ui()));
 }
 
 static void GetHandlersAsListValue(
     const ProtocolHandlerRegistry::ProtocolHandlerList& handlers,
     base::ListValue* handler_list) {
   ProtocolHandlerRegistry::ProtocolHandlerList::const_iterator handler;
   for (handler = handlers.begin(); handler != handlers.end(); ++handler) {
-    std::unique_ptr<base::ListValue> handler_value(new base::ListValue());
-    handler_value->AppendString(handler->protocol());
-    handler_value->AppendString(handler->url().spec());
-    handler_value->AppendString(handler->url().host());
+    std::unique_ptr<base::DictionaryValue> handler_value(
+        new base::DictionaryValue());
+    handler_value->SetString("protocol", handler->protocol());
+    handler_value->SetString("spec", handler->url().spec());
+    handler_value->SetString("host", handler->url().host());
     handler_list->Append(std::move(handler_value));
   }
 }
 
 void HandlerOptionsHandler::GetHandlersForProtocol(
     const std::string& protocol,
     base::DictionaryValue* handlers_value) {
   ProtocolHandlerRegistry* registry = GetProtocolHandlerRegistry();
   // The items which are to be written into |handlers_value| are also described
   // in chrome/browser/resources/options/handler_options.js in @typedef
@@ skipping 13 matching lines @@
 }
 
 void HandlerOptionsHandler::GetIgnoredHandlers(base::ListValue* handlers) {
   ProtocolHandlerRegistry* registry = GetProtocolHandlerRegistry();
   ProtocolHandlerRegistry::ProtocolHandlerList ignored_handlers =
       registry->GetIgnoredHandlers();
   return GetHandlersAsListValue(ignored_handlers, handlers);
 }
 
 void HandlerOptionsHandler::UpdateHandlerList() {
+  AllowJavascript();

[michaelpg, 2016/07/08 20:56:00]

there's some circular logic with having AllowJavascript here...
UpdateHandlerList can be called by Observe, whose observer is added in
OnJavascriptAllowed which is called after AllowJavascript.

It *works*, but I think the convention to call AllowJavascript in message
callback handlers (functions taking "ListValue* args") is more obvious.

The way you have it, it *seems* like you're always calling AllowJavascript *in
order to allow javascript* before you call JavaScript, which could be wrong.
What I see elsewhere is to call AllowJavascript in javascript message handlers
to *signal that JavaScript has been allowed*.

[Finnur, 2016/07/10 00:40:17]

Done.

+
   ProtocolHandlerRegistry* registry = GetProtocolHandlerRegistry();
   std::vector<std::string> protocols;
   registry->GetRegisteredProtocols(&protocols);
 
   base::ListValue handlers;
   for (std::vector<std::string>::iterator protocol = protocols.begin();
        protocol != protocols.end(); protocol++) {
     std::unique_ptr<base::DictionaryValue> handler_value(
         new base::DictionaryValue());
     GetHandlersForProtocol(*protocol, handler_value.get());
     handlers.Append(std::move(handler_value));
   }
 
   std::unique_ptr<base::ListValue> ignored_handlers(new base::ListValue());
   GetIgnoredHandlers(ignored_handlers.get());
-  web_ui()->CallJavascriptFunctionUnsafe("HandlerOptions.setHandlers",
-                                         handlers);
-  web_ui()->CallJavascriptFunctionUnsafe("HandlerOptions.setIgnoredHandlers",
-                                         *ignored_handlers);
+  CallJavascriptFunction("cr.webUIListenerCallback",
+                         base::StringValue("setProtocolHandlers"),
+                         handlers);
+  CallJavascriptFunction("cr.webUIListenerCallback",
+                         base::StringValue("setIgnoredProtocolHandlers"),
+                         *ignored_handlers);
+}
+
+void HandlerOptionsHandler::InitializeProtocolHandlerList(
+    const base::ListValue* args) {
+  SendHandlersEnabledValue();
+  UpdateHandlerList();
+}
+
+void HandlerOptionsHandler::SendHandlersEnabledValue() {
+  AllowJavascript();
+  CallJavascriptFunction("cr.webUIListenerCallback",
+                         base::StringValue("setHandlersEnabled"),
+                         base::FundamentalValue(
+                            GetProtocolHandlerRegistry()->enabled()));

[michaelpg, 2016/07/08 20:56:00]

indent off

[Finnur, 2016/07/10 00:40:17]

Wow. I'm impressed. Did you really catch this by eyeballing it? :)

Done.

[michaelpg, 2016/07/10 17:54:43]

No, I only looked closely because it looked different from the function calls at
129 and 132.

 }
 
 void HandlerOptionsHandler::RemoveHandler(const base::ListValue* args) {
   const base::ListValue* list;
   if (!args->GetList(0, &list)) {
     NOTREACHED();
     return;
   }
 
   ProtocolHandler handler(ParseHandlerFromArgs(list));
@@ skipping 49 matching lines @@
     return ProtocolHandler::EmptyProtocolHandler();
   return ProtocolHandler::CreateProtocolHandler(base::UTF16ToUTF8(protocol),
                                                 GURL(base::UTF16ToUTF8(url)));
 }
 
 void HandlerOptionsHandler::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_PROTOCOL_HANDLER_REGISTRY_CHANGED, type);
+  SendHandlersEnabledValue();
   UpdateHandlerList();
 }
 
-}  // namespace options
+}  // namespace settings