[Fuchsia] Update CastRunner to use ComponentContext to find services.

Previously CastRunner would look for Agent-provided services (e.g.
CastChannel) in the service namespace supplied on component launch.

If a ComponentContext is found in the service namespace passed to
StartComponent() then the CastRunner will now use that to connect to
the CastAgent to find Cast-specific services.

This requires temporary work-arounds e.g. to fall-back to looking at
the "additional services" provided in the component's StartupInfo,
until we migrate to the new component-manager APIs.

Bug: 920920, 893229
Change-Id: I43aec5c47bc24a0d2a537de621b20fad3342a5f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1472333
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638215}

base/fuchsia/startup_context.cc

@@ -26,7 +26,7 @@ StartupContext::StartupContext(::fuchsia::sys::StartupInfo startup_info)
     }
   }
 
-  // TODO(https://crbug.com/933834): Remove this workaround when we migrate to
+  // TODO(https://crbug.com/933834): Remove these workarounds when we migrate to
   // the new component manager.
   if (!incoming_services_ && startup_info_.launch_info.flat_namespace) {
     LOG(WARNING) << "Falling back to LaunchInfo namespace";
@@ -40,6 +40,28 @@ StartupContext::StartupContext(::fuchsia::sys::StartupInfo startup_info)
       }
     }
   }
+  if (!incoming_services_ && startup_info_.launch_info.additional_services) {
+    LOG(WARNING) << "Falling back to additional ServiceList services";
+
+    // Construct a ServiceDirectory and publish the additional services into it.
+    fidl::InterfaceHandle<::fuchsia::io::Directory> incoming_directory;
+    additional_services_.Bind(
+        std::move(startup_info_.launch_info.additional_services->provider));
+    additional_services_directory_ =
+        std::make_unique<base::fuchsia::ServiceDirectory>(
+            incoming_directory.NewRequest());
+    for (auto& name : startup_info_.launch_info.additional_services->names) {
+      additional_services_directory_->AddServiceUnsafe(
+          name, base::BindRepeating(
+                    &::fuchsia::sys::ServiceProvider::ConnectToService,
+                    base::Unretained(additional_services_.get()), name));
+    }
+
+    // Publish those services to the caller as |incoming_services_|.
+    incoming_services_ = std::make_unique<ServiceDirectoryClient>(
+        fidl::InterfaceHandle<::fuchsia::io::Directory>(
+            std::move(incoming_directory)));
+  }
 }
 
 StartupContext::~StartupContext() = default;

base/fuchsia/startup_context.h

@@ -28,6 +28,7 @@ class BASE_EXPORT StartupContext {
 
   // Returns the namespace of services published for use by the component.
   const ServiceDirectoryClient* incoming_services() const {
+    DCHECK(incoming_services_);
     return incoming_services_.get();
   }
 
@@ -43,6 +44,11 @@ class BASE_EXPORT StartupContext {
   std::unique_ptr<ServiceDirectoryClient> incoming_services_;
   std::unique_ptr<ServiceDirectory> public_services_;
 
+  // TODO(https://crbug.com/933834): Remove these when we migrate to the new
+  // component manager APIs.
+  ::fuchsia::sys::ServiceProviderPtr additional_services_;
+  std::unique_ptr<ServiceDirectory> additional_services_directory_;
+
   DISALLOW_COPY_AND_ASSIGN(StartupContext);
 };
 

fuchsia/base/fake_component_context.cc

@@ -9,15 +9,20 @@
 #include <string>
 #include <utility>
 
+#include "base/logging.h"
+
 namespace cr_fuchsia {
 
 FakeComponentContext::FakeComponentContext(
     AgentImpl::CreateComponentStateCallback create_component_state_callback,
     base::fuchsia::ServiceDirectory* service_directory,
-    std::string component_url)
-    : agent_impl_(service_directory,
+    base::StringPiece component_url)
+    : binding_(service_directory, this),
+      // Publishing the Agent to |service_directory| is not necessary, but
+      // also shouldn't do any harm.
+      agent_impl_(service_directory,
                   std::move(create_component_state_callback)),
-      component_url_(component_url) {}
+      component_url_(component_url.as_string()) {}
 
 void FakeComponentContext::ConnectToAgent(
     std::string agent_url,
@@ -26,6 +31,10 @@ void FakeComponentContext::ConnectToAgent(
   agent_impl_.Connect(component_url_, std::move(services));
 }
 
+void FakeComponentContext::NotImplemented_(const std::string& name) {
+  NOTIMPLEMENTED() << " API: " << name;
+}
+
 FakeComponentContext::~FakeComponentContext() = default;
 
 }  // namespace cr_fuchsia

fuchsia/base/fake_component_context.h

@@ -11,6 +11,8 @@
 #include <string>
 #include <utility>
 
+#include "base/strings/string_piece.h"
+
 namespace cr_fuchsia {
 
 // Used to test interactions with an Agent in unit-tests for a component.
@@ -26,17 +28,20 @@ class FakeComponentContext
   explicit FakeComponentContext(
       AgentImpl::CreateComponentStateCallback create_component_state_callback,
       base::fuchsia::ServiceDirectory* service_directory,
-      std::string component_url);
+      base::StringPiece component_url);
   ~FakeComponentContext() override;
 
-  // fuchsia::modular::ComponentContext implementation.
+  // fuchsia::modular::ComponentContext_TestBase implementation.
   void ConnectToAgent(
       std::string agent_url,
       fidl::InterfaceRequest<::fuchsia::sys::ServiceProvider> services,
       fidl::InterfaceRequest<fuchsia::modular::AgentController> controller)
       override;
+  void NotImplemented_(const std::string& name) override;
 
  private:
+  base::fuchsia::ScopedServiceBinding<fuchsia::modular::ComponentContext>
+      binding_;
   AgentImpl agent_impl_;
   const std::string component_url_;
 

fuchsia/runners/BUILD.gn

@@ -53,6 +53,7 @@ source_set("cast_runner_core") {
     "//base",
     "//fuchsia:web_fidl",
     "//fuchsia/base",
+    "//third_party/fuchsia-sdk/sdk:modular",
     "//url",
   ]
   public_deps = [

fuchsia/runners/cast/cast_channel_bindings.cc

@@ -36,7 +36,7 @@ CastChannelBindings::CastChannelBindings(
   DCHECK(frame_);
 
   channel_consumer_.set_error_handler([this](zx_status_t status) mutable {
-    ZX_LOG(ERROR, status) << " Agent disconnected.";
+    ZX_LOG(ERROR, status) << " Agent disconnected";
     std::move(on_error_closure_).Run();
   });
 

fuchsia/runners/cast/cast_component.cc

@@ -16,6 +16,8 @@
 
 namespace {
 constexpr int kBindingsFailureExitCode = 129;
+const char kCastAgentPackageUrl[] =
+    "fuchsia-pkg://fuchsia.com/cast_agent#meta/cast_agent.cmx";
 }  // namespace
 
 CastComponent::CastComponent(
@@ -31,11 +33,23 @@ CastComponent::CastComponent(
       navigation_observer_binding_(this) {
   base::AutoReset<bool> constructor_active_reset(&constructor_active_, true);
 
+  // Fetch the ComponentContext for this instance.
+  fuchsia::modular::ComponentContextPtr component_context;
+  startup_context()->incoming_services()->ConnectToService(
+      component_context.NewRequest());
+
+  // Request to connect to the Cast agent.
+  component_context->ConnectToAgent(kCastAgentPackageUrl,
+                                    agent_services_.NewRequest(),
+                                    agent_controller_.NewRequest());
+
+  // Bind to the CastChannel service provided by the Agent.
+  chromium::cast::CastChannelPtr cast_channel_ptr;
+  agent_services_->ConnectToService(
+      chromium::cast::CastChannel::Name_,
+      cast_channel_ptr.NewRequest().TakeChannel());
   cast_channel_ = std::make_unique<CastChannelBindings>(
-      frame(), &connector_,
-      startup_context()
-          ->incoming_services()
-          ->ConnectToService<chromium::cast::CastChannel>(),
+      frame(), &connector_, std::move(cast_channel_ptr),
       base::BindOnce(&CastComponent::DestroyComponent, base::Unretained(this),
                      kBindingsFailureExitCode,
                      fuchsia::sys::TerminationReason::INTERNAL_ERROR));

fuchsia/runners/cast/cast_component.h

@@ -5,6 +5,8 @@
 #ifndef FUCHSIA_RUNNERS_CAST_CAST_COMPONENT_H_
 #define FUCHSIA_RUNNERS_CAST_CAST_COMPONENT_H_
 
+#include <fuchsia/modular/cpp/fidl.h>
+#include <fuchsia/sys/cpp/fidl.h>
 #include <lib/fidl/cpp/binding.h>
 #include <memory>
 
@@ -43,6 +45,9 @@ class CastComponent : public WebComponent,
   std::unique_ptr<CastChannelBindings> cast_channel_;
   QueryableDataBindings queryable_data_;
 
+  fuchsia::sys::ServiceProviderPtr agent_services_;
+  fuchsia::modular::AgentControllerPtr agent_controller_;
+
   fidl::Binding<chromium::web::NavigationEventObserver>
       navigation_observer_binding_;
 

fuchsia/runners/cast/cast_runner_integration_test.cc

@@ -13,6 +13,8 @@
 #include "base/strings/stringprintf.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/test_timeouts.h"
+#include "fuchsia/base/agent_impl.h"
+#include "fuchsia/base/fake_component_context.h"
 #include "fuchsia/base/fit_adapter.h"
 #include "fuchsia/base/result_receiver.h"
 #include "fuchsia/engine/test/test_common.h"
@@ -38,37 +40,62 @@ void ComponentErrorHandler(zx_status_t status) {
 
 class FakeCastChannel : public chromium::cast::CastChannel {
  public:
-  FakeCastChannel() = default;
+  explicit FakeCastChannel(base::fuchsia::ServiceDirectory* directory)
+      : binding_(directory, this) {}
 
+  // Returns null if the Cast channel is not open.
+  const chromium::web::MessagePortPtr& port() const { return port_; }
+
+  void set_on_opened(base::OnceClosure on_opened) {
+    on_opened_ = std::move(on_opened);
+  }
+
+ protected:
   // chromium::web::CastChannel implementation.
   void OnOpened(fidl::InterfaceHandle<chromium::web::MessagePort> channel,
                 OnOpenedCallback callback_ignored) override {
-    // |callback_ignored| is dropped because these tests don't exercise multiple
-    // channel lifetimes.
-    connected_channel_ = channel.Bind();
+    port_ = channel.Bind();
+
+    if (on_opened_)
+      std::move(on_opened_).Run();
 
-    if (on_channel_connected_)
-      std::move(on_channel_connected_).Run();
+    callback_ignored();
   }
 
-  chromium::web::MessagePort* connected_channel() const {
-    // Return nullptr if |connected_channel_| is not currently bound.
-    return connected_channel_ ? connected_channel_.get() : nullptr;
+  const base::fuchsia::ScopedServiceBinding<chromium::cast::CastChannel>
+      binding_;
+
+  // Null until the Cast app connects to the Cast channel.
+  chromium::web::MessagePortPtr port_;
+
+  // Invoked when the contect opens a new Cast channel, if set.
+  base::OnceClosure on_opened_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeCastChannel);
+};
+
+class FakeComponentState : public cr_fuchsia::AgentImpl::ComponentStateBase {
+ public:
+  explicit FakeComponentState(base::StringPiece component_url)
+      : ComponentStateBase(component_url),
+        cast_channel_(std::make_unique<FakeCastChannel>(service_directory())) {}
+  ~FakeComponentState() override {
+    if (on_delete_)
+      std::move(on_delete_).Run();
   }
 
-  void set_on_channel_connected(base::OnceClosure on_channel_connected) {
-    on_channel_connected_ = std::move(on_channel_connected);
+  void set_on_delete(base::OnceClosure on_delete) {
+    on_delete_ = std::move(on_delete);
   }
 
- protected:
-  // The connected Cast Channel.
-  chromium::web::MessagePortPtr connected_channel_;
+  FakeCastChannel* cast_channel() { return cast_channel_.get(); }
+  void ClearCastChannel() { cast_channel_.reset(); }
 
-  // A pending on-connect callback, to be invoked once a Cast Channel is
-  // received.
-  base::OnceClosure on_channel_connected_;
+ protected:
+  std::unique_ptr<FakeCastChannel> cast_channel_;
+  base::OnceClosure on_delete_;
 
-  DISALLOW_COPY_AND_ASSIGN(FakeCastChannel);
+  DISALLOW_COPY_AND_ASSIGN(FakeComponentState);
 };
 
 }  // namespace
@@ -120,27 +147,28 @@ class CastRunnerIntegrationTest : public testing::Test {
   }
 
   void WaitUntilCastChannelOpened() {
-    if (cast_channel_.connected_channel())
+    if (component_state_->cast_channel()->port())
       return;
 
     base::RunLoop run_loop;
-    cast_channel_.set_on_channel_connected(run_loop.QuitClosure());
+    component_state_->cast_channel()->set_on_opened(run_loop.QuitClosure());
     run_loop.Run();
 
-    DCHECK(cast_channel_.connected_channel());
+    DCHECK(component_state_->cast_channel()->port());
   }
 
   fuchsia::sys::ComponentControllerPtr StartCastComponent(
       base::StringPiece component_url) {
-    DCHECK(!component_services_);
+    DCHECK(!component_state_);
 
-    // Create a ServiceDirectory and publish the CastChannel into it.
+    // Create a ServiceDirectory and publish the ComponentContext into it.
     fidl::InterfaceHandle<fuchsia::io::Directory> directory;
     component_services_ = std::make_unique<base::fuchsia::ServiceDirectory>(
         directory.NewRequest());
-    cast_channel_binding_ = std::make_unique<
-        base::fuchsia::ScopedServiceBinding<chromium::cast::CastChannel>>(
-        component_services_.get(), &cast_channel_);
+    component_context_ = std::make_unique<cr_fuchsia::FakeComponentContext>(
+        base::BindRepeating(&CastRunnerIntegrationTest::OnComponentConnect,
+                            base::Unretained(this)),
+        component_services_.get(), component_url);
 
     // Configure the Runner, including a service directory channel to publish
     // services to.
@@ -165,6 +193,13 @@ class CastRunnerIntegrationTest : public testing::Test {
   }
 
  protected:
+  std::unique_ptr<cr_fuchsia::AgentImpl::ComponentStateBase> OnComponentConnect(
+      base::StringPiece component_url) {
+    auto component_state = std::make_unique<FakeComponentState>(component_url);
+    component_state_ = component_state.get();
+    return component_state;
+  }
+
   const base::RunLoop::ScopedRunTimeoutForTest run_timeout_;
   base::MessageLoopForIO message_loop_;
   net::EmbeddedTestServer test_server_;
@@ -173,12 +208,10 @@ class CastRunnerIntegrationTest : public testing::Test {
   FakeApplicationConfigManager app_config_manager_;
   fidl::Binding<chromium::cast::ApplicationConfigManager> app_config_binding_;
 
-  // Incoming service directory, CastChannel and service bindings.
+  // Incoming service directory, ComponentContext and per-component state.
   std::unique_ptr<base::fuchsia::ServiceDirectory> component_services_;
-  FakeCastChannel cast_channel_;
-  std::unique_ptr<
-      base::fuchsia::ScopedServiceBinding<chromium::cast::CastChannel>>
-      cast_channel_binding_;
+  std::unique_ptr<cr_fuchsia::FakeComponentContext> component_context_;
+  FakeComponentState* component_state_ = nullptr;
 
   // ServiceDirectory into which the CastRunner will publish itself.
   std::unique_ptr<base::fuchsia::ServiceDirectory> public_services_;
@@ -230,18 +263,18 @@ TEST_F(CastRunnerIntegrationTest, BasicRequest) {
   }
 
   // Verify that the component is torn down when |component_controller| is
-  // unbound, by observing the destruction of its child Interfaces.
-  base::RunLoop destruction_run_loop;
-  nav_controller.set_error_handler(
-      [&destruction_run_loop](zx_status_t) { destruction_run_loop.Quit(); });
+  // unbound.
+  base::RunLoop run_loop;
+  component_state_->set_on_delete(run_loop.QuitClosure());
   component_controller.Unbind();
-  destruction_run_loop.Run();
+  run_loop.Run();
 }
 
 TEST_F(CastRunnerIntegrationTest, IncorrectCastAppId) {
   // Launch the a component with an invalid Cast app Id.
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent("cast:99999999");
+  component_controller.set_error_handler(&ComponentErrorHandler);
 
   // Run the loop until the ComponentController is dropped, or a WebComponent is
   // created.
@@ -302,17 +335,17 @@ TEST_F(CastRunnerIntegrationTest, CastChannel) {
     base::RunLoop run_loop;
     cr_fuchsia::ResultReceiver<chromium::web::WebMessage> message(
         run_loop.QuitClosure());
-    cast_channel_.connected_channel()->ReceiveMessage(
+    component_state_->cast_channel()->port()->ReceiveMessage(
         cr_fuchsia::CallbackToFitFunction(message.GetReceiveCallback()));
     run_loop.Run();
 
     EXPECT_EQ(cr_fuchsia::StringFromMemBufferOrDie(message->data), expected);
   }
 
-  // Shutdown the component and wait for the CastChannel to close.
-  component_controller.Unbind();
+  // Shutdown the component and wait for the teardown of its state.
   base::RunLoop run_loop;
-  cast_channel_binding_->SetOnLastClientCallback(run_loop.QuitClosure());
+  component_state_->set_on_delete(run_loop.QuitClosure());
+  component_controller.Unbind();
   run_loop.Run();
 }
 
@@ -326,15 +359,25 @@ TEST_F(CastRunnerIntegrationTest, CastChannelConsumerDropped) {
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent(base::StringPrintf("cast:%s", kCastChannelAppId));
 
-  // Expect that disconnecting the Cast Channel consumer service will trigger
-  // the destruction of the Cast Component.
-  cast_channel_binding_.reset();
+  // Spin the message loop to handle creation of the component state.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(component_state_);
+
+  // Disconnecting the CastChannel should trigger the component to teardown,
+  // resulting in our ComponentControllerPtr being dropped, and the component
+  // state held by the agent being deleted.
+  component_state_->set_on_delete(base::MakeExpectedRunClosure(FROM_HERE));
+  component_state_->ClearCastChannel();
   base::RunLoop run_loop;
-  component_controller.set_error_handler(
-      [&run_loop](zx_status_t) { run_loop.Quit(); });
+  component_controller.set_error_handler([&run_loop](zx_status_t status) {
+    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    run_loop.Quit();
+  });
   run_loop.Run();
-
   EXPECT_FALSE(component_controller.is_bound());
+
+  // Give the fake agent an opportunity to perform teardown cleanup.
+  base::RunLoop().RunUntilIdle();
 }
 
 TEST_F(CastRunnerIntegrationTest, CastChannelComponentControllerDropped) {
@@ -347,12 +390,15 @@ TEST_F(CastRunnerIntegrationTest, CastChannelComponentControllerDropped) {
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent(base::StringPrintf("cast:%s", kCastChannelAppId));
 
-  // Expect that disconnecting the ComponentController will kill the Cast
-  // Component, which we verify indirectly by listening for the disconnection
-  // of one of its CastChannel FIDL client.
-  component_controller.Unbind();
+  // Spin the message loop to handle creation of the component state.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(component_state_);
+
+  // Expect that disconnecting the ComponentController will destroy the Cast
+  // component.
   base::RunLoop run_loop;
-  cast_channel_binding_->SetOnLastClientCallback(run_loop.QuitClosure());
+  component_state_->set_on_delete(run_loop.QuitClosure());
+  component_controller.Unbind();
   run_loop.Run();
 }
 

fuchsia/runners/common/web_component.cc

@@ -49,7 +49,7 @@ WebComponent::WebComponent(
     controller_binding_.Bind(std::move(controller_request));
     controller_binding_.set_error_handler([this](zx_status_t status) {
       ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
-          << " ComponentController disconnected.";
+          << " ComponentController disconnected";
       // Teardown the component with dummy values, since ComponentController
       // channel isn't there to receive them.
       DestroyComponent(0, fuchsia::sys::TerminationReason::EXITED);

fuchsia/runners/common/web_content_runner.cc

@@ -94,7 +94,6 @@ void WebContentRunner::GetWebComponentForTest(
 }
 
 void WebContentRunner::DestroyComponent(WebComponent* component) {
-  LOG(ERROR) << "DestroyComponent " << components_.size();
   components_.erase(components_.find(component));
 
   if (components_.empty())

fuchsia/runners/web/web_runner_smoke_test.cc

@@ -61,6 +61,8 @@ class WebRunnerSmokeTest : public testing::Test {
   net::EmbeddedTestServer test_server_;
 
   base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebRunnerSmokeTest);
 };
 
 TEST_F(WebRunnerSmokeTest, RequestHtmlAndImage) {