Convert TestRunner client to use BrowserInterfaceBroker

This CL converts TestRunner mojom interface client in chrome to
use BrowserInterfaceBroker and introduces
WebUITestContentBrowserClient class and overrides
ChromeContentBrowserClient::RegisterBrowserInterfaceBindersForFrame
to register its binder.

This change also updates JS files in closure_compile for Mojo
bindInterface and interface_definition.tmpl for WebUI tests.

Bug: 1006361, 1002647
Change-Id: Id830ef957eeb8b33ee47ff22918cb98b5c7f6650
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900798Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#715937}

chrome/app/BUILD.gn

@@ -417,7 +417,6 @@ source_set("chrome_content_browser_overlay_manifest") {
     "//chrome/common:mojo_bindings",
     "//chrome/common:offline_page_auto_fetcher_mojom",
     "//chrome/common/media_router/mojom:media_router",
-    "//chrome/test/data:web_ui_test_bindings",
     "//components/autofill/content/common/mojom",
     "//components/contextual_search/content/common/mojom",
     "//components/data_reduction_proxy/core/common:interfaces",

chrome/app/chrome_content_browser_overlay_manifest.cc

@@ -22,7 +22,6 @@
 #include "chrome/common/media_router/mojom/media_router.mojom.h"
 #include "chrome/common/net_benchmarking.mojom.h"
 #include "chrome/common/offline_page_auto_fetcher.mojom.h"
-#include "chrome/test/data/webui/web_ui_test.mojom.h"
 #include "components/autofill/content/common/mojom/autofill_driver.mojom.h"
 #include "components/contextual_search/content/common/mojom/contextual_search_js_api_service.mojom.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy.mojom.h"
@@ -185,8 +184,7 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
                 mojom::OmniboxPageHandler, mojom::ResetPasswordHandler,
                 mojom::SiteEngagementDetailsProvider,
                 mojom::UsbInternalsPageHandler,
-                snippets_internals::mojom::PageHandlerFactory,
-                web_ui_test::mojom::TestRunner>())
+                snippets_internals::mojom::PageHandlerFactory>())
         .PackageService(prefs::GetManifest())
 #if defined(OS_CHROMEOS)
         .PackageService(chromeos::multidevice_setup::GetManifest())

chrome/test/base/mojo_web_ui_browser_test.cc

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/webui/web_ui_test_handler.h"
 #include "chrome/common/chrome_paths.h"
@@ -18,8 +19,10 @@
 #include "chrome/test/data/webui/web_ui_test.mojom.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_client.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
+#include "services/service_manager/public/cpp/binder_map.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
@@ -54,10 +57,44 @@ class WebUITestPageHandler : public web_ui_test::mojom::TestRunner,
 
 }  // namespace
 
-MojoWebUIBrowserTest::MojoWebUIBrowserTest() {
-  registry_.AddInterface<web_ui_test::mojom::TestRunner>(base::BindRepeating(
-      &MojoWebUIBrowserTest::BindTestRunner, base::Unretained(this)));
-}
+class MojoWebUIBrowserTest::WebUITestContentBrowserClient
+    : public ChromeContentBrowserClient {
+ public:
+  WebUITestContentBrowserClient() {}
+  WebUITestContentBrowserClient(const WebUITestContentBrowserClient&) = delete;
+  WebUITestContentBrowserClient& operator=(
+      const WebUITestContentBrowserClient&) = delete;
+
+  ~WebUITestContentBrowserClient() override {}
+
+  void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map)
+      override {
+    ChromeContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(map);
+    map->Add<web_ui_test::mojom::TestRunner>(
+        base::BindRepeating(&WebUITestContentBrowserClient::BindWebUITestRunner,
+                            base::Unretained(this)));
+  }
+
+  void set_test_page_handler(WebUITestPageHandler* test_page_handler) {
+    test_page_handler_ = test_page_handler;
+  }
+
+  void BindWebUITestRunner(
+      content::RenderFrameHost* const render_frame_host,
+      mojo::PendingReceiver<web_ui_test::mojom::TestRunner> receiver) {
+    // Right now, this is expected to be called only for main frames.
+    ASSERT_FALSE(render_frame_host->GetParent());
+    test_page_handler_->BindToTestRunnerReceiver(std::move(receiver));
+  }
+
+ private:
+  WebUITestPageHandler* test_page_handler_;
+};
+
+MojoWebUIBrowserTest::MojoWebUIBrowserTest()
+    : test_content_browser_client_(
+          std::make_unique<WebUITestContentBrowserClient>()) {}
 
 MojoWebUIBrowserTest::~MojoWebUIBrowserTest() = default;
 
@@ -69,26 +106,8 @@ void MojoWebUIBrowserTest::SetUpOnMainThread() {
   pak_path = pak_path.AppendASCII("browser_tests.pak");
   ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath(
       pak_path, ui::SCALE_FACTOR_NONE);
-}
 
-void MojoWebUIBrowserTest::OnInterfaceRequestFromFrame(
-    content::RenderFrameHost* render_frame_host,
-    const std::string& interface_name,
-    mojo::ScopedMessagePipeHandle* interface_pipe) {
-  // Right now, this is expected to be called only for main frames.
-  if (render_frame_host->GetParent()) {
-    FAIL() << "Terminating renderer for requesting " << interface_name
-           << " interface from subframe";
-    render_frame_host->GetProcess()->ShutdownForBadMessage(
-        content::RenderProcessHost::CrashReportMode::GENERATE_CRASH_DUMP);
-    return;
-  }
-  registry_.TryBindInterface(interface_name, interface_pipe);
-}
-
-void MojoWebUIBrowserTest::BindTestRunner(
-    mojo::PendingReceiver<web_ui_test::mojom::TestRunner> receiver) {
-  test_page_handler_->BindToTestRunnerReceiver(std::move(receiver));
+  content::SetBrowserClientForTesting(test_content_browser_client_.get());
 }
 
 void MojoWebUIBrowserTest::SetupHandlers() {
@@ -99,10 +118,8 @@ void MojoWebUIBrowserTest::SetupHandlers() {
   ASSERT_TRUE(web_ui_instance != nullptr);
 
   auto test_handler = std::make_unique<WebUITestPageHandler>(web_ui_instance);
-  test_page_handler_ = test_handler.get();
+  test_content_browser_client_->set_test_page_handler(test_handler.get());
   set_test_handler(std::move(test_handler));
-
-  Observe(web_ui_instance->GetWebContents());
 }
 
 void MojoWebUIBrowserTest::BrowsePreload(const GURL& browse_to) {

chrome/test/base/mojo_web_ui_browser_test.h

@@ -10,13 +10,6 @@
 #include "chrome/test/base/web_ui_browser_test.h"
 #include "chrome/test/data/webui/web_ui_test.mojom.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "mojo/public/cpp/bindings/pending_receiver.h"
-#include "mojo/public/cpp/system/message_pipe.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
-
-namespace {
-class WebUITestPageHandler;
-}
 
 // The runner of Mojo WebUI javascript based tests. The main difference between
 // this and WebUIBrowserTest is that tests subclassing from this class use a
@@ -34,19 +27,10 @@ class MojoWebUIBrowserTest : public BaseWebUIBrowserTest,
   void SetUpOnMainThread() override;
   void SetupHandlers() override;
 
-  // content::WebContentsObserver:
-  void OnInterfaceRequestFromFrame(
-      content::RenderFrameHost* render_frame_host,
-      const std::string& interface_name,
-      mojo::ScopedMessagePipeHandle* interface_pipe) override;
-
  private:
-  void BindTestRunner(
-      mojo::PendingReceiver<web_ui_test::mojom::TestRunner> receiver);
-
-  WebUITestPageHandler* test_page_handler_;
+  class WebUITestContentBrowserClient;
+  std::unique_ptr<WebUITestContentBrowserClient> test_content_browser_client_;
 
-  service_manager::BinderRegistry registry_;
   bool use_mojo_lite_bindings_ = false;
 };
 

chrome/test/data/webui/test_api.js

@@ -716,10 +716,11 @@ function testDone(result) {
         const mojoMakeRequest = () => mojo.makeRequest(testRunner);
 
         Mojo.bindInterface(
-            webUiTest.mojom.TestRunner.name, mojoMakeRequest().handle);
+            webUiTest.mojom.TestRunner.name, mojoMakeRequest().handle,
+            'context', true);
       } else if (webUiTest.mojom.TestRunnerRemote) {
         // For mojo-lite WebUI tests.
-        testRunner = webUiTest.mojom.TestRunner.getRemote();
+        testRunner = webUiTest.mojom.TestRunner.getRemote(true);
       } else {
         assertNotReached(
             'Mojo bindings found, but no valid test interface loaded');

mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl

@@ -164,10 +164,11 @@ goog.provide('{{module.namespace}}.{{interface.name}}PendingReceiver');
    * @return {!{{module.namespace}}.{{interface.name}}Remote}
    * @export
    */
-  static getRemote() {
+  static getRemote(useBrowserInterfaceBroker = false) {
     let remote = new {{module.namespace}}.{{interface.name}}Remote;
     Mojo.bindInterface(this.$interfaceName,
-                       remote.$.bindNewPipeAndPassReceiver().handle);
+                       remote.$.bindNewPipeAndPassReceiver().handle,
+                       "context", useBrowserInterfaceBroker);
     return remote;
   }
 };

third_party/closure_compiler/externs/mojo.js

@@ -7,8 +7,11 @@ const Mojo = {};
 /**
  * @param {string} name
  * @param {MojoHandle} handle
+ * @param {string=} scope
+ * @param {boolean=} useBrowserInterfaceBroker
  */
-Mojo.bindInterface = function(name, handle) {};
+Mojo.bindInterface = function(
+    name, handle, scope, useBrowserInterfaceBroker) {};
 
 const MojoHandle = class {};
 

third_party/closure_compiler/externs/mojo_core.js

@@ -7,8 +7,11 @@ const Mojo = {};
 /**
  * @param {string} name
  * @param {MojoHandle} handle
+ * @param {string=} scope
+ * @param {boolean=} useBrowserInterfaceBroker
  */
-Mojo.bindInterface = function(name, handle) {};
+Mojo.bindInterface = function(
+    name, handle, scope, useBrowserInterfaceBroker) {};
 
 /** @typedef {number} */
 let MojoResult;