Prepare Cast Shell for InterfaceProvider removal

Upstream Chromium no longer uses an InterfaceProvider to expose browser
interfaces to render frames. Cast Shell still does this for a small
handful of interfaces registered by downstream internal-only logic.

In order to support the transition of the internal repository away from
this mechanism, this CL introduces a bit of redundancy: interfaces
registered with CastWebContents through either of its two registration
mechanisms will now be exposed to *both* InterfaceProvider consumers as
well as BrowserInterfaceBroker.

Once this lands, internal renderer code can simply be rewritten to use
RenderFrame::GetBrowserInterfaceBroker instead of
RenderFrame::GetRemoteInterfaces(), and then upstream Chromium can
completely remove all its gnarly support for a browser-exposed
InterfaceProvider.

Bug: 977637
Change-Id: I21ae1fc31b35b0ca77ab2a00645feafe0ac67a73
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547054Reviewed-by: Sean Topping <seantopping@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#829858}

chromecast/browser/BUILD.gn

@@ -6,6 +6,7 @@ import("//build/buildflag_header.gni")
 import("//build/config/ui.gni")
 import("//chromecast/chromecast.gni")
 import("//media/media_options.gni")
+import("//mojo/public/tools/bindings/mojom.gni")
 import("//testing/test.gni")
 import("//tools/grit/grit_rule.gni")
 
@@ -540,6 +541,11 @@ cast_source_set("test_support") {
   ]
 }
 
+mojom("test_interfaces") {
+  testonly = true
+  sources = [ "test_interfaces.test-mojom" ]
+}
+
 cast_source_set("browsertests") {
   testonly = true
   sources = [
@@ -555,6 +561,7 @@ cast_source_set("browsertests") {
   defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
 
   deps = [
+    ":test_interfaces",
     ":test_support",
     "//base",
     "//chromecast:chromecast_buildflags",
@@ -567,6 +574,8 @@ cast_source_set("browsertests") {
     "//content/test:test_support",
     "//media:test_support",
     "//net:test_support",
+    "//services/service_manager/public/cpp",
+    "//services/service_manager/public/mojom",
   ]
 
   data = [

chromecast/browser/cast_browser_interface_binders.cc

@@ -54,6 +54,26 @@ void BindMediaRemotingRemotee(
       ::media::mojom::Remotee::Name_, &interface_pipe);
 }
 
+// Some Cast internals still dynamically set up interface binders after
+// frame host initialization. This is used to generically forward incoming
+// interface receivers to those objects until they can be reworked as static
+// registrations below.
+bool HandleGenericReceiver(content::RenderFrameHost* frame_host,
+                           mojo::GenericPendingReceiver& receiver) {
+  auto* web_contents = content::WebContents::FromRenderFrameHost(frame_host);
+  if (!web_contents)
+    return false;
+
+  // Only WebContents created for Cast Webviews will have a CastWebContents
+  // object associated with them. We ignore these requests for any other
+  // WebContents.
+  auto* cast_web_contents = CastWebContents::FromWebContents(web_contents);
+  if (!cast_web_contents || !cast_web_contents->can_bind_interfaces())
+    return false;
+
+  return cast_web_contents->TryBindReceiver(receiver);
+}
+
 }  // namespace
 
 void PopulateCastFrameBinders(
@@ -65,6 +85,9 @@ void PopulateCastFrameBinders(
       base::BindRepeating(&BindApplicationMediaCapabilities));
   binder_map->Add<::media::mojom::Remotee>(
       base::BindRepeating(&BindMediaRemotingRemotee));
+
+  binder_map->SetDefaultBinderDeprecated(
+      base::BindRepeating(&HandleGenericReceiver));
 }
 
 }  // namespace shell

chromecast/browser/cast_web_contents.h

@@ -16,6 +16,7 @@
 #include "base/strings/string16.h"
 #include "chromecast/common/mojom/feature_manager.mojom.h"
 #include "content/public/common/media_playback_renderer_type.mojom.h"
+#include "mojo/public/cpp/bindings/generic_pending_receiver.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/messaging/web_message_port.h"
@@ -380,6 +381,10 @@ class CastWebContents {
   // when it is ready.
   virtual service_manager::BinderRegistry* binder_registry() = 0;
 
+  // Asks the CastWebContents to bind an interface receiver using either its
+  // registry or any registered InterfaceProvider.
+  virtual bool TryBindReceiver(mojo::GenericPendingReceiver& receiver) = 0;
+
   // Used for owner to pass its |InterfaceProvider| pointers to CastWebContents.
   // It is owner's responsibility to make sure each |InterfaceProvider| pointer
   // has distinct mojo interface set.

chromecast/browser/cast_web_contents_browsertest.cc

@@ -19,11 +19,13 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "chromecast/base/chromecast_switches.h"
 #include "chromecast/base/metrics/cast_metrics_helper.h"
 #include "chromecast/browser/cast_browser_context.h"
 #include "chromecast/browser/cast_browser_process.h"
 #include "chromecast/browser/cast_web_contents_impl.h"
+#include "chromecast/browser/test_interfaces.test-mojom.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ -33,11 +35,16 @@
 #include "content/public/test/browser_test_base.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/url_loader_interceptor.h"
-#include "mojo/public/cpp/bindings/connector.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/http/http_status_code.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
+#include "services/service_manager/public/mojom/interface_provider.mojom.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -239,8 +246,9 @@ class CastWebContentsBrowserTest : public content::BrowserTestBase,
     SetUpCommandLine(base::CommandLine::ForCurrentProcess());
     BrowserTestBase::SetUp();
   }
-  void SetUpCommandLine(base::CommandLine* command_line) final {
+  void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitchASCII(switches::kTestType, "browser");
+    command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures, "MojoJS");
   }
   void PreRunTestOnMainThread() override {
     // Pump startup related events.
@@ -955,6 +963,147 @@ IN_PROC_BROWSER_TEST_F(CastWebContentsBrowserTest, ExecuteJavaScript) {
   run_loop2.Run();
 }
 
+// Helper for the test below. This exposes two interfaces, TestAdder and
+// TestDoubler. TestAdder is exposed only through a binder (see MakeAdderBinder)
+// which the test will register in the CastWebContents' binder_registry().
+// TestDoubler is exposed only through an InterfaceProvider, registered with the
+// CastWebContents using RegisterInterfaceProvider.
+class TestInterfaceProvider : public service_manager::mojom::InterfaceProvider,
+                              public mojom::TestAdder,
+                              public mojom::TestDoubler {
+ public:
+  TestInterfaceProvider()
+      : provider_(receiver_.BindNewPipeAndPassRemote(),
+                  base::SequencedTaskRunnerHandle::Get()) {}
+  ~TestInterfaceProvider() override = default;
+
+  size_t num_adders() const { return adders_.size(); }
+  size_t num_doublers() const { return doublers_.size(); }
+
+  service_manager::InterfaceProvider* interface_provider() {
+    return &provider_;
+  }
+
+  base::RepeatingCallback<void(mojo::PendingReceiver<mojom::TestAdder>)>
+  MakeAdderBinder() {
+    return base::BindLambdaForTesting(
+        [this](mojo::PendingReceiver<mojom::TestAdder> receiver) {
+          adders_.Add(this, std::move(receiver));
+          OnRequestHandled();
+        });
+  }
+
+  // Waits for some number of new interface binding requests to be dispatched
+  // and then invokes `callback`.
+  void WaitForRequests(size_t n, base::OnceClosure callback) {
+    wait_callback_ = std::move(callback);
+    num_requests_to_wait_for_ = n;
+  }
+
+  // service_manager::mojom::InterfaceProvider:
+  void GetInterface(const std::string& interface_name,
+                    mojo::ScopedMessagePipeHandle interface_pipe) override {
+    if (interface_name == mojom::TestDoubler::Name_) {
+      doublers_.Add(this, mojo::PendingReceiver<mojom::TestDoubler>(
+                              std::move(interface_pipe)));
+      OnRequestHandled();
+    }
+  }
+
+  // mojom::TestAdder:
+  void Add(int32_t a, int32_t b, AddCallback callback) override {
+    std::move(callback).Run(a + b);
+  }
+
+  // mojom::TestDouble:
+  void Double(int32_t x, DoubleCallback callback) override {
+    std::move(callback).Run(x * 2);
+  }
+
+ private:
+  void OnRequestHandled() {
+    if (num_requests_to_wait_for_ == 0)
+      return;
+    DCHECK(wait_callback_);
+    if (--num_requests_to_wait_for_ == 0)
+      std::move(wait_callback_).Run();
+  }
+
+  mojo::Receiver<service_manager::mojom::InterfaceProvider> receiver_{this};
+  service_manager::InterfaceProvider provider_;
+  mojo::ReceiverSet<mojom::TestAdder> adders_;
+  mojo::ReceiverSet<mojom::TestDoubler> doublers_;
+  size_t num_requests_to_wait_for_ = 0;
+  base::OnceClosure wait_callback_;
+};
+
+IN_PROC_BROWSER_TEST_F(CastWebContentsBrowserTest, InterfaceBinding) {
+  // This test verifies that interfaces registered with the CastWebContents --
+  // either via its binder_registry() or its RegisterInterfaceProvider() API --
+  // are reachable from render frames using either the deprecated
+  // InterfaceProvider API (which results in an OnInterfaceRequestFromFrame call
+  // on the WebContents) or the newer BrowserInterfaceBroker API which is used
+  // in most other places (including from Mojo JS).
+  TestInterfaceProvider provider;
+  cast_web_contents_->binder_registry()->AddInterface(
+      provider.MakeAdderBinder());
+  cast_web_contents_->RegisterInterfaceProvider(
+      CastWebContents::InterfaceSet{mojom::TestDoubler::Name_},
+      provider.interface_provider());
+
+  // First verify that both interfaces are reachable using the deprecated
+  // WebContents path, which is triggered only by renderer-side use of
+  // RenderFrame::GetRemoteInterfaces(). Since poking renderer state in browser
+  // tests is challenging, we simply simulate the resulting WebContentsObbserver
+  // calls here instead and verify end-to-end connection for each interface.
+  content::RenderFrameHost* main_frame =
+      cast_web_contents_->web_contents()->GetMainFrame();
+  mojo::Remote<mojom::TestAdder> adder;
+  mojo::ScopedMessagePipeHandle adder_receiver_pipe =
+      adder.BindNewPipeAndPassReceiver().PassPipe();
+  cast_web_contents_->OnInterfaceRequestFromFrame(
+      main_frame, mojom::TestAdder::Name_, &adder_receiver_pipe);
+  mojo::Remote<mojom::TestDoubler> doubler;
+  mojo::ScopedMessagePipeHandle doubler_receiver_pipe =
+      doubler.BindNewPipeAndPassReceiver().PassPipe();
+  cast_web_contents_->OnInterfaceRequestFromFrame(
+      main_frame, mojom::TestDoubler::Name_, &doubler_receiver_pipe);
+
+  base::RunLoop add_loop;
+  adder->Add(37, 5, base::BindLambdaForTesting([&](int32_t result) {
+               EXPECT_EQ(42, result);
+               add_loop.Quit();
+             }));
+  add_loop.Run();
+
+  base::RunLoop double_loop;
+  doubler->Double(21, base::BindLambdaForTesting([&](int32_t result) {
+                    EXPECT_EQ(42, result);
+                    double_loop.Quit();
+                  }));
+  double_loop.Run();
+
+  EXPECT_EQ(1u, provider.num_adders());
+  EXPECT_EQ(1u, provider.num_doublers());
+
+  // Now verify that the same interfaces are also reachable at the same binders
+  // when going through the newer BrowserInterfaceBroker path. For simplicity
+  // the test JS here does not have access to bindings and so does not make
+  // calls on the interfaces. It is however totally sufficient for us to verify
+  // that the page's requests result in new receivers being bound inside
+  // TestInterfaceProvider.
+  base::RunLoop loop;
+  provider.WaitForRequests(2, loop.QuitClosure());
+  embedded_test_server()->ServeFilesFromSourceDirectory(GetTestDataPath());
+  StartTestServer();
+  const GURL kUrl{embedded_test_server()->GetURL("/interface_binding.html")};
+  cast_web_contents_->LoadUrl(kUrl);
+  loop.Run();
+
+  EXPECT_EQ(2u, provider.num_adders());
+  EXPECT_EQ(2u, provider.num_doublers());
+}
+
 }  // namespace chromecast
 
 #endif  // CHROMECAST_BROWSER_CAST_WEB_CONTENTS_BROWSERTEST_H_

chromecast/browser/cast_web_contents_impl.cc

@@ -397,6 +397,31 @@ service_manager::BinderRegistry* CastWebContentsImpl::binder_registry() {
   return &binder_registry_;
 }
 
+bool CastWebContentsImpl::TryBindReceiver(
+    mojo::GenericPendingReceiver& receiver) {
+  const std::string interface_name = *receiver.interface_name();
+  mojo::ScopedMessagePipeHandle interface_pipe = receiver.PassPipe();
+  if (binder_registry_.TryBindInterface(interface_name, &interface_pipe)) {
+    return true;
+  }
+
+  for (auto& entry : interface_providers_map_) {
+    auto const& interface_set = entry.first;
+    // Interface is provided by this InterfaceProvider.
+    if (interface_set.find(interface_name) != interface_set.end()) {
+      auto* interface_provider = entry.second;
+      interface_provider->GetInterfaceByName(interface_name,
+                                             std::move(interface_pipe));
+      return true;
+    }
+  }
+
+  // Unsuccessful, so give the caller its receiver back.
+  receiver =
+      mojo::GenericPendingReceiver(interface_name, std::move(interface_pipe));
+  return false;
+}
+
 void CastWebContentsImpl::RegisterInterfaceProvider(
     const InterfaceSet& interface_set,
     service_manager::InterfaceProvider* interface_provider) {
@@ -504,18 +529,12 @@ void CastWebContentsImpl::OnInterfaceRequestFromFrame(
   if (!can_bind_interfaces()) {
     return;
   }
-  if (binder_registry_.TryBindInterface(interface_name, interface_pipe)) {
-    return;
-  }
-  for (auto& entry : interface_providers_map_) {
-    auto const& interface_set = entry.first;
-    // Interface is provided by this InterfaceProvider.
-    if (interface_set.find(interface_name) != interface_set.end()) {
-      auto* interface_provider = entry.second;
-      interface_provider->GetInterfaceByName(interface_name,
-                                             std::move(*interface_pipe));
-      break;
-    }
+
+  mojo::GenericPendingReceiver receiver(interface_name,
+                                        std::move(*interface_pipe));
+  if (!TryBindReceiver(receiver)) {
+    // If binding was unsuccessful, give the caller its pipe back.
+    *interface_pipe = receiver.PassPipe();
   }
 }
 

chromecast/browser/cast_web_contents_impl.h

@@ -63,6 +63,7 @@ class CastWebContentsImpl : public CastWebContents,
       const InterfaceSet& interface_set,
       service_manager::InterfaceProvider* interface_provider) override;
   service_manager::BinderRegistry* binder_registry() override;
+  bool TryBindReceiver(mojo::GenericPendingReceiver& receiver) override;
   void BlockMediaLoading(bool blocked) override;
   void BlockMediaStarting(bool blocked) override;
   void EnableBackgroundVideoPlayback(bool enabled) override;

chromecast/browser/test/data/interface_binding.html

+<html>
+    <head><title>Binding some Mojo interfaces</title></head>
+    <body>
+        <script>
+          function bindInterface(name) {
+            const {handle0, handle1} = Mojo.createMessagePipe();
+            Mojo.bindInterface(name, handle0);
+          }
+
+          // The browser test which uses this page will succeed once these
+          // bindInterface requests reach browser-side binders registered by
+          // the test.
+          bindInterface('chromecast.mojom.TestAdder');
+          bindInterface('chromecast.mojom.TestDoubler');
+        </script>
+    </body>
+</html>

chromecast/browser/test/mock_cast_web_view.cc

@@ -13,6 +13,10 @@ service_manager::BinderRegistry* MockCastWebContents::binder_registry() {
   return &registry_;
 }
 
+bool MockCastWebContents::TryBindReceiver(mojo::GenericPendingReceiver&) {
+  return false;
+}
+
 MockCastWebView::MockCastWebView() {
   mock_cast_web_contents_ = std::make_unique<MockCastWebContents>();
 }

chromecast/browser/test/mock_cast_web_view.h

@@ -64,6 +64,7 @@ class MockCastWebContents : public CastWebContents {
   MOCK_METHOD(bool, can_bind_interfaces, (), (override));
 
   service_manager::BinderRegistry* binder_registry() override;
+  bool TryBindReceiver(mojo::GenericPendingReceiver&) override;
 
  private:
   service_manager::BinderRegistry registry_;

chromecast/browser/test_interfaces.test-mojom

+// Copyright 2020 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.
+
+module chromecast.mojom;
+
+interface TestAdder {
+  Add(int32 a, int32 b) => (int32 result);
+};
+
+interface TestDoubler {
+  Double(int32 x) => (int32 result);
+};

mojo/public/cpp/bindings/binder_map.h

@@ -100,12 +100,24 @@ class BinderMapWithContext {
                   "ContextType is void.");
     auto it = binders_.find(*receiver->interface_name());
     if (it == binders_.end())
-      return false;
+      return default_binder_ && default_binder_.Run(context, *receiver);
 
     it->second->BindInterface(std::move(context), receiver->PassPipe());
     return true;
   }
 
+  // DO NOT USE. This sets a generic default handler for any receiver that
+  // doesn't match a registered binder. It's a transitional API to help migrate
+  // some older code to BinderMap. Reliance on this mechanism makes security
+  // auditing more difficult. Note that this intentionally only supports use
+  // with a non-void ContextType, since that's the only existing use case.
+  using DefaultBinder =
+      base::RepeatingCallback<bool(ContextValueType context,
+                                   mojo::GenericPendingReceiver&)>;
+  void SetDefaultBinderDeprecated(DefaultBinder binder) {
+    default_binder_ = std::move(binder);
+  }
+
  private:
   using IsVoidContext = std::is_same<ContextType, void>;
 
@@ -113,6 +125,7 @@ class BinderMapWithContext {
       std::string,
       std::unique_ptr<internal::GenericCallbackBinderWithContext<ContextType>>>
       binders_;
+  DefaultBinder default_binder_;
 };
 
 // Common alias for BinderMapWithContext that has no context. Binders added to