[chromecast] Implement ApiBindings::Connect

- BindingsManagerCast implements ApiBindings::Connect.
- NamedMessagePortConnector will live in browser component
  (CastWebContents).
- Dependency on CastWebContents has been removed from
  BindingsManagerCast class.

Merge-With: eureka-internal/603702

Test: BindingsManagerCast, CastWebContentsBrowsertest
Bug: Internal b/183378843
Change-Id: I908611d06cfc085e36ba9ab53e1e1067c2893668
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2835080
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Commit-Queue: Jiawei Li <lijiawei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#892782}

chromecast/bindings/BUILD.gn

@@ -56,18 +56,14 @@ if (is_linux || is_chromeos || is_android) {
     sources = [
       "bindings_manager_cast.cc",
       "bindings_manager_cast.h",
-      "named_message_port_connector_cast.cc",
-      "named_message_port_connector_cast.h",
     ]
 
     deps = [
       ":bindings_manager",
       "//base",
       "//chromecast/bindings/public/mojom",
-      "//chromecast/browser:public",
       "//components/cast/api_bindings:manager",
       "//components/cast/message_port",
-      "//components/cast/named_message_port_connector",
       "//components/on_load_script_injector/browser",
       "//mojo/public/cpp/bindings",
       "//third_party/blink/public/common",
@@ -76,6 +72,7 @@ if (is_linux || is_chromeos || is_android) {
 
   source_set("browsertests_cast") {
     testonly = true
+
     sources = [ "bindings_manager_cast_browsertest.cc" ]
 
     defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]

chromecast/bindings/bindings_manager_cast.cc

@@ -4,31 +4,18 @@
 
 #include "chromecast/bindings/bindings_manager_cast.h"
 
+#include <memory>
 #include <utility>
 #include <vector>
 
-#include "base/logging.h"
+#include "base/bind.h"
 #include "base/macros.h"
-#include "base/notreached.h"
-#include "chromecast/bindings/named_message_port_connector_cast.h"
-#include "components/on_load_script_injector/browser/on_load_script_injector_host.h"
+#include "components/cast/message_port/message_port_cast.h"
 
 namespace chromecast {
 namespace bindings {
 
-BindingsManagerCast::BindingsManagerCast(
-    chromecast::CastWebContents* cast_web_contents)
-    : cast_web_contents_(cast_web_contents) {
-  DCHECK(cast_web_contents_);
-
-  CastWebContents::Observer::Observe(cast_web_contents_);
-
-  port_connector_ =
-      std::make_unique<NamedMessagePortConnectorCast>(cast_web_contents_, this);
-
-  port_connector_->RegisterPortHandler(base::BindRepeating(
-      &BindingsManagerCast::OnPortConnected, base::Unretained(this)));
-}
+BindingsManagerCast::BindingsManagerCast() = default;
 
 BindingsManagerCast::~BindingsManagerCast() = default;
 
@@ -52,30 +39,6 @@ void BindingsManagerCast::OnClientDisconnected() {
   receiver_.reset();
 }
 
-void BindingsManagerCast::OnPageStateChanged(
-    CastWebContents* cast_web_contents) {
-  // TODO(b/183378843): Remove usage of CWC in this class and
-  // move the CWC::Observer logics to other components, e.g.
-  // NMPC or |ApiBindingsClient|.
-  auto page_state = cast_web_contents->page_state();
-
-  switch (page_state) {
-    case CastWebContents::PageState::DESTROYED:
-    case CastWebContents::PageState::ERROR:
-      CastWebContents::Observer::Observe(nullptr);
-      cast_web_contents_ = nullptr;
-      port_connector_.reset();
-      break;
-    case CastWebContents::PageState::LOADED:
-      port_connector_->OnPageLoaded();
-      break;
-    case CastWebContents::PageState::IDLE:
-    case CastWebContents::PageState::LOADING:
-    case CastWebContents::PageState::CLOSED:
-      break;
-  }
-}
-
 void BindingsManagerCast::GetAll(GetAllCallback callback) {
   std::vector<chromecast::mojom::ApiBindingPtr> bindings_vector;
   for (const auto& bindings_name_and_script : bindings_) {
@@ -87,8 +50,8 @@ void BindingsManagerCast::GetAll(GetAllCallback callback) {
 
 void BindingsManagerCast::Connect(const std::string& port_name,
                                   blink::MessagePortDescriptor port) {
-  // TODO(b/183378843): Implements this method and migrate NMPC to use it.
-  NOTIMPLEMENTED_LOG_ONCE();
+  OnPortConnected(port_name,
+                  cast_api_bindings::MessagePortCast::Create(std::move(port)));
 }
 
 }  // namespace bindings

chromecast/bindings/bindings_manager_cast.h

@@ -5,23 +5,22 @@
 #ifndef CHROMECAST_BINDINGS_BINDINGS_MANAGER_CAST_H_
 #define CHROMECAST_BINDINGS_BINDINGS_MANAGER_CAST_H_
 
+#include "base/callback.h"
 #include "chromecast/bindings/bindings_manager.h"
 #include "chromecast/bindings/public/mojom/api_bindings.mojom.h"
-#include "chromecast/browser/cast_web_contents.h"
 #include "components/cast/api_bindings/manager.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 
 namespace chromecast {
 namespace bindings {
 
-class NamedMessagePortConnectorCast;
-
 // Implements the CastOS BindingsManager.
 class BindingsManagerCast : public BindingsManager,
-                            public CastWebContents::Observer,
                             public chromecast::mojom::ApiBindings {
  public:
-  explicit BindingsManagerCast(chromecast::CastWebContents* cast_web_contents);
+  // |cast_web_contents|: Used to inject bindings scripts into document early.
+  //                      Must outlive |this|.
+  BindingsManagerCast();
   ~BindingsManagerCast() override;
 
   BindingsManagerCast(const BindingsManagerCast&) = delete;
@@ -38,17 +37,11 @@ class BindingsManagerCast : public BindingsManager,
  private:
   void OnClientDisconnected();
 
-  // CastWebContents::Observer implementation.
-  void OnPageStateChanged(CastWebContents* cast_web_contents) override;
-
   // chromecast::mojom::ApiBindings implementation.
   void GetAll(GetAllCallback callback) override;
   void Connect(const std::string& port_name,
                blink::MessagePortDescriptor port) override;
 
-  chromecast::CastWebContents* cast_web_contents_;
-  std::unique_ptr<NamedMessagePortConnectorCast> port_connector_;
-
   // Stores all bindings, keyed on the string-based IDs provided by the
   // ApiBindings interface.
   std::map<std::string, std::string> bindings_;

chromecast/bindings/bindings_manager_cast_browsertest.cc

@@ -168,8 +168,7 @@ class BindingsManagerCastBrowserTest : public content::BrowserTestBase {
     cast_web_contents_ =
         std::make_unique<CastWebContentsImpl>(web_contents_.get(), init_params);
     title_change_observer_.Observe(cast_web_contents_.get());
-    bindings_manager_ = std::make_unique<bindings::BindingsManagerCast>(
-        cast_web_contents_.get());
+    bindings_manager_ = std::make_unique<bindings::BindingsManagerCast>();
     cast_web_contents_->ConnectToBindingsService(
         bindings_manager_->CreateRemote());
   }

chromecast/browser/BUILD.gn

@@ -75,6 +75,21 @@ cast_source_set("web_preferences") {
   ]
 }
 
+cast_source_set("named_message_port_connector_cast") {
+  sources = [
+    "named_message_port_connector_cast.cc",
+    "named_message_port_connector_cast.h",
+  ]
+
+  deps = [
+    "//base",
+    "//chromecast/browser:public",
+    "//components/cast/message_port",
+    "//components/cast/named_message_port_connector",
+    "//components/on_load_script_injector/browser",
+  ]
+}
+
 group("browser") {
   public_deps = [ ":browser_base" ]
 }
@@ -181,6 +196,7 @@ cast_source_set("browser_base") {
 
   deps = [
     ":browser_buildflags",
+    ":named_message_port_connector_cast",
     ":web_preferences",
     "//base",
     "//base:i18n",
@@ -216,6 +232,7 @@ cast_source_set("browser_base") {
     "//chromecast/net",
     "//chromecast/net:connectivity_checker",
     "//chromecast/service",
+    "//components/cast/message_port:message_port_cast",
     "//components/cdm/browser",
     "//components/download/public/common:public",
     "//components/media_control/browser",

chromecast/browser/DEPS

@@ -1,6 +1,7 @@
 include_rules = [
   "+cc/base/switches.h",
   "+chromecast/activity",
+  "+chromecast/bindings",
   "+chromecast/bindings/public/mojom",
   "+chromecast/common",
   "+chromecast/common/mojom",
@@ -14,6 +15,7 @@ include_rules = [
   "+chromecast/net",
   "+chromecast/service",
   "+chromecast/ui",
+  "+components/cast",
   "+components/cdm/browser",
   "+components/crash",
   "+components/download/public/common",

chromecast/browser/cast_web_contents.h

@@ -32,11 +32,6 @@ class NavigationHandle;
 class WebContents;
 }  // namespace content
 
-namespace on_load_script_injector {
-template <typename>
-class OnLoadScriptInjectorHost;
-}  // namespace on_load_script_injector
-
 namespace chromecast {
 
 struct RendererFeature {
@@ -241,6 +236,8 @@ class CastWebContents {
     // Whether WebRTC peer connections are allowed to use legacy versions of the
     // TLS/DTLS protocols.
     bool webrtc_allow_legacy_tls_protocols = false;
+    // Enable NamedMessagePortConnectorCast JS APIs. This is only meant to be
+    // modified by testing targets.
 
     InitParams();
     InitParams(const InitParams& other);
@@ -345,11 +342,6 @@ class CastWebContents {
   // Page Communication
   // ===========================================================================
 
-  // Returns the script injector instance, which injects scripts at page load
-  // time.
-  virtual on_load_script_injector::OnLoadScriptInjectorHost<uint64_t>*
-  script_injector() = 0;
-
   // Executes a UTF-8 encoded |script| for every subsequent page load where
   // the frame's URL has an origin reflected in |origins|. The script is
   // executed early, prior to the execution of the document's scripts.

chromecast/browser/cast_web_contents_impl.cc

@@ -24,6 +24,7 @@
 #include "chromecast/common/mojom/queryable_data_store.mojom.h"
 #include "chromecast/common/queryable_data.h"
 #include "chromecast/net/connectivity_checker.h"
+#include "components/cast/message_port/message_port_cast.h"
 #include "components/media_control/mojom/media_playback_options.mojom.h"
 #include "content/public/browser/message_port_provider.h"
 #include "content/public/browser/navigation_entry.h"
@@ -349,11 +350,6 @@ void CastWebContentsImpl::SetAppProperties(const std::string& session_id,
       web_contents_, session_id, is_audio_app);
 }
 
-on_load_script_injector::OnLoadScriptInjectorHost<uint64_t>*
-CastWebContentsImpl::script_injector() {
-  return &script_injector_;
-}
-
 void CastWebContentsImpl::AddBeforeLoadJavaScript(uint64_t id,
                                                   base::StringPiece script) {
   script_injector_.AddScriptForAllOrigins(id, std::string(script));
@@ -397,6 +393,11 @@ void CastWebContentsImpl::ConnectToBindingsService(
 
   bindings_received_ = false;
 
+  named_message_port_connector_ =
+      std::make_unique<NamedMessagePortConnectorCast>(this);
+  named_message_port_connector_->RegisterPortHandler(base::BindRepeating(
+      &CastWebContentsImpl::OnPortConnected, base::Unretained(this)));
+
   api_bindings_.Bind(std::move(api_bindings_remote));
   // Fetch bindings and inject scripts into |script_injector_|.
   api_bindings_->GetAll(base::BindOnce(&CastWebContentsImpl::OnBindingsReceived,
@@ -598,6 +599,19 @@ void CastWebContentsImpl::OnBindingsReceived(
   }
 }
 
+bool CastWebContentsImpl::OnPortConnected(
+    base::StringPiece port_name,
+    std::unique_ptr<cast_api_bindings::MessagePort> port) {
+  DCHECK(api_bindings_);
+
+  api_bindings_->Connect(
+      std::string(port_name),
+      cast_api_bindings::MessagePortCast::FromMessagePort(port.get())
+          ->TakePort()
+          .PassPort());
+  return true;
+}
+
 void CastWebContentsImpl::OnInterfaceRequestFromFrame(
     content::RenderFrameHost* /* render_frame_host */,
     const std::string& interface_name,

chromecast/browser/cast_web_contents_impl.h

@@ -22,6 +22,7 @@
 #include "chromecast/bindings/public/mojom/api_bindings.mojom.h"
 #include "chromecast/browser/cast_media_blocker.h"
 #include "chromecast/browser/cast_web_contents.h"
+#include "chromecast/browser/named_message_port_connector_cast.h"
 #include "components/on_load_script_injector/browser/on_load_script_injector_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/web_contents.h"
@@ -75,8 +76,6 @@ class CastWebContentsImpl : public CastWebContents,
   void BlockMediaLoading(bool blocked) override;
   void BlockMediaStarting(bool blocked) override;
   void EnableBackgroundVideoPlayback(bool enabled) override;
-  on_load_script_injector::OnLoadScriptInjectorHost<uint64_t>* script_injector()
-      override;
   void AddBeforeLoadJavaScript(uint64_t id, base::StringPiece script) override;
   void PostMessageToMainFrame(
       const std::string& target_origin,
@@ -154,6 +153,8 @@ class CastWebContentsImpl : public CastWebContents,
   std::vector<chromecast::shell::mojom::FeaturePtr> GetRendererFeatures();
   void OnBindingsReceived(
       std::vector<chromecast::mojom::ApiBindingPtr> bindings);
+  bool OnPortConnected(base::StringPiece port_name,
+                       std::unique_ptr<cast_api_bindings::MessagePort> port);
 
   content::WebContents* web_contents_;
   base::WeakPtr<Delegate> delegate_;
@@ -199,6 +200,9 @@ class CastWebContentsImpl : public CastWebContents,
   bool bindings_received_{false};
   GURL pending_load_url_;
 
+  // Used to open a MessageChannel for connecting API bindings.
+  std::unique_ptr<NamedMessagePortConnectorCast> named_message_port_connector_;
+
   base::ObserverList<Observer>::Unchecked observer_list_;
 
   service_manager::BinderRegistry binder_registry_;