Convert KeepAlive client to use BrowserInterfaceBroker

This CL converts KeepAlive mojom interface clients in extension to
use BrowserInterfaceBroker, and introduces
RegisterBrowserInterfaceBindersForFrame method in
ExtensionsBrowserClient class to register the extension interfaces
to a binder map for BrowserInterfaceBroker.

Bug: 936482
Change-Id: I6e793427ef32138454ae191c8623cabcee9a61f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946188
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721868}

chrome/app/chrome_content_browser_overlay_manifest.cc

@@ -67,10 +67,6 @@
 #include "chrome/browser/ui/webui/discards/discards.mojom.h"
 #endif
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "extensions/common/mojom/keep_alive.mojom.h"  // nogncheck
-#endif
-
 const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
   static base::NoDestructor<service_manager::Manifest> manifest {
     service_manager::ManifestBuilder()
@@ -139,9 +135,6 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
                 chromeos::multidevice_setup::mojom::PrivilegedHostDeviceSetter,
                 chromeos::network_config::mojom::CrosNetworkConfig,
                 cros::mojom::CameraAppDeviceProvider,
-#endif
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-                extensions::KeepAlive,
 #endif
                 media::mojom::MediaEngagementScoreDetailsProvider,
                 media_router::mojom::MediaRouter,

chrome/browser/chrome_content_browser_client.h

@@ -388,6 +388,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       service_manager::BinderRegistry* registry,
       content::RenderFrameHost* render_frame_host) override;
   void RegisterBrowserInterfaceBindersForFrame(
+      content::RenderFrameHost* render_frame_host,
       service_manager::BinderMapWithContext<content::RenderFrameHost*>* map)
       override;
   void BindInterfaceRequestFromFrame(

chrome/browser/chrome_content_browser_client_receiver_bindings.cc

@@ -50,6 +50,12 @@
 #include "chrome/browser/win/conflicts/module_event_sink_impl.h"
 #endif
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
+#include "chrome/browser/extensions/chrome_extensions_browser_client.h"
+#include "extensions/browser/extensions_browser_client.h"
+#endif
+
 #if BUILDFLAG(ENABLE_LIBRARY_CDMS)
 #include "chrome/browser/media/output_protection_impl.h"
 #include "chrome/browser/media/platform_verification_impl.h"
@@ -195,8 +201,26 @@ void ChromeContentBrowserClient::ExposeInterfacesToMediaService(
 }
 
 void ChromeContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    content::RenderFrameHost* render_frame_host,
     service_manager::BinderMapWithContext<content::RenderFrameHost*>* map) {
   chrome::internal::PopulateChromeFrameBinders(map);
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  if (!web_contents)
+    return;
+  auto* client = extensions::ExtensionsBrowserClient::Get();
+  auto* web_observer = client->GetExtensionWebContentsObserver(web_contents);
+  if (!web_observer)
+    return;
+  const extensions::Extension* extension =
+      web_observer->GetExtensionFromFrame(render_frame_host, false);
+  if (!extension)
+    return;
+  client->RegisterBrowserInterfaceBindersForFrame(map, render_frame_host,
+                                                  extension);
+#endif
 }
 
 void ChromeContentBrowserClient::BindInterfaceRequestFromFrame(

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -56,6 +56,7 @@
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/extensions_browser_interface_binders.h"
 #include "extensions/browser/mojo/interface_registration.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/browser/url_request_util.h"
@@ -336,6 +337,14 @@ void ChromeExtensionsBrowserClient::RegisterExtensionInterfaces(
   RegisterChromeInterfacesForExtension(registry, render_frame_host, extension);
 }
 
+void ChromeExtensionsBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) const {
+  PopulateExtensionFrameBinders(map, render_frame_host, extension);
+  PopulateChromeFrameBindersForExtension(map, render_frame_host, extension);
+}
+
 std::unique_ptr<RuntimeAPIDelegate>
 ChromeExtensionsBrowserClient::CreateRuntimeAPIDelegate(
     content::BrowserContext* context) const {

chrome/browser/extensions/chrome_extensions_browser_client.h

@@ -107,6 +107,10 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
                                        content::RenderFrameHost*>* registry,
                                    content::RenderFrameHost* render_frame_host,
                                    const Extension* extension) const override;
+  void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+      content::RenderFrameHost* render_frame_host,
+      const Extension* extension) const override;
   std::unique_ptr<RuntimeAPIDelegate> CreateRuntimeAPIDelegate(
       content::BrowserContext* context) const override;
   const ComponentExtensionResourceManager*

chrome/browser/extensions/chrome_extensions_interface_registration.cc

@@ -186,4 +186,9 @@ void RegisterChromeInterfacesForExtension(
 #endif
 }
 
+void PopulateChromeFrameBindersForExtension(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) {}
+
 }  // namespace extensions

chrome/browser/extensions/chrome_extensions_interface_registration.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_CHROME_EXTENSIONS_INTERFACE_REGISTRATION_H_
 #define CHROME_BROWSER_EXTENSIONS_CHROME_EXTENSIONS_INTERFACE_REGISTRATION_H_
 
+#include "services/service_manager/public/cpp/binder_map.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 
 namespace content {
@@ -20,6 +21,10 @@ void RegisterChromeInterfacesForExtension(
         registry,
     content::RenderFrameHost* render_frame_host,
     const Extension* extension);
+void PopulateChromeFrameBindersForExtension(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension);
 
 }  // namespace extensions
 

chrome/renderer/resources/extensions/media_router_bindings.js

@@ -851,8 +851,9 @@ MediaRouter.prototype.setKeepAlive = function(keepAlive) {
     this.keepAlive_ = null;
   } else if (keepAlive === true && !this.keepAlive_) {
     this.keepAlive_ = new extensions.KeepAlivePtr;
-    Mojo.bindInterface(extensions.KeepAlive.name,
-                       mojo.makeRequest(this.keepAlive_).handle);
+    Mojo.bindInterface(
+        extensions.KeepAlive.name, mojo.makeRequest(this.keepAlive_).handle,
+        'context', true);
   }
 };
 

chrome/test/base/mojo_web_ui_browser_test.cc

@@ -68,9 +68,11 @@ class MojoWebUIBrowserTest::WebUITestContentBrowserClient
   ~WebUITestContentBrowserClient() override {}
 
   void RegisterBrowserInterfaceBindersForFrame(
+      content::RenderFrameHost* render_frame_host,
       service_manager::BinderMapWithContext<content::RenderFrameHost*>* map)
       override {
-    ChromeContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(map);
+    ChromeContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+        render_frame_host, map);
     map->Add<web_ui_test::mojom::TestRunner>(
         base::BindRepeating(&WebUITestContentBrowserClient::BindWebUITestRunner,
                             base::Unretained(this)));

chromecast/browser/extensions/cast_extensions_browser_client.cc

@@ -26,6 +26,7 @@
 #include "extensions/browser/api/runtime/runtime_api_delegate.h"
 #include "extensions/browser/core_extensions_browser_api_provider.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/extensions_browser_interface_binders.h"
 #include "extensions/browser/mojo/interface_registration.h"
 #include "extensions/browser/null_app_sorting.h"
 #include "extensions/browser/updater/null_extension_cache.h"
@@ -211,6 +212,13 @@ void CastExtensionsBrowserClient::RegisterExtensionInterfaces(
   RegisterInterfacesForExtension(registry, render_frame_host, extension);
 }
 
+void CastExtensionsBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) const {
+  PopulateExtensionFrameBinders(map, render_frame_host, extension);
+}
+
 std::unique_ptr<RuntimeAPIDelegate>
 CastExtensionsBrowserClient::CreateRuntimeAPIDelegate(
     content::BrowserContext* context) const {

chromecast/browser/extensions/cast_extensions_browser_client.h

@@ -98,6 +98,10 @@ class CastExtensionsBrowserClient : public ExtensionsBrowserClient {
                                        content::RenderFrameHost*>* registry,
                                    content::RenderFrameHost* render_frame_host,
                                    const Extension* extension) const override;
+  void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+      content::RenderFrameHost* render_frame_host,
+      const Extension* extension) const override;
   std::unique_ptr<RuntimeAPIDelegate> CreateRuntimeAPIDelegate(
       content::BrowserContext* context) const override;
   const ComponentExtensionResourceManager*

content/browser/browser_interface_binders.cc

@@ -602,7 +602,8 @@ void PopulateBinderMapWithContext(
       base::BindRepeating(&BindTextSuggestionHostForFrame));
 #endif  // defined(OS_ANDROID)
 
-  GetContentClient()->browser()->RegisterBrowserInterfaceBindersForFrame(map);
+  GetContentClient()->browser()->RegisterBrowserInterfaceBindersForFrame(host,
+                                                                         map);
 }
 
 void PopulateBinderMap(RenderFrameHostImpl* host,

content/browser/webui/web_ui_mojo_browsertest.cc

@@ -246,6 +246,7 @@ class TestWebUIContentBrowserClient : public ContentBrowserClient {
   ~TestWebUIContentBrowserClient() override {}
 
   void RegisterBrowserInterfaceBindersForFrame(
+      RenderFrameHost* render_frame_host,
       service_manager::BinderMapWithContext<content::RenderFrameHost*>* map)
       override {
     map->Add<mojom::BrowserTarget>(

content/public/browser/content_browser_client.h

@@ -982,6 +982,7 @@ class CONTENT_EXPORT ContentBrowserClient {
   // This mechanism will replace interface registries and binders used for
   // handling InterfaceProvider's GetInterface() calls (see crbug.com/718652).
   virtual void RegisterBrowserInterfaceBindersForFrame(
+      RenderFrameHost* render_frame_host,
       service_manager::BinderMapWithContext<RenderFrameHost*>* map) {}
 
   // Content was unable to bind a request for this interface, so the embedder

extensions/browser/BUILD.gn

@@ -195,6 +195,8 @@ jumbo_source_set("browser_sources") {
     "extensions_browser_api_provider.h",
     "extensions_browser_client.cc",
     "extensions_browser_client.h",
+    "extensions_browser_interface_binders.cc",
+    "extensions_browser_interface_binders.h",
     "external_install_info.cc",
     "external_install_info.h",
     "external_provider_interface.h",

extensions/browser/OWNERS

@@ -10,3 +10,7 @@ per-file extension_function_histogram_value.h=file://extensions/common/api/API_O
 
 # DeviceLocalAccount reviewers.
 per-file device_local_account_util*=isandrk@chromium.org
+
+# For security review.
+per-file extensions_browser_interface_binders.cc=set noparent
+per-file extensions_browser_interface_binders.cc=file://ipc/SECURITY_OWNERS

extensions/browser/extensions_browser_client.h

@@ -21,6 +21,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "services/network/public/mojom/url_loader.mojom-forward.h"
+#include "services/service_manager/public/cpp/binder_map.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "ui/base/page_transition_types.h"
 
@@ -226,13 +227,20 @@ class ExtensionsBrowserClient {
   // ExtensionSystem::Get.
   virtual ExtensionSystemProvider* GetExtensionSystemFactory() = 0;
 
-  // Registers additional interfaces to expose to a RenderFrame.
+  // [Deprecated] Registers additional interfaces to expose to a RenderFrame.
   virtual void RegisterExtensionInterfaces(
       service_manager::BinderRegistryWithArgs<content::RenderFrameHost*>*
           registry,
       content::RenderFrameHost* render_frame_host,
       const Extension* extension) const = 0;
 
+  // Registers additional interfaces to a binder map for a browser interface
+  // broker.
+  virtual void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+      content::RenderFrameHost* render_frame_host,
+      const Extension* extension) const = 0;
+
   // Creates a RuntimeAPIDelegate responsible for handling extensions
   // management-related events such as update and installation on behalf of the
   // core runtime API implementation.

extensions/browser/extensions_browser_interface_binders.cc

+// Copyright 2019 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 "extensions/browser/extensions_browser_interface_binders.h"
+
+#include "base/bind.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "extensions/browser/mojo/keep_alive_impl.h"
+#include "extensions/common/mojom/keep_alive.mojom.h"  // nogncheck
+
+namespace extensions {
+
+void PopulateExtensionFrameBinders(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) {
+  DCHECK(extension);
+
+  map->Add<KeepAlive>(
+      base::BindRepeating(&KeepAliveImpl::Create,
+                          render_frame_host->GetProcess()->GetBrowserContext(),
+                          base::RetainedRef(extension)));
+}
+
+}  // namespace extensions

extensions/browser/extensions_browser_interface_binders.h

+// Copyright 2019 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.
+
+#ifndef EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_INTERFACE_BINDERS_H_
+#define EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_INTERFACE_BINDERS_H_
+
+#include "services/service_manager/public/cpp/binder_map.h"
+
+namespace content {
+class RenderFrameHost;
+}
+
+namespace extensions {
+
+class Extension;
+
+void PopulateExtensionFrameBinders(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension);
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_EXTENSIONS_BROWSER_INTERFACE_BINDERS_H_

extensions/browser/mojo/interface_registration.cc

@@ -13,7 +13,6 @@
 #include "content/public/browser/site_instance.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_browser_client.h"
-#include "extensions/browser/mojo/keep_alive_impl.h"
 #include "extensions/browser/process_map.h"
 #include "extensions/buildflags/buildflags.h"
 #include "extensions/common/constants.h"
@@ -51,11 +50,6 @@ void RegisterInterfacesForExtension(service_manager::BinderRegistryWithArgs<
                                     const Extension* extension) {
   DCHECK(extension);
 
-  registry->AddInterface(
-      base::Bind(KeepAliveImpl::Create,
-                 render_frame_host->GetProcess()->GetBrowserContext(),
-                 base::RetainedRef(extension)));
-
 #if BUILDFLAG(ENABLE_WIFI_DISPLAY)
   if (ExtensionHasPermission(extension, render_frame_host->GetProcess(),
                              "displaySource")) {

extensions/browser/mojo/keep_alive_impl.cc

@@ -15,8 +15,8 @@ namespace extensions {
 // static
 void KeepAliveImpl::Create(content::BrowserContext* context,
                            const Extension* extension,
-                           mojo::PendingReceiver<KeepAlive> receiver,
-                           content::RenderFrameHost* render_frame_host) {
+                           content::RenderFrameHost* render_frame_host,
+                           mojo::PendingReceiver<KeepAlive> receiver) {
   // Owns itself.
   new KeepAliveImpl(context, extension, std::move(receiver));
 }

extensions/browser/mojo/keep_alive_impl.h

@@ -30,8 +30,8 @@ class KeepAliveImpl : public KeepAlive, public ExtensionRegistryObserver {
   // |receiver|. When the receiver closes its pipe, the keep alive ends.
   static void Create(content::BrowserContext* browser_context,
                      const Extension* extension,
-                     mojo::PendingReceiver<KeepAlive> receiver,
-                     content::RenderFrameHost* render_frame_host);
+                     content::RenderFrameHost* render_frame_host,
+                     mojo::PendingReceiver<KeepAlive> receiver);
 
  private:
   KeepAliveImpl(content::BrowserContext* context,

extensions/browser/mojo/keep_alive_impl_unittest.cc

@@ -53,8 +53,8 @@ class KeepAliveTest : public ExtensionsTest {
   }
 
   void CreateKeepAlive(mojo::PendingReceiver<KeepAlive> receiver) {
-    KeepAliveImpl::Create(browser_context(), extension_.get(),
-                          std::move(receiver), nullptr);
+    KeepAliveImpl::Create(browser_context(), extension_.get(), nullptr,
+                          std::move(receiver));
   }
 
   const Extension* extension() { return extension_.get(); }

extensions/browser/test_extensions_browser_client.cc

@@ -212,6 +212,11 @@ void TestExtensionsBrowserClient::RegisterExtensionInterfaces(
     content::RenderFrameHost* render_frame_host,
     const Extension* extension) const {}
 
+void TestExtensionsBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) const {}
+
 std::unique_ptr<RuntimeAPIDelegate>
 TestExtensionsBrowserClient::CreateRuntimeAPIDelegate(
     content::BrowserContext* context) const {

extensions/browser/test_extensions_browser_client.h

@@ -123,6 +123,10 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
                                        content::RenderFrameHost*>* registry,
                                    content::RenderFrameHost* render_frame_host,
                                    const Extension* extension) const override;
+  void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+      content::RenderFrameHost* render_frame_host,
+      const Extension* extension) const override;
   std::unique_ptr<RuntimeAPIDelegate> CreateRuntimeAPIDelegate(
       content::BrowserContext* context) const override;
   const ComponentExtensionResourceManager*

extensions/renderer/resources/keep_alive.js

@@ -20,7 +20,7 @@ function KeepAlive() {
    * @private
    */
   this.handle_ = pipe.handle0;
-  Mojo.bindInterface(extensions.KeepAlive.name, pipe.handle1);
+  Mojo.bindInterface(extensions.KeepAlive.name, pipe.handle1, 'context', true);
 }
 
 /**

extensions/shell/browser/shell_extensions_browser_client.cc

@@ -19,6 +19,7 @@
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/core_extensions_browser_api_provider.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/extensions_browser_interface_binders.h"
 #include "extensions/browser/mojo/interface_registration.h"
 #include "extensions/browser/null_app_sorting.h"
 #include "extensions/browser/updater/null_extension_cache.h"
@@ -221,6 +222,13 @@ void ShellExtensionsBrowserClient::RegisterExtensionInterfaces(
   RegisterInterfacesForExtension(registry, render_frame_host, extension);
 }
 
+void ShellExtensionsBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+    content::RenderFrameHost* render_frame_host,
+    const Extension* extension) const {
+  PopulateExtensionFrameBinders(map, render_frame_host, extension);
+}
+
 std::unique_ptr<RuntimeAPIDelegate>
 ShellExtensionsBrowserClient::CreateRuntimeAPIDelegate(
     content::BrowserContext* context) const {

extensions/shell/browser/shell_extensions_browser_client.h

@@ -94,6 +94,10 @@ class ShellExtensionsBrowserClient : public ExtensionsBrowserClient {
                                        content::RenderFrameHost*>* registry,
                                    content::RenderFrameHost* render_frame_host,
                                    const Extension* extension) const override;
+  void RegisterBrowserInterfaceBindersForFrame(
+      service_manager::BinderMapWithContext<content::RenderFrameHost*>* map,
+      content::RenderFrameHost* render_frame_host,
+      const Extension* extension) const override;
   std::unique_ptr<RuntimeAPIDelegate> CreateRuntimeAPIDelegate(
       content::BrowserContext* context) const override;
   const ComponentExtensionResourceManager*

fuchsia/engine/browser/web_engine_content_browser_client.cc

@@ -109,6 +109,7 @@ void WebEngineContentBrowserClient::OverrideWebkitPrefs(
 }
 
 void WebEngineContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
+    content::RenderFrameHost* render_frame_host,
     service_manager::BinderMapWithContext<content::RenderFrameHost*>* map) {
   PopulateFuchsiaFrameBinders(map, &cdm_service_);
 }

fuchsia/engine/browser/web_engine_content_browser_client.h

@@ -37,6 +37,7 @@ class WebEngineContentBrowserClient : public content::ContentBrowserClient {
   void OverrideWebkitPrefs(content::RenderViewHost* rvh,
                            content::WebPreferences* web_prefs) final;
   void RegisterBrowserInterfaceBindersForFrame(
+      content::RenderFrameHost* render_frame_host,
       service_manager::BinderMapWithContext<content::RenderFrameHost*>* map)
       final;
   void RegisterNonNetworkNavigationURLLoaderFactories(