Migrate ContextualSearchJsApiService to BrowserInterfaceBroker

This CL migrates the mojo client in the renderer side of the
contextual_search component to the new BrowserInterfaceBroker,
and adapts the code in the browser process to register the binder
from PopulateChromeFrameBinders(), instead of doing it inside of
ContextualSearchObserver.

Last, this CL also extracts the ContextualSearchObserver class
out of contextual_search_manager.cc, so that the receiver can
be bound now via the BinderMap owned by the BrowserInterfaceBroker.

Bug: 936482,1006361
Change-Id: I95c8046650883c872750dd7408a38e7433ed9df3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906667Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#721405}

chrome/app/chrome_content_browser_overlay_manifest.cc

@@ -140,7 +140,6 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
                 chromeos::network_config::mojom::CrosNetworkConfig,
                 cros::mojom::CameraAppDeviceProvider,
 #endif
-                contextual_search::mojom::ContextualSearchJsApiService,
 #if BUILDFLAG(ENABLE_EXTENSIONS)
                 extensions::KeepAlive,
 #endif

chrome/browser/BUILD.gn

@@ -2374,6 +2374,8 @@ jumbo_static_library("browser") {
       "android/contextualsearch/contextual_search_field_trial.h",
       "android/contextualsearch/contextual_search_manager.cc",
       "android/contextualsearch/contextual_search_manager.h",
+      "android/contextualsearch/contextual_search_observer.cc",
+      "android/contextualsearch/contextual_search_observer.h",
       "android/contextualsearch/contextual_search_ranker_logger_impl.cc",
       "android/contextualsearch/contextual_search_ranker_logger_impl.h",
       "android/contextualsearch/contextual_search_tab_helper.cc",

chrome/browser/android/contextualsearch/contextual_search_manager.cc

@@ -15,66 +15,23 @@
 #include "base/time/time.h"
 #include "chrome/android/chrome_jni_headers/ContextualSearchManager_jni.h"
 #include "chrome/browser/android/contextualsearch/contextual_search_delegate.h"
+#include "chrome/browser/android/contextualsearch/contextual_search_observer.h"
 #include "chrome/browser/android/contextualsearch/resolved_search_term.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "components/contextual_search/content/browser/contextual_search_js_api_service_impl.h"
 #include "components/navigation_interception/intercept_navigation_delegate.h"
 #include "components/variations/variations_associated_data.h"
-#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "net/url_request/url_fetcher_impl.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
 
 using base::android::JavaParamRef;
 using base::android::JavaRef;
 using content::WebContents;
 
-namespace {
-
-const char kContextualSearchObserverKey[] = "contextual_search_observer";
-
-class ContextualSearchObserver : public content::WebContentsObserver,
-                                 public base::SupportsUserData::Data {
- public:
-  ContextualSearchObserver(
-      content::WebContents* contents,
-      contextual_search::ContextualSearchJsApiHandler* api_handler)
-      : content::WebContentsObserver(contents) {
-    registry_.AddInterface(base::Bind(
-        &contextual_search::CreateContextualSearchJsApiService, api_handler));
-  }
-  ~ContextualSearchObserver() override = default;
-
-  static void EnsureForWebContents(
-      content::WebContents* contents,
-      contextual_search::ContextualSearchJsApiHandler* api_handler) {
-    // Clobber any prior registered observer.
-    contents->SetUserData(
-        kContextualSearchObserverKey,
-        std::make_unique<ContextualSearchObserver>(contents, api_handler));
-  }
-
- private:
-  // content::WebContentsObserver:
-  void OnInterfaceRequestFromFrame(
-      content::RenderFrameHost* render_frame_host,
-      const std::string& interface_name,
-      mojo::ScopedMessagePipeHandle* interface_pipe) override {
-    registry_.TryBindInterface(interface_name, interface_pipe);
-  }
-
-  service_manager::BinderRegistry registry_;
-
-  DISALLOW_COPY_AND_ASSIGN(ContextualSearchObserver);
-};
-
-}  // namespace
-
 // This class manages the native behavior of the Contextual Search feature.
 // Instances of this class are owned by the Java ContextualSearchManager.
 // Most of the work is actually done in an associated delegate to this class:
@@ -221,7 +178,11 @@ void ContextualSearchManager::EnableContextualSearchJsApiForWebContents(
   WebContents* overlay_web_contents =
       WebContents::FromJavaWebContents(j_overlay_web_contents);
   DCHECK(overlay_web_contents);
-  ContextualSearchObserver::EnsureForWebContents(overlay_web_contents, this);
+
+  // It's safe to use a raw pointer since the lifetime of |this| matches the
+  // application lifetime, and therefore spans multiple WebContents.
+  contextual_search::ContextualSearchObserver::SetHandlerForWebContents(
+      overlay_web_contents, this);
 }
 
 void ContextualSearchManager::WhitelistContextualSearchJsApiUrl(

chrome/browser/android/contextualsearch/contextual_search_observer.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 "chrome/browser/android/contextualsearch/contextual_search_observer.h"
+
+#include "components/contextual_search/content/browser/contextual_search_js_api_handler.h"
+#include "content/public/browser/web_contents.h"
+
+namespace contextual_search {
+
+ContextualSearchObserver::ContextualSearchObserver(
+    content::WebContents* web_contents)
+    : content::WebContentsObserver(web_contents) {}
+
+ContextualSearchObserver::~ContextualSearchObserver() = default;
+
+// static
+void ContextualSearchObserver::SetHandlerForWebContents(
+    content::WebContents* web_contents,
+    ContextualSearchJsApiHandler* handler) {
+  DCHECK(web_contents);
+  DCHECK(handler);
+
+  // Clobber any prior registered observer.
+  web_contents->RemoveUserData(UserDataKey());
+  CreateForWebContents(web_contents);
+  auto* contextual_search_observer = FromWebContents(web_contents);
+  contextual_search_observer->set_api_handler(handler);
+}
+
+WEB_CONTENTS_USER_DATA_KEY_IMPL(ContextualSearchObserver)
+
+}  // namespace contextual_search

chrome/browser/android/contextualsearch/contextual_search_observer.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 CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_CONTEXTUAL_SEARCH_OBSERVER_H_
+#define CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_CONTEXTUAL_SEARCH_OBSERVER_H_
+
+#include "base/macros.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace content {
+class WebContents;
+}
+
+namespace contextual_search {
+
+class ContextualSearchJsApiHandler;
+
+// Observer for the |web_contents| passed to the constructor, which will be used
+// later on to call contextual_search::CreateContextualSearchJsApiService() at
+// the time of binding the mojo::Receiver<mojom::ContextualSearchJsApiService>
+// to the implementation of the ContextualSearchJsApiService interface.
+class ContextualSearchObserver
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<ContextualSearchObserver> {
+ public:
+  explicit ContextualSearchObserver(content::WebContents* web_contents);
+  ~ContextualSearchObserver() override;
+
+  static void SetHandlerForWebContents(content::WebContents* web_contents,
+                                       ContextualSearchJsApiHandler* handler);
+
+  ContextualSearchJsApiHandler* api_handler() const { return api_handler_; }
+
+ private:
+  friend class content::WebContentsUserData<ContextualSearchObserver>;
+
+  void set_api_handler(ContextualSearchJsApiHandler* handler) {
+    api_handler_ = handler;
+  }
+
+  ContextualSearchJsApiHandler* api_handler_;
+
+  WEB_CONTENTS_USER_DATA_KEY_DECL();
+
+  DISALLOW_COPY_AND_ASSIGN(ContextualSearchObserver);
+};
+
+}  // namespace contextual_search
+
+#endif  // CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_CONTEXTUAL_SEARCH_OBSERVER_H_

chrome/browser/chrome_browser_interface_binders.cc

@@ -44,9 +44,12 @@
 #endif  // BUILDFLAG(ENABLE_UNHANDLED_TAP)
 
 #if defined(OS_ANDROID)
+#include "chrome/browser/android/contextualsearch/contextual_search_observer.h"
 #include "chrome/browser/android/dom_distiller/distiller_ui_handle_android.h"
 #include "chrome/browser/offline_pages/android/offline_page_auto_fetcher.h"
 #include "chrome/common/offline_page_auto_fetcher.mojom.h"
+#include "components/contextual_search/content/browser/contextual_search_js_api_service_impl.h"
+#include "components/contextual_search/content/common/mojom/contextual_search_js_api_service.mojom.h"
 #include "content/public/browser/web_contents.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/mojom/installedapp/installed_app_provider.mojom.h"
@@ -85,6 +88,26 @@ void BindUnhandledTapWebContentsObserver(
 }
 #endif  // BUILDFLAG(ENABLE_UNHANDLED_TAP)
 
+#if defined(OS_ANDROID)
+void BindContextualSearchObserver(
+    content::RenderFrameHost* const host,
+    mojo::PendingReceiver<
+        contextual_search::mojom::ContextualSearchJsApiService> receiver) {
+  // Early return if the RenderFrameHost's delegate is not a WebContents.
+  auto* web_contents = content::WebContents::FromRenderFrameHost(host);
+  if (!web_contents)
+    return;
+
+  auto* contextual_search_observer =
+      contextual_search::ContextualSearchObserver::FromWebContents(
+          web_contents);
+  if (contextual_search_observer) {
+    contextual_search::CreateContextualSearchJsApiService(
+        contextual_search_observer->api_handler(), std::move(receiver));
+  }
+}
+#endif
+
 // Forward image Annotator requests to the profile's AccessibilityLabelsService.
 void BindImageAnnotator(
     content::RenderFrameHost* frame_host,
@@ -258,6 +281,12 @@ void PopulateChromeFrameBinders(
         base::BindRepeating(&payments::CreatePaymentRequest));
   }
 #endif
+
+#if defined(OS_ANDROID)
+  map->Add<contextual_search::mojom::ContextualSearchJsApiService>(
+      base::BindRepeating(&BindContextualSearchObserver));
+#endif
+
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   map->Add<extensions::mime_handler::MimeHandlerService>(
       base::BindRepeating(&BindMimeHandlerService));

components/contextual_search/content/renderer/contextual_search_wrapper.cc

@@ -9,7 +9,7 @@
 #include "content/public/renderer/render_frame.h"
 #include "gin/arguments.h"
 #include "gin/object_template_builder.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/web/blink.h"
 #include "third_party/blink/public/web/web_frame.h"
 #include "third_party/blink/public/web/web_local_frame.h"
@@ -76,7 +76,7 @@ gin::ObjectTemplateBuilder ContextualSearchWrapper::GetObjectTemplateBuilder(
 
 bool ContextualSearchWrapper::EnsureServiceConnected() {
   if (render_frame() && !contextual_search_js_api_service_) {
-    render_frame()->GetRemoteInterfaces()->GetInterface(
+    render_frame()->GetBrowserInterfaceBroker()->GetInterface(
         contextual_search_js_api_service_.BindNewPipeAndPassReceiver());
     return true;
   }

components/contextual_search/content/renderer/overlay_js_render_frame_observer.cc

@@ -10,7 +10,7 @@
 #include "components/contextual_search/content/renderer/contextual_search_wrapper.h"
 #include "content/public/renderer/render_frame.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/platform/web_url.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "v8/include/v8.h"
@@ -45,7 +45,7 @@ void OverlayJsRenderFrameObserver::EnableJsApi(bool should_enable) {
 
 bool OverlayJsRenderFrameObserver::EnsureServiceConnected() {
   if (render_frame() && !contextual_search_js_api_service_) {
-    render_frame()->GetRemoteInterfaces()->GetInterface(
+    render_frame()->GetBrowserInterfaceBroker()->GetInterface(
         contextual_search_js_api_service_.BindNewPipeAndPassReceiver());
     return true;
   }