Stop registering content::GenericHandler blindly for all WebUIs.

 - Move GenericHandler from content/ to chrome/
 - Rename to NavigationHandler
 - Stop registering it blindly for all WebUI pages within WebContentsImpl.
 - Register it explicitly in HistoryUI, the only currently known usage of
   it.
 - Change util.js so that a call to listenForPrivilegedLinkClicks() is
   required to trigger that functionality. Previously listeners were added
   as soon as util.js was evaluated.

Note there are probably a few more places that use NavigationHandler to be
registered. The goal of this CL is to narrow down who needs this functionality
and explicitly add it there. So some temporary regressions are expected.

Bug: 1020360
Change-Id: I2e2f5a0fe06cb8268a38380f29547afa94e7450c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894911
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712251}

chrome/browser/resources/history/app.js

@@ -111,6 +111,11 @@ Polymer({
   /** @private {?function(!Event)} */
   boundOnCommand_: null,
 
+  /** @override */
+  created: function() {
+    listenForPrivilegedLinkClicks();
+  },
+
   /** @override */
   attached: function() {
     cr.ui.decorate('command', cr.ui.Command);

chrome/browser/ui/BUILD.gn

@@ -261,6 +261,8 @@ jumbo_static_library("ui") {
     "webui/memory_internals_ui.h",
     "webui/metrics_handler.cc",
     "webui/metrics_handler.h",
+    "webui/navigation_handler.cc",
+    "webui/navigation_handler.h",
     "webui/net_export_ui.cc",
     "webui/net_export_ui.h",
     "webui/net_internals/net_internals_ui.cc",

chrome/browser/ui/webui/history_ui.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/ui/webui/localized_string.h"
 #include "chrome/browser/ui/webui/managed_ui_handler.h"
 #include "chrome/browser/ui/webui/metrics_handler.h"
+#include "chrome/browser/ui/webui/navigation_handler.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/browser_resources.h"
@@ -181,6 +182,7 @@ HistoryUI::HistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   ManagedUIHandler::Initialize(web_ui, data_source);
   content::WebUIDataSource::Add(profile, data_source);
 
+  web_ui->AddMessageHandler(std::make_unique<webui::NavigationHandler>());
   web_ui->AddMessageHandler(std::make_unique<BrowsingHistoryHandler>());
   web_ui->AddMessageHandler(std::make_unique<MetricsHandler>());
 

content/browser/webui/generic_handler.cc → chrome/browser/ui/webui/navigation_handler.cc

@@ -2,31 +2,30 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/webui/generic_handler.h"
+#include "chrome/browser/ui/webui/navigation_handler.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/values.h"
-#include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui.h"
 #include "ui/base/window_open_disposition.h"
 
-namespace content {
+namespace webui {
 
-GenericHandler::GenericHandler() {
-}
+NavigationHandler::NavigationHandler() {}
 
-GenericHandler::~GenericHandler() {
-}
+NavigationHandler::~NavigationHandler() {}
 
-void GenericHandler::RegisterMessages() {
+void NavigationHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
-      "navigateToUrl", base::BindRepeating(&GenericHandler::HandleNavigateToUrl,
+      "navigateToUrl",
+      base::BindRepeating(&NavigationHandler::HandleNavigateToUrl,
                           base::Unretained(this)));
 }
 
-void GenericHandler::HandleNavigateToUrl(const base::ListValue* args) {
+void NavigationHandler::HandleNavigateToUrl(const base::ListValue* args) {
   std::string url_string;
   std::string target_string;
   double button;
@@ -52,13 +51,11 @@ void GenericHandler::HandleNavigateToUrl(const base::ListValue* args) {
       target_string == "_blank")
     disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
 
-  web_ui()->GetWebContents()->OpenURL(OpenURLParams(GURL(url_string),
-                                                    Referrer(),
-                                                    disposition,
-                                                    ui::PAGE_TRANSITION_LINK,
-                                                    false));
+  web_ui()->GetWebContents()->OpenURL(
+      content::OpenURLParams(GURL(url_string), content::Referrer(), disposition,
+                             ui::PAGE_TRANSITION_LINK, false));
 
   // This may delete us!
 }
 
-}  // namespace content
+}  // namespace webui

content/browser/webui/generic_handler.h → chrome/browser/ui/webui/navigation_handler.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CONTENT_BROWSER_WEBUI_GENERIC_HANDLER_H_
-#define CONTENT_BROWSER_WEBUI_GENERIC_HANDLER_H_
+#ifndef CHROME_BROWSER_UI_WEBUI_NAVIGATION_HANDLER_H_
+#define CHROME_BROWSER_UI_WEBUI_NAVIGATION_HANDLER_H_
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -13,13 +13,13 @@ namespace base {
 class ListValue;
 }
 
-namespace content {
+namespace webui {
 
 // A place to add handlers for messages shared across all WebUI pages.
-class GenericHandler : public WebUIMessageHandler {
+class NavigationHandler : public content::WebUIMessageHandler {
  public:
-  GenericHandler();
-  ~GenericHandler() override;
+  NavigationHandler();
+  ~NavigationHandler() override;
 
   // WebUIMessageHandler implementation.
   void RegisterMessages() override;
@@ -27,9 +27,9 @@ class GenericHandler : public WebUIMessageHandler {
  private:
   void HandleNavigateToUrl(const base::ListValue* args);
 
-  DISALLOW_COPY_AND_ASSIGN(GenericHandler);
+  DISALLOW_COPY_AND_ASSIGN(NavigationHandler);
 };
 
-}  // namespace content
+}  // namespace webui
 
-#endif  // CONTENT_BROWSER_WEBUI_GENERIC_HANDLER_H_
+#endif  // CHROME_BROWSER_UI_WEBUI_NAVIGATION_HANDLER_H_

chrome/test/data/webui/js/util_test.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {$, quoteString} from 'chrome://resources/js/util.m.js';
+import {$, listenForPrivilegedLinkClicks, quoteString} from 'chrome://resources/js/util.m.js';
 
 suite('UtilModuleTest', function() {
   test('quote string', function() {
@@ -24,6 +24,7 @@ suite('UtilModuleTest', function() {
   });
 
   test('click handler', function() {
+    listenForPrivilegedLinkClicks();
     document.body.innerHTML = `
       <a id="file" href="file:///path/to/file">File</a>
       <a id="chrome" href="about:chrome">Chrome</a>

chrome/test/data/webui/util_test.html

@@ -24,6 +24,7 @@ function testQuoteString() {
 }
 
 function testClickHandler() {
+  listenForPrivilegedLinkClicks();
   var clickArgs = null;
   var oldSend = chrome.send;
   chrome.send = function(message, args) {

content/browser/BUILD.gn

@@ -1922,8 +1922,6 @@ jumbo_source_set("browser") {
     "websockets/websocket_handshake_request_info_impl.h",
     "webui/content_web_ui_controller_factory.cc",
     "webui/content_web_ui_controller_factory.h",
-    "webui/generic_handler.cc",
-    "webui/generic_handler.h",
     "webui/network_error_url_loader.cc",
     "webui/network_error_url_loader.h",
     "webui/shared_resources_data_source.cc",

content/browser/web_contents/web_contents_impl.cc

@@ -86,7 +86,6 @@
 #include "content/browser/web_contents/javascript_dialog_navigation_deferrer.h"
 #include "content/browser/web_contents/web_contents_view_child_frame.h"
 #include "content/browser/web_contents/web_contents_view_guest.h"
-#include "content/browser/webui/generic_handler.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/browser/webui/web_ui_impl.h"
 #include "content/common/browser_plugin/browser_plugin_constants.h"
@@ -6822,7 +6821,6 @@ std::unique_ptr<WebUIImpl> WebContentsImpl::CreateWebUI(const GURL& url) {
       WebUIControllerFactoryRegistry::GetInstance()
           ->CreateWebUIControllerForURL(web_ui.get(), url));
   if (controller) {
-    web_ui->AddMessageHandler(std::make_unique<GenericHandler>());
     web_ui->SetController(std::move(controller));
     return web_ui;
   }

ui/webui/resources/js/util.js

@@ -155,9 +155,13 @@
       element, HTMLElement, 'Missing required element: ' + selectors);
 }
 
-// Handle click on a link. If the link points to a chrome: or file: url, then
-// call into the browser to do the navigation.
-['click', 'auxclick'].forEach(function(eventName) {
+// Adds click/auxclick listeners for any link on the page. If the link points to
+// a chrome: or file: url, then calls into the browser to do the navigation.
+// Note: This method is *not* re-entrant. Every call to it, will re-add
+// listeners on |document|. It's up to callers to ensure this is only called
+// once.
+/* #export */ function listenForPrivilegedLinkClicks() {
+  ['click', 'auxclick'].forEach(function(eventName) {
     document.addEventListener(eventName, function(e) {
       if (e.button > 1) {
         return;
@@ -202,7 +206,8 @@
         e.preventDefault();
       }
     });
-});
+  });
+}
 
 /**
  * Creates a new URL which is the old URL with a GET param of key=value.