Site Settings Desktop: Implement protocol handler section.

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

Index: chrome/browser/ui/webui/settings/settings_handler_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/handler_options_handler.cc b/chrome/browser/ui/webui/settings/settings_handler_options_handler.cc
similarity index 78%
copy from chrome/browser/ui/webui/options/handler_options_handler.cc
copy to chrome/browser/ui/webui/settings/settings_handler_options_handler.cc
index 0e5ea47773ccf5c383087ac1f52bba846da501d2..45d871e192d2e0db2d27ecb5323698122651dc9d 100644
--- a/chrome/browser/ui/webui/options/handler_options_handler.cc
+++ b/chrome/browser/ui/webui/settings/settings_handler_options_handler.cc
@@ -1,8 +1,8 @@
-// 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>
@@ -22,7 +22,7 @@
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_ui.h"
 
-namespace options {
+namespace settings {
 
 HandlerOptionsHandler::HandlerOptionsHandler() {
 }
@@ -30,40 +30,20 @@ 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)));
@@ -91,10 +71,11 @@ static void GetHandlersAsListValue(
     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));
   }
 }
@@ -128,6 +109,8 @@ void HandlerOptionsHandler::GetIgnoredHandlers(base::ListValue* 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);
@@ -143,10 +126,26 @@ void HandlerOptionsHandler::UpdateHandlerList() {
 
   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) {
@@ -216,7 +215,8 @@ void HandlerOptionsHandler::Observe(
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_PROTOCOL_HANDLER_REGISTRY_CHANGED, type);
+  SendHandlersEnabledValue();
   UpdateHandlerList();
 }
 
-}  // namespace options
+}  // namespace settings