[Fuchsia] Refactor cast_runner_intergration_tests to isolate CastRunner

Previously cast_runner_integration_tests were creating CastRunner
instance in-process instead of launching it in a separate component.
As result the tests were not executing all parts of CastRunner.
Particularly code in main.cc was not tested. Updated the tests to start
cast_runner component in a separate process.
Also updated cast_runner to allow enabling headless mode with the
--headless argument.

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

fuchsia/runners/BUILD.gn

@@ -60,6 +60,8 @@ source_set("cast_runner_core") {
     "cast/cast_component.h",
     "cast/cast_runner.cc",
     "cast/cast_runner.h",
+    "cast/cast_runner_switches.cc",
+    "cast/cast_runner_switches.h",
     "cast/cast_streaming.cc",
     "cast/cast_streaming.h",
     "cast/named_message_port_connector_fuchsia.cc",
@@ -195,10 +197,16 @@ test("cast_runner_integration_tests") {
     "//third_party/fuchsia-sdk/sdk/pkg/sys_cpp",
   ]
   data_deps = [ ":cast_runner_core" ]
-  package_deps = [ [
-        "//fuchsia/engine:web_engine",
-        "web_engine",
-      ] ]
+  package_deps = [
+    [
+      ":cast_runner_pkg",
+      "cast_runner",
+    ],
+    [
+      "//fuchsia/engine:web_engine",
+      "web_engine",
+    ],
+  ]
 }
 
 test("cast_runner_browsertests") {

fuchsia/runners/cast/cast_runner.cc

@@ -130,11 +130,6 @@ fuchsia::web::FrameHost* CastRunner::main_context_frame_host() const {
   return main_context_.get();
 }
 
-void CastRunner::SetOnMainContextLostCallbackForTest(
-    base::OnceClosure on_context_lost) {
-  main_context_->SetOnContextLostCallbackForTest(std::move(on_context_lost));
-}
-
 void CastRunner::LaunchPendingComponent(PendingCastComponent* pending_component,
                                         CastComponent::Params params) {
   // Save the list of CORS exemptions so that they can be used in Context

fuchsia/runners/cast/cast_runner.h

@@ -56,8 +56,6 @@ class CastRunner : public fuchsia::sys::Runner,
   // host non-isolated Cast applications.
   fuchsia::web::FrameHost* main_context_frame_host() const;
 
-  void SetOnMainContextLostCallbackForTest(base::OnceClosure on_context_lost);
-
   // Disables use of the VULKAN feature when creating Contexts. Must be set
   // before calling StartComponent().
   void set_disable_vulkan_for_test() { disable_vulkan_for_test_ = true; }

fuchsia/runners/cast/cast_runner_integration_test.cc

@@ -20,8 +20,8 @@
 #include "base/fuchsia/file_utils.h"
 #include "base/fuchsia/filtered_service_directory.h"
 #include "base/fuchsia/fuchsia_logging.h"
+#include "base/fuchsia/process_context.h"
 #include "base/fuchsia/scoped_service_binding.h"
-#include "base/fuchsia/test_component_context_for_process.h"
 #include "base/path_service.h"
 #include "base/strings/strcat.h"
 #include "base/strings/stringprintf.h"
@@ -34,13 +34,13 @@
 #include "fuchsia/base/fake_component_context.h"
 #include "fuchsia/base/fit_adapter.h"
 #include "fuchsia/base/frame_test_util.h"
-#include "fuchsia/base/fuchsia_dir_scheme.h"
 #include "fuchsia/base/mem_buffer_util.h"
 #include "fuchsia/base/result_receiver.h"
 #include "fuchsia/base/string_util.h"
 #include "fuchsia/base/test_devtools_list_fetcher.h"
 #include "fuchsia/base/url_request_rewrite_test_util.h"
 #include "fuchsia/runners/cast/cast_runner.h"
+#include "fuchsia/runners/cast/cast_runner_switches.h"
 #include "fuchsia/runners/cast/fake_application_config_manager.h"
 #include "fuchsia/runners/cast/test_api_bindings.h"
 #include "mojo/core/embedder/embedder.h"
@@ -61,21 +61,6 @@ constexpr char kTestServerRoot[] = "fuchsia/runners/cast/testdata";
 constexpr char kDummyAgentUrl[] =
     "fuchsia-pkg://fuchsia.com/dummy_agent#meta/dummy_agent.cmx";
 
-// Helper used to ensure that cr_fuchsia::RegisterFuchsiaDirScheme() is called
-// once per process to register fuchsia-dir scheme. In cast_runner this function
-// is called in main.cc, but that code is not executed in
-// cast_runner_integration_tests.
-//
-// TODO(crbug.com/1062351): Update the tests to start cast_runner component
-// instead of creating CastRunner in process. Then remove this function.
-void EnsureFuchsiaDirSchemeInitialized() {
-  class SchemeInitializer {
-   public:
-    SchemeInitializer() { cr_fuchsia::RegisterFuchsiaDirScheme(); }
-  };
-  static SchemeInitializer initializer;
-}
-
 class FakeUrlRequestRewriteRulesProvider
     : public chromium::cast::UrlRequestRewriteRulesProvider {
  public:
@@ -214,6 +199,52 @@ class FakeComponentState : public cr_fuchsia::AgentImpl::ComponentStateBase {
       context_binding_;
   base::OnceClosure on_delete_;
 };
+
+sys::ServiceDirectory StartCastRunner(
+    fidl::InterfaceHandle<fuchsia::io::Directory> web_engine_host_directory,
+    bool enable_headless,
+    bool enable_vulkan,
+    fidl::InterfaceRequest<fuchsia::sys::ComponentController>
+        component_controller_request) {
+  fuchsia::sys::LaunchInfo launch_info;
+  launch_info.url =
+      "fuchsia-pkg://fuchsia.com/cast_runner#meta/cast_runner.cmx";
+
+  // Clone stderr from the current process to CastRunner and ask it to
+  // redirect all logs to stderr.
+  launch_info.err = fuchsia::sys::FileDescriptor::New();
+  launch_info.err->type0 = PA_FD;
+  zx_status_t status = fdio_fd_clone(
+      STDERR_FILENO, launch_info.err->handle0.reset_and_get_address());
+  ZX_CHECK(status == ZX_OK, status);
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchASCII("enable-logging", "stderr");
+  if (enable_headless)
+    command_line.AppendSwitch(kForceHeadlessForTestsSwitch);
+  if (!enable_vulkan)
+    command_line.AppendSwitch(kDisableVulkanForTestsSwitch);
+  launch_info.arguments.emplace(std::vector<std::string>(
+      command_line.argv().begin() + 1, command_line.argv().end()));
+
+  fidl::InterfaceHandle<fuchsia::io::Directory> cast_runner_services_dir;
+  launch_info.directory_request =
+      cast_runner_services_dir.NewRequest().TakeChannel();
+
+  // Redirect ContextProvider to |web_engine_host_directory|.
+  launch_info.additional_services =
+      std::make_unique<fuchsia::sys::ServiceList>();
+  launch_info.additional_services->host_directory =
+      web_engine_host_directory.TakeChannel();
+  launch_info.additional_services->names.push_back(
+      fuchsia::web::ContextProvider::Name_);
+
+  fuchsia::sys::LauncherPtr launcher;
+  base::ComponentContextForProcess()->svc()->Connect(launcher.NewRequest());
+  launcher->CreateComponent(std::move(launch_info),
+                            std::move(component_controller_request));
+  return sys::ServiceDirectory(std::move(cast_runner_services_dir));
+}
+
 }  // namespace
 
 class CastRunnerIntegrationTest : public testing::Test {
@@ -230,34 +261,26 @@ class CastRunnerIntegrationTest : public testing::Test {
   void TearDown() override {
     if (component_controller_)
       ShutdownComponent();
-
-    // Disconnect the CastRunner & let things tear-down.
-    cast_runner_ptr_.Unbind();
-    base::RunLoop().RunUntilIdle();
   }
 
  protected:
   explicit CastRunnerIntegrationTest(bool enable_headless, bool enable_vulkan)
       : app_config_manager_binding_(&component_services_,
                                     &app_config_manager_) {
-    EnsureFuchsiaDirSchemeInitialized();
-
-    // Create the CastRunner, published into |outgoing_directory_|.
-    cast_runner_ = std::make_unique<CastRunner>(enable_headless);
-    if (!enable_vulkan)
-      cast_runner_->set_disable_vulkan_for_test();
-    cast_runner_binding_.emplace(&outgoing_directory_, cast_runner_.get());
-
     StartAndPublishWebEngine();
 
+    // Start CastRunner.
+    fidl::InterfaceHandle<::fuchsia::io::Directory> incoming_services;
+    services_for_cast_runner_.GetOrCreateDirectory("svc")->Serve(
+        ::fuchsia::io::OPEN_RIGHT_READABLE | ::fuchsia::io::OPEN_RIGHT_WRITABLE,
+        incoming_services.NewRequest().TakeChannel());
+    sys::ServiceDirectory cast_runner_services =
+        StartCastRunner(std::move(incoming_services), enable_headless,
+                        enable_vulkan, cast_runner_controller_.NewRequest());
+
     // Connect to the CastRunner's fuchsia.sys.Runner interface.
-    fidl::InterfaceHandle<fuchsia::io::Directory> directory;
-    outgoing_directory_.GetOrCreateDirectory("svc")->Serve(
-        fuchsia::io::OPEN_RIGHT_READABLE | fuchsia::io::OPEN_RIGHT_WRITABLE,
-        directory.NewRequest().TakeChannel());
-    sys::ServiceDirectory public_directory_client(std::move(directory));
-    cast_runner_ptr_ = public_directory_client.Connect<fuchsia::sys::Runner>();
-    cast_runner_ptr_.set_error_handler([](zx_status_t status) {
+    cast_runner_ = cast_runner_services.Connect<fuchsia::sys::Runner>();
+    cast_runner_.set_error_handler([](zx_status_t status) {
       ZX_LOG(ERROR, status) << "CastRunner closed channel.";
       ADD_FAILURE();
     });
@@ -311,9 +334,9 @@ class CastRunnerIntegrationTest : public testing::Test {
     sys::ServiceDirectory web_engine_outgoing_services(
         std::move(web_engine_outgoing_dir));
 
-    test_component_context_.additional_services()
-        ->RemovePublicService<fuchsia::web::ContextProvider>();
-    test_component_context_.additional_services()->AddPublicService(
+    services_for_cast_runner_
+        .RemovePublicService<fuchsia::web::ContextProvider>();
+    services_for_cast_runner_.AddPublicService(
         std::make_unique<vfs::Service>(
             [web_engine_outgoing_services =
                  std::move(web_engine_outgoing_services)](
@@ -491,6 +514,7 @@ class CastRunnerIntegrationTest : public testing::Test {
   net::EmbeddedTestServer test_server_;
 
   fuchsia::sys::ComponentControllerPtr web_engine_controller_;
+  fuchsia::sys::ComponentControllerPtr cast_runner_controller_;
 
   FakeApplicationConfigManager app_config_manager_;
   TestApiBindings api_bindings_;
@@ -509,15 +533,12 @@ class CastRunnerIntegrationTest : public testing::Test {
 
   base::OnceClosure component_state_created_callback_;
 
-  // ServiceDirectory into which the CastRunner will publish itself.
-  sys::OutgoingDirectory outgoing_directory_;
+  // Directory used to publish test ContextProvider to CastRunner. Some tests
+  // restart ContextProvider, so we can't pass the services directory from
+  // ContextProvider to CastRunner directly.
+  sys::OutgoingDirectory services_for_cast_runner_;
 
-  std::unique_ptr<CastRunner> cast_runner_;
-  base::Optional<base::fuchsia::ScopedServiceBinding<fuchsia::sys::Runner>>
-      cast_runner_binding_;
-  fuchsia::sys::RunnerPtr cast_runner_ptr_;
-  base::TestComponentContextForProcess test_component_context_{
-      base::TestComponentContextForProcess::InitialState::kCloneAll};
+  fuchsia::sys::RunnerPtr cast_runner_;
 };
 
 // A basic integration test ensuring a basic cast request launches the right
@@ -547,17 +568,10 @@ TEST_F(CastRunnerIntegrationTest, CanRecreateContext) {
 
     CheckAppUrl(app_url);
 
-    // Setup a loop to wait for Context destruction after WebEngine is killed
-    // below.
-    base::RunLoop context_lost_loop;
-    cast_runner_->SetOnMainContextLostCallbackForTest(
-        context_lost_loop.QuitClosure());
-
     web_engine_controller_->Kill();
 
     // Wait for the component and the Context to be torn down.
     WaitForComponentDestroyed();
-    context_lost_loop.Run();
 
     // Start a new WebEngine instance for the next iteration.
     if (i < 1)

fuchsia/runners/cast/cast_runner_switches.cc

+// 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.
+
+#include "fuchsia/runners/cast/cast_runner_switches.h"
+
+const char kForceHeadlessForTestsSwitch[] = "force-headless-for-tests";
+
+const char kDisableVulkanForTestsSwitch[] = "disable-vulkan-for-tests";

fuchsia/runners/cast/cast_runner_switches.h

+// 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.
+
+#ifndef FUCHSIA_RUNNERS_CAST_CAST_RUNNER_SWITCHES_H_
+#define FUCHSIA_RUNNERS_CAST_CAST_RUNNER_SWITCHES_H_
+
+// Force headless mode.
+extern const char kForceHeadlessForTestsSwitch[];
+
+// Disable Vulkan flag for the cast runner. Used for tests.
+extern const char kDisableVulkanForTestsSwitch[];
+
+#endif  // FUCHSIA_RUNNERS_CAST_CAST_RUNNER_SWITCHES_H_
\ No newline at end of file

fuchsia/runners/cast/main.cc

@@ -19,6 +19,7 @@
 #include "fuchsia/base/init_logging.h"
 #include "fuchsia/base/inspect.h"
 #include "fuchsia/runners/cast/cast_runner.h"
+#include "fuchsia/runners/cast/cast_runner_switches.h"
 #include "mojo/core/embedder/embedder.h"
 
 namespace {
@@ -31,6 +32,12 @@ constexpr char kComponentUrl[] =
 bool IsHeadless() {
   constexpr char kHeadlessConfigKey[] = "headless";
 
+  // In tests headless mode can be enabled with a command-line flag.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          kForceHeadlessForTestsSwitch)) {
+    return true;
+  }
+
   const base::Optional<base::Value>& config = cr_fuchsia::LoadPackageConfig();
   if (config)
     return config->FindBoolPath(kHeadlessConfigKey).value_or(false);
@@ -52,7 +59,6 @@ bool AllowMainContextSharing() {
 
 int main(int argc, char** argv) {
   base::SingleThreadTaskExecutor io_task_executor(base::MessagePumpType::IO);
-  base::RunLoop run_loop;
 
   cr_fuchsia::RegisterProductDataForCrashReporting(kComponentUrl,
                                                    kCrashProductName);
@@ -74,6 +80,11 @@ int main(int argc, char** argv) {
   base::fuchsia::ScopedServiceBinding<fuchsia::sys::Runner> binding(
       outgoing_directory, &runner);
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          kDisableVulkanForTestsSwitch)) {
+    runner.set_disable_vulkan_for_test();  // IN-TEST
+  }
+
   // Optionally publish the fuchsia.web.FrameHost service, to allow the Cast
   // application web.Context to be shared by other components.
   base::Optional<base::fuchsia::ScopedServiceBinding<fuchsia::web::FrameHost>>
@@ -89,6 +100,7 @@ int main(int argc, char** argv) {
   cr_fuchsia::PublishVersionInfoToInspect(base::ComponentInspectorForProcess());
 
   // TODO(https://crbug.com/952560): Implement Components v2 graceful exit.
+  base::RunLoop run_loop;
   run_loop.Run();
 
   return 0;

fuchsia/runners/common/web_content_runner.cc

@@ -57,11 +57,8 @@ void WebContentRunner::CreateFrameWithParams(
   if (!context_) {
     DCHECK(get_context_params_callback_);
     context_ = CreateWebContext(get_context_params_callback_.Run());
-    context_.set_error_handler([this](zx_status_t status) {
+    context_.set_error_handler([](zx_status_t status) {
       ZX_LOG(ERROR, status) << "Connection to Context lost.";
-      if (on_context_lost_callback_) {
-        std::move(on_context_lost_callback_).Run();
-      }
     });
   }
 
@@ -112,8 +109,3 @@ void WebContentRunner::RegisterComponent(
 void WebContentRunner::SetOnEmptyCallback(base::OnceClosure on_empty) {
   on_empty_callback_ = std::move(on_empty);
 }
-
-void WebContentRunner::SetOnContextLostCallbackForTest(
-    base::OnceClosure callback) {
-  on_context_lost_callback_ = std::move(callback);
-}

fuchsia/runners/common/web_content_runner.h

@@ -61,9 +61,6 @@ class WebContentRunner : public fuchsia::sys::Runner,
   // Sets a callback to invoke when |components_| next becomes empty.
   void SetOnEmptyCallback(base::OnceClosure on_empty);
 
-  // Sets a callback that's called when |context_| disconnects.
-  void SetOnContextLostCallbackForTest(base::OnceClosure on_context_lost);
-
   // TODO(https://crbug.com/1065707): Remove this once capability routing for
   // the fuchsia.legacymetrics.Provider service is properly set up.
   // Returns a pointer to any currently running component, or nullptr if no
@@ -83,8 +80,6 @@ class WebContentRunner : public fuchsia::sys::Runner,
 
   base::OnceClosure on_empty_callback_;
 
-  base::OnceClosure on_context_lost_callback_;
-
   DISALLOW_COPY_AND_ASSIGN(WebContentRunner);
 };