[Fuchsia] Cleanup cast_runner integration tests.

Removed test-only methods from CastRunner, WebContentRunner and
CastComponent that were previously used in
cast_runner_integration_tests.

1. Removed CastRunner and WebContentRunner constructors that take
web.Context. Now context is always created in WebContentRunner.
2. Removed test-only methods.
3. Updated TestComponentContext to allow initialization with all
services cloned from /svc.

The tests now inject a script into the frame in order to test
functionality that was previously tested with the test-only methods.

Bug: 1062351
Change-Id: I169651b68e0e9344d3450371db1485d9046c5c16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135154
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757571}

base/fuchsia/test_component_context_for_process.cc

@@ -9,6 +9,7 @@
 #include <lib/fidl/cpp/interface_handle.h>
 #include <lib/sys/cpp/component_context.h>
 
+#include "base/files/file_enumerator.h"
 #include "base/fuchsia/default_context.h"
 #include "base/fuchsia/filtered_service_directory.h"
 #include "base/fuchsia/fuchsia_logging.h"
@@ -16,7 +17,8 @@
 
 namespace base {
 
-TestComponentContextForProcess::TestComponentContextForProcess() {
+TestComponentContextForProcess::TestComponentContextForProcess(
+    InitialState initial_state) {
   // TODO(https://crbug.com/1038786): Migrate to sys::ComponentContextProvider
   // once it provides access to an sys::OutgoingDirectory or PseudoDir through
   // which to publish additional_services().
@@ -26,6 +28,15 @@ TestComponentContextForProcess::TestComponentContextForProcess() {
   context_services_ = std::make_unique<fuchsia::FilteredServiceDirectory>(
       base::fuchsia::ComponentContextForCurrentProcess()->svc().get());
 
+  // Push all services from /svc to the test context if requested.
+  if (initial_state == InitialState::kCloneAll) {
+    base::FileEnumerator file_enum(base::FilePath("/svc"), false,
+                                   base::FileEnumerator::FILES);
+    for (auto file = file_enum.Next(); !file.empty(); file = file_enum.Next()) {
+      AddService(file.BaseName().value());
+    }
+  }
+
   // Create a ServiceDirectory backed by the contents of |incoming_directory|.
   fidl::InterfaceHandle<::fuchsia::io::Directory> incoming_directory;
   context_services_->ConnectClient(incoming_directory.NewRequest());
@@ -60,10 +71,15 @@ sys::OutgoingDirectory* TestComponentContextForProcess::additional_services() {
   return context_services_->outgoing_directory();
 }
 
+void TestComponentContextForProcess::AddService(
+    const base::StringPiece service) {
+  context_services_->AddService(service);
+}
+
 void TestComponentContextForProcess::AddServices(
     base::span<const base::StringPiece> services) {
   for (auto service : services)
-    context_services_->AddService(service);
+    AddService(service);
 }
 
 }  // namespace base

base/fuchsia/test_component_context_for_process.h

@@ -41,18 +41,25 @@ class FilteredServiceDirectory;
 // test base-class:
 //
 //   TEST(MyFunkyTest, IsFunky) {
-//     TestComponentContextForTest test_context;
+//     TestComponentContextForProcess test_context;
 //     // Configure the |test_context|.
 //     // Run tests of code that uses ComponentContextForProcess().
 //   }
 //
-// Services from the original process-global ComponentContext (usually the
-// environment in which the test process is running), can be exposed through the
-// |test_context| with AddServices(), during test setup:
+// By default created context doesn't expose any services. Services from the
+// original process-global ComponentContext (usually the environment in which
+// the test process is running), can be exposed through the |test_context| with
+// AddServices(), during test setup:
 //
 //   test_context.AddServices({fuchsia::memorypressure::Provider::Name_, ...});
 //   // ... Execute tests which use fuchsia.memorypressure.Provider ...
 //
+// Alternatively InitialState::kEmpty can be passed to the constructor to expose
+// all services listed in /svc, e.g.:
+//
+//   TestComponentContextForProcess test_context(
+//       TestComponentContextForProcess::InitialState::kEmpty);
+//
 // Fake/mock implementations can be exposed via additional_services():
 //
 //   ScopedServiceBinding<funky::Service> binding(
@@ -68,7 +75,13 @@ class FilteredServiceDirectory;
 //
 class BASE_EXPORT TestComponentContextForProcess {
  public:
-  TestComponentContextForProcess();
+  enum class InitialState {
+    kEmpty,
+    kCloneAll,
+  };
+
+  TestComponentContextForProcess(
+      InitialState initial_state = InitialState::kEmpty);
   ~TestComponentContextForProcess();
 
   TestComponentContextForProcess(const TestComponentContextForProcess&) =
@@ -80,8 +93,9 @@ class BASE_EXPORT TestComponentContextForProcess {
   // published for use by the code-under test.
   sys::OutgoingDirectory* additional_services();
 
-  // Allows the specified services from the original ComponentContext to be
+  // Allows the specified service(s) from the original ComponentContext to be
   // exposed via the test default ComponentContext.
+  void AddService(const base::StringPiece service);
   void AddServices(base::span<const base::StringPiece> services);
 
   // Returns the directory of services that the code under test has published

fuchsia/base/context_provider_test_connector.cc

@@ -20,7 +20,7 @@
 
 namespace cr_fuchsia {
 
-fuchsia::web::ContextProviderPtr ConnectContextProvider(
+fidl::InterfaceHandle<fuchsia::io::Directory> StartWebEngineForTests(
     fidl::InterfaceRequest<fuchsia::sys::ComponentController>
         component_controller_request,
     const base::CommandLine& command_line) {
@@ -49,12 +49,16 @@ fuchsia::web::ContextProviderPtr ConnectContextProvider(
   launcher->CreateComponent(std::move(launch_info),
                             std::move(component_controller_request));
 
-  sys::ServiceDirectory web_engine_service_dir(
-      std::move(web_engine_services_dir));
+  return web_engine_services_dir;
+}
 
-  fuchsia::web::ContextProviderPtr context_provider;
-  web_engine_service_dir.Connect(context_provider.NewRequest());
-  return context_provider;
+fuchsia::web::ContextProviderPtr ConnectContextProvider(
+    fidl::InterfaceRequest<fuchsia::sys::ComponentController>
+        component_controller_request,
+    const base::CommandLine& command_line) {
+  sys::ServiceDirectory web_engine_service_dir(StartWebEngineForTests(
+      std::move(component_controller_request), command_line));
+  return web_engine_service_dir.Connect<fuchsia::web::ContextProvider>();
 }
 
 }  // namespace cr_fuchsia

fuchsia/base/context_provider_test_connector.h

@@ -8,11 +8,18 @@
 #include <fuchsia/sys/cpp/fidl.h>
 #include <fuchsia/web/cpp/fidl.h>
 #include <lib/fidl/cpp/interface_request.h>
+#include <lib/sys/cpp/service_directory.h>
 
 #include "base/command_line.h"
 
 namespace cr_fuchsia {
 
+fidl::InterfaceHandle<fuchsia::io::Directory> StartWebEngineForTests(
+    fidl::InterfaceRequest<fuchsia::sys::ComponentController>
+        component_controller_request,
+    const base::CommandLine& command_line =
+        base::CommandLine(base::CommandLine::NO_PROGRAM));
+
 // TODO(crbug.com/1046615): Use test manifests for package specification.
 fuchsia::web::ContextProviderPtr ConnectContextProvider(
     fidl::InterfaceRequest<fuchsia::sys::ComponentController>

fuchsia/runners/cast/cast_component.cc

@@ -156,7 +156,4 @@ void CastComponent::OnZxHandleSignalled(zx_handle_t handle,
   DCHECK(runner()->is_headless());
 
   frame()->DisableHeadlessRendering();
-
-  if (on_headless_disconnect_cb_)
-    std::move(on_headless_disconnect_cb_).Run();
 }

fuchsia/runners/cast/cast_component.h

@@ -11,7 +11,6 @@
 #include <vector>
 
 #include "base/fuchsia/startup_context.h"
-#include "base/gtest_prod_util.h"
 #include "base/message_loop/message_pump_for_io.h"
 #include "base/message_loop/message_pump_fuchsia.h"
 #include "base/optional.h"
@@ -58,13 +57,6 @@ class CastComponent : public WebComponent,
   // WebComponent overrides.
   void StartComponent() final;
 
-  // Sets a callback that will be invoked when the handle controlling the
-  // lifetime of a headless "view" is dropped.
-  void set_on_headless_disconnect_for_test(
-      base::OnceClosure on_headless_disconnect_cb) {
-    on_headless_disconnect_cb_ = std::move(on_headless_disconnect_cb);
-  }
-
   const chromium::cast::ApplicationConfig& application_config() {
     return application_config_;
   }
@@ -74,8 +66,6 @@ class CastComponent : public WebComponent,
   CastRunner* runner() const;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(HeadlessCastRunnerIntegrationTest, Headless);
-
   void OnRewriteRulesReceived(
       std::vector<fuchsia::web::UrlRequestRewriteRule> rewrite_rules);
 
@@ -113,8 +103,6 @@ class CastComponent : public WebComponent,
   zx::eventpair headless_view_token_;
   base::MessagePumpForIO::ZxHandleWatchController headless_disconnect_watch_;
 
-  base::OnceClosure on_headless_disconnect_cb_;
-
   fidl::Binding<fuchsia::web::NavigationEventListener>
       navigation_listener_binding_;
 

fuchsia/runners/cast/cast_runner.cc

@@ -117,24 +117,13 @@ const char CastRunner::kAgentComponentUrl[] =
 
 CastRunner::CastRunner(
     WebContentRunner::GetContextParamsCallback get_context_params_callback,
-    bool is_headless,
-    sys::OutgoingDirectory* outgoing_directory)
+    bool is_headless)
     : WebContentRunner(base::BindRepeating(&CastRunner::GetMainContextParams,
-                                           base::Unretained(this)),
-                       outgoing_directory),
+                                           base::Unretained(this))),
       get_context_params_callback_(std::move(get_context_params_callback)),
       is_headless_(is_headless),
-      common_create_context_params_(BuildCreateContextParamsForIsolatedRunners(
-          get_context_params_callback_.Run())),
       service_directory_(CreateServiceDirectory()) {}
 
-CastRunner::CastRunner(OnDestructionCallback on_destruction_callback,
-                       fuchsia::web::ContextPtr context,
-                       bool is_headless)
-    : WebContentRunner(std::move(context)),
-      is_headless_(is_headless),
-      on_destruction_callback_(std::move(on_destruction_callback)) {}
-
 CastRunner::~CastRunner() = default;
 
 void CastRunner::StartComponent(
@@ -212,10 +201,6 @@ void CastRunner::StartComponent(
   pending_components_.emplace(std::move(pending_component_params));
 }
 
-size_t CastRunner::GetChildCastRunnerCountForTest() {
-  return isolated_runners_.size();
-}
-
 void CastRunner::DestroyComponent(WebComponent* component) {
   WebContentRunner::DestroyComponent(component);
 
@@ -370,16 +355,12 @@ void CastRunner::CreateAndRegisterCastComponent(
 
 CastRunner* CastRunner::CreateChildRunnerForIsolatedComponent(
     CastComponent::CastComponentParams* component_params) {
-  // Construct the CreateContextParams in order to create a new Context.
-  // Some common parameters must be inherited from
-  // |common_create_context_params_|.
-  fuchsia::web::CreateContextParams isolated_context_params;
-  zx_status_t status =
-      common_create_context_params_.Clone(&isolated_context_params);
-  if (status != ZX_OK) {
-    ZX_LOG(ERROR, status) << "clone";
-    return nullptr;
-  }
+  // Construct the CreateContextParams in order to create a new Context.  Some
+  // common parameters must be inherited from default params returned from
+  // |get_context_params_callback_|.
+  fuchsia::web::CreateContextParams isolated_context_params =
+      BuildCreateContextParamsForIsolatedRunners(
+          get_context_params_callback_.Run());
 
   // Service redirection is not necessary for isolated context. Pass default
   // /svc as is, without overriding any services.
@@ -391,17 +372,16 @@ CastRunner* CastRunner::CreateChildRunnerForIsolatedComponent(
       std::move(*component_params->app_config
                      .mutable_content_directories_for_isolated_application()));
 
-  std::unique_ptr<CastRunner> cast_runner(new CastRunner(
-      base::BindOnce(&CastRunner::OnChildRunnerDestroyed,
-                     base::Unretained(this)),
-      CreateWebContext(std::move(isolated_context_params)), is_headless()));
+  auto create_context_params_callback = base::BindRepeating(
+      [](fuchsia::web::CreateContextParams isolated_context_params) {
+        return isolated_context_params;
+      },
+      base::Passed(std::move(isolated_context_params)));
 
-  // If test code is listening for Component creation events, then wire up the
-  // isolated CastRunner to signal component creation events.
-  if (web_component_created_callback_for_test()) {
-    cast_runner->SetWebComponentCreatedCallbackForTest(
-        web_component_created_callback_for_test());
-  }
+  auto cast_runner = std::make_unique<CastRunner>(
+      std::move(create_context_params_callback), is_headless());
+  cast_runner->on_destruction_callback_ = base::BindOnce(
+      &CastRunner::OnChildRunnerDestroyed, base::Unretained(this));
 
   CastRunner* cast_runner_ptr = cast_runner.get();
   isolated_runners_.insert(std::move(cast_runner));

fuchsia/runners/cast/cast_runner.h

@@ -29,16 +29,17 @@ class FilteredServiceDirectory;
 // sys::Runner which instantiates Cast activities specified via cast/casts URIs.
 class CastRunner : public WebContentRunner {
  public:
+  using OnDestructionCallback = base::OnceCallback<void(CastRunner*)>;
+
+  static constexpr uint16_t kRemoteDebuggingPort = 9222;
+
   // Creates a Runner for Cast components and publishes it into the specified
   // OutgoingDirectory.
   // |get_context_params_callback|: Returns the context parameters to use.
   // |is_headless|: True if |get_context_params_callback| sets the HEADLESS
   //     feature flag.
-  // |outgoing_directory|: The directory that this CastRunner will publish
-  //     itself to.
   CastRunner(GetContextParamsCallback get_context_params_callback,
-             bool is_headless,
-             sys::OutgoingDirectory* outgoing_directory);
+             bool is_headless);
   ~CastRunner() override;
 
   CastRunner(const CastRunner&) = delete;
@@ -59,20 +60,7 @@ class CastRunner : public WebContentRunner {
   // Returns true if this Runner is configured not to use Scenic.
   bool is_headless() const { return is_headless_; }
 
-  // Returns the number of active CastRunner instances.
-  size_t GetChildCastRunnerCountForTest();
-
  private:
-  using OnDestructionCallback = base::OnceCallback<void(CastRunner*)>;
-
-  // Constructor used for creating CastRunners that run apps in dedicated
-  // Contexts. Child CastRunners may only spawn one Component and will be
-  // destroyed by their parents when their singleton Components are destroyed.
-  // |on_destruction_callback| is invoked when the child component is destroyed.
-  CastRunner(OnDestructionCallback on_destruction_callback,
-             fuchsia::web::ContextPtr context,
-             bool is_headless);
-
   // Creates and returns the service directory that is passed to the main web
   // context.
   std::unique_ptr<base::fuchsia::FilteredServiceDirectory>
@@ -116,9 +104,6 @@ class CastRunner : public WebContentRunner {
                  base::UniquePtrComparator>
       pending_components_;
 
-  // Used as a template for creating the ContextPtrs of isolated Runners.
-  fuchsia::web::CreateContextParams common_create_context_params_;
-
   // Invoked upon destruction of "isolated" runners, used to signal termination
   // to parents.
   OnDestructionCallback on_destruction_callback_;

fuchsia/runners/cast/main.cc

@@ -63,8 +63,8 @@ fuchsia::web::CreateContextParams CreateMainContextParams() {
   create_context_params.set_user_agent_product("CrKey");
   create_context_params.set_user_agent_version("1.43");
 
-  const uint16_t kRemoteDebuggingPort = 9222;
-  create_context_params.set_remote_debugging_port(kRemoteDebuggingPort);
+  create_context_params.set_remote_debugging_port(
+      CastRunner::kRemoteDebuggingPort);
 
   // TODO(crbug.com/1023514): Remove this switch when it is no longer
   // necessary.
@@ -90,8 +90,8 @@ int main(int argc, char** argv) {
   WebContentRunner::GetContextParamsCallback get_context_params_callback =
       base::BindRepeating(&CreateMainContextParams);
 
-  CastRunner runner(
-      std::move(get_context_params_callback), IsHeadless(),
+  CastRunner runner(std::move(get_context_params_callback), IsHeadless());
+  runner.PublishRunnerService(
       base::fuchsia::ComponentContextForCurrentProcess()->outgoing().get());
 
   base::fuchsia::ComponentContextForCurrentProcess()

fuchsia/runners/cast/test_api_bindings.cc

@@ -4,6 +4,7 @@
 
 #include "fuchsia/runners/cast/test_api_bindings.h"
 
+#include "base/fuchsia/fuchsia_logging.h"
 #include "base/run_loop.h"
 
 TestApiBindings::TestApiBindings() = default;
@@ -25,7 +26,14 @@ TestApiBindings::RunUntilMessagePortReceived(base::StringPiece port_name) {
 }
 
 void TestApiBindings::GetAll(GetAllCallback callback) {
-  callback(std::move(bindings_));
+  std::vector<chromium::cast::ApiBinding> bindings_clone;
+  for (auto& binding : bindings_) {
+    chromium::cast::ApiBinding binding_clone;
+    zx_status_t status = binding.Clone(&binding_clone);
+    ZX_CHECK(status == ZX_OK, status);
+    bindings_clone.push_back(std::move(binding_clone));
+  }
+  callback(std::move(bindings_clone));
 }
 
 void TestApiBindings::Connect(

fuchsia/runners/cast/testdata/css_animation.html

@@ -31,14 +31,14 @@
         isStarted = true;
       });
       animated.addEventListener('animationend', function() {
-        document.title = 'animation finished';
-      });
+        cast.__platform__.PortConnector.bind('animation_finished');
 
-      // Indicates that no animation has run within a second of document load.
-      setTimeout(function() {
-        if (!isStarted)
-          document.title = 'animation never started';
-      }, 1000);
+        document.addEventListener("visibilitychange", ()=>{
+          if (document.hidden) {
+            cast.__platform__.PortConnector.bind('view_hidden');
+          }
+        }, false);
+      });
     </script>
   </body>
 </html>

fuchsia/runners/cast/testdata/empty.html

+<html></html>

fuchsia/runners/cast/testdata/mic.html → fuchsia/runners/cast/testdata/microphone.html

 <script>
+function connectMicrophone() {
     navigator.mediaDevices.getUserMedia({ audio: true, video: false })
         .then((stream) => { document.title = 'done'; });
+}
 </script>
\ No newline at end of file

fuchsia/runners/common/web_content_runner.cc

@@ -23,17 +23,16 @@
 #include "url/gurl.h"
 
 WebContentRunner::WebContentRunner(
-    GetContextParamsCallback get_context_params_callback,
-    sys::OutgoingDirectory* outgoing_directory)
-    : get_context_params_callback_(std::move(get_context_params_callback)) {
-  service_binding_.emplace(outgoing_directory, this);
-}
-
-WebContentRunner::WebContentRunner(fuchsia::web::ContextPtr context)
-    : context_(std::move(context)) {}
+    GetContextParamsCallback get_context_params_callback)
+    : get_context_params_callback_(std::move(get_context_params_callback)) {}
 
 WebContentRunner::~WebContentRunner() = default;
 
+void WebContentRunner::PublishRunnerService(
+    sys::OutgoingDirectory* outgoing_directory) {
+  service_binding_.emplace(outgoing_directory, this);
+}
+
 fuchsia::web::ContextPtr WebContentRunner::CreateWebContext(
     fuchsia::web::CreateContextParams context_params) {
   fuchsia::web::ContextPtr web_context;
@@ -77,34 +76,15 @@ void WebContentRunner::StartComponent(
   RegisterComponent(std::move(component));
 }
 
-void WebContentRunner::SetWebComponentCreatedCallbackForTest(
-    base::RepeatingCallback<void(WebComponent*)> callback) {
-  DCHECK(components_.empty());
-  web_component_created_callback_for_test_ = std::move(callback);
-}
-
 void WebContentRunner::DestroyComponent(WebComponent* component) {
   components_.erase(components_.find(component));
 }
 
 void WebContentRunner::RegisterComponent(
     std::unique_ptr<WebComponent> component) {
-  if (web_component_created_callback_for_test_)
-    web_component_created_callback_for_test_.Run(component.get());
-
   components_.insert(std::move(component));
 }
 
-void WebContentRunner::SetContextProviderForTest(
-    fuchsia::web::ContextProviderPtr context_provider) {
-  DCHECK(context_provider);
-  context_provider_ = std::move(context_provider);
-}
-
-void WebContentRunner::DisconnectContextForTest() {
-  context_.Unbind();
-}
-
 fuchsia::web::ContextProvider* WebContentRunner::GetContextProvider() {
   if (!context_provider_) {
     context_provider_ = base::fuchsia::ComponentContextForCurrentProcess()

fuchsia/runners/common/web_content_runner.h

@@ -29,18 +29,14 @@ class WebContentRunner : public fuchsia::sys::Runner {
   // specified OutgoingDirectory.
   // |get_context_params_callback|: Returns parameters for the Runner's
   //     web.Context.
-  // |outgoing_directory|: The directory that the Runner's services will be
-  //     published to.
-  WebContentRunner(GetContextParamsCallback get_context_params_callback,
-                   sys::OutgoingDirectory* outgoing_directory);
-
-  // Creates a Runner which launches components using the specified |context|.
-  // The caller may publish the Runner, or call StartComponent() manually to
-  // create new components with it.
-  explicit WebContentRunner(fuchsia::web::ContextPtr context);
+  explicit WebContentRunner(
+      GetContextParamsCallback get_context_params_callback);
 
   ~WebContentRunner() override;
 
+  // Publishes the fuchsia.sys.Runner service to |outgoing_directory|.
+  void PublishRunnerService(sys::OutgoingDirectory* outgoing_directory);
+
   // TODO(crbug.com/1046615): Make this static when the injected ContextProvider
   // goes away.
   fuchsia::web::ContextPtr CreateWebContext(
@@ -59,27 +55,9 @@ class WebContentRunner : public fuchsia::sys::Runner {
                       fidl::InterfaceRequest<fuchsia::sys::ComponentController>
                           controller_request) override;
 
-  // Used by tests to asynchronously access the first WebComponent.
-  void SetWebComponentCreatedCallbackForTest(
-      base::RepeatingCallback<void(WebComponent*)> callback);
-
   // Registers a WebComponent, or specialization, with this Runner.
   void RegisterComponent(std::unique_ptr<WebComponent> component);
 
-  // Overrides the environment's the ContextProvider to use.
-  // TODO(crbug.com/1046615): Use test manifests for package specification.
-  void SetContextProviderForTest(
-      fuchsia::web::ContextProviderPtr context_provider);
-
-  // Disconnects the Context used by this Runner.
-  void DisconnectContextForTest();
-
- protected:
-  base::RepeatingCallback<void(WebComponent*)>
-  web_component_created_callback_for_test() const {
-    return web_component_created_callback_for_test_;
-  }
-
  private:
   fuchsia::web::ContextProvider* GetContextProvider();
 

fuchsia/runners/web/main.cc

@@ -57,8 +57,8 @@ int main(int argc, char** argv) {
   WebContentRunner::GetContextParamsCallback get_context_params_callback =
       base::BindRepeating(&GetContextParams);
 
-  WebContentRunner runner(
-      std::move(get_context_params_callback),
+  WebContentRunner runner(std::move(get_context_params_callback));
+  runner.PublishRunnerService(
       base::fuchsia::ComponentContextForCurrentProcess()->outgoing().get());
 
   base::fuchsia::ComponentContextForCurrentProcess()