Reland "[fuchsia] Implement per-app remote debugging in CastRunner."

This is a reland of 304c2408

Original change's description:
> [fuchsia] Implement per-app remote debugging in CastRunner.
>
> * Handle the |enable_remote_debugging| parameter in
>   chromium.cast.ApplicationConfig to enable per-app remote debugging
>   in CastRunner.
> * Set the CastRunner debugging port to 9222.
> * Remove a buildflag to enable remote debugging Context-wide.
> * Add a test to check remote debugging is properly enabled per-App.
>
> Bug: 997895
>
> Change-Id: I19aced4e90790e77cc3f55b1118fbf459ae19b14
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1773707
> Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
> Reviewed-by: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#702264}

Bug: 997895
Change-Id: Ife9ac25676dbae1866a71cf2445899973f2e9962
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837487Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702508}

fuchsia/runners/BUILD.gn

@@ -10,10 +10,6 @@ import("//build/config/fuchsia/symbol_archive.gni")
 import("//testing/test.gni")
 
 declare_args() {
-  # Enable remote debugging for WebContentRunner instances on this port.
-  # Set to 0 to disable.
-  enable_remote_debugging_on_port = 0
-
   # Allow the Cast Runner to use software decoders for rendering video.
   # TODO(crbug.com/1000858): Revisit this value once hardware playback is fixed
   #                          (fxb/13659).
@@ -22,10 +18,7 @@ declare_args() {
 
 buildflag_header("buildflags") {
   header = "buildflags.h"
-  flags = [
-    "ENABLE_REMOTE_DEBUGGING_ON_PORT=$enable_remote_debugging_on_port",
-    "ENABLE_SOFTWARE_VIDEO_DECODERS=$enable_software_video_decoders",
-  ]
+  flags = [ "ENABLE_SOFTWARE_VIDEO_DECODERS=$enable_software_video_decoders" ]
   visibility = [ ":*" ]
 }
 
@@ -38,7 +31,6 @@ source_set("common") {
     "common/web_content_runner.h",
   ]
   deps = [
-    ":buildflags",
     "//base",
     "//fuchsia/base",
     "//fuchsia/base:modular",

fuchsia/runners/cast/cast_component.cc

@@ -37,25 +37,36 @@ CastComponent::CastComponent(CastRunner* runner,
       agent_manager_(std::move(params.agent_manager)),
       application_config_(std::move(params.app_config)),
       rewrite_rules_provider_(std::move(params.rewrite_rules_provider)),
-      connector_(frame()),
+      initial_rewrite_rules_(std::move(params.rewrite_rules.value())),
       api_bindings_client_(std::move(params.api_bindings_client)),
       navigation_listener_binding_(this) {
   base::AutoReset<bool> constructor_active_reset(&constructor_active_, true);
+}
+
+CastComponent::~CastComponent() = default;
+
+void CastComponent::StartComponent() {
+  if (application_config_.has_enable_remote_debugging() &&
+      application_config_.enable_remote_debugging()) {
+    WebComponent::EnableRemoteDebugging();
+  }
+
+  WebComponent::StartComponent();
+
+  connector_ = std::make_unique<NamedMessagePortConnector>(frame());
 
   rewrite_rules_provider_.set_error_handler([this](zx_status_t status) {
     ZX_LOG(ERROR, status) << "UrlRequestRewriteRulesProvider disconnected.";
     DestroyComponent(kRewriteRulesProviderDisconnectExitCode,
                      fuchsia::sys::TerminationReason::INTERNAL_ERROR);
   });
-
-  DCHECK(params.rewrite_rules);
-  OnRewriteRulesReceived(std::move(params.rewrite_rules.value()));
+  OnRewriteRulesReceived(std::move(initial_rewrite_rules_));
 
   frame()->SetEnableInput(false);
   frame()->SetNavigationEventListener(
       navigation_listener_binding_.NewBinding());
   api_bindings_client_->AttachToFrame(
-      frame(), &connector_,
+      frame(), connector_.get(),
       base::BindOnce(&CastComponent::DestroyComponent, base::Unretained(this),
                      kBindingsFailureExitCode,
                      fuchsia::sys::TerminationReason::INTERNAL_ERROR));
@@ -66,8 +77,6 @@ CastComponent::CastComponent(CastRunner* runner,
                    CastRunner::kAgentComponentUrl));
 }
 
-CastComponent::~CastComponent() = default;
-
 void CastComponent::DestroyComponent(int termination_exit_code,
                                      fuchsia::sys::TerminationReason reason) {
   DCHECK(!constructor_active_);
@@ -87,6 +96,6 @@ void CastComponent::OnNavigationStateChanged(
     fuchsia::web::NavigationState change,
     OnNavigationStateChangedCallback callback) {
   if (change.has_is_main_document_loaded() && change.is_main_document_loaded())
-    connector_.OnPageLoad();
+    connector_->OnPageLoad();
   callback();
 }

fuchsia/runners/cast/cast_component.h

@@ -43,6 +43,9 @@ class CastComponent : public WebComponent,
   CastComponent(CastRunner* runner, CastComponentParams params);
   ~CastComponent() override;
 
+  // WebComponent overrides.
+  void StartComponent() override;
+
  private:
   // TODO(crbug.com/953958): Remove this.
   void InitializeCastPlatformBindings();
@@ -63,9 +66,10 @@ class CastComponent : public WebComponent,
   std::unique_ptr<cr_fuchsia::AgentManager> agent_manager_;
   chromium::cast::ApplicationConfig application_config_;
   chromium::cast::UrlRequestRewriteRulesProviderPtr rewrite_rules_provider_;
+  std::vector<fuchsia::web::UrlRequestRewriteRule> initial_rewrite_rules_;
 
   bool constructor_active_ = false;
-  NamedMessagePortConnector connector_;
+  std::unique_ptr<NamedMessagePortConnector> connector_;
   std::unique_ptr<ApiBindingsClient> api_bindings_client_;
   std::unique_ptr<ApplicationControllerImpl> application_controller_;
 

fuchsia/runners/cast/cast_runner.cc

@@ -136,6 +136,7 @@ void CastRunner::MaybeStartComponent(
       std::make_unique<CastComponent>(this, std::move(*pending_component));
   pending_components_.erase(pending_component);
 
+  component->StartComponent();
   component->LoadUrl(std::move(cast_app_url),
                      std::vector<fuchsia::net::http::Header>());
   RegisterComponent(std::move(component));

fuchsia/runners/cast/cast_runner_integration_test.cc

@@ -22,6 +22,7 @@
 #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/test_navigation_listener.h"
 #include "fuchsia/base/url_request_rewrite_test_util.h"
 #include "fuchsia/runners/cast/cast_runner.h"
@@ -168,11 +169,17 @@ class CastRunnerIntegrationTest : public testing::Test {
             base::MakeExpectedNotRunClosure(FROM_HERE, "Run() timed out.")) {
     // Create the CastRunner, published into |test_services_|.
     constexpr fuchsia::web::ContextFeatureFlags kFeatures = {};
+
+    fuchsia::web::CreateContextParams create_context_params =
+        WebContentRunner::BuildCreateContextParams(
+            fidl::InterfaceHandle<fuchsia::io::Directory>(), kFeatures);
+
+    const uint16_t kRemoteDebuggingAnyPort = 0;
+    create_context_params.set_remote_debugging_port(kRemoteDebuggingAnyPort);
+
     cast_runner_ = std::make_unique<CastRunner>(
         &outgoing_directory_,
-        WebContentRunner::CreateWebContext(
-            WebContentRunner::BuildCreateContextParams(
-                fidl::InterfaceHandle<fuchsia::io::Directory>(), kFeatures)));
+        WebContentRunner::CreateWebContext(std::move(create_context_params)));
 
     // Connect to the CastRunner's fuchsia.sys.Runner interface.
     fidl::InterfaceHandle<fuchsia::io::Directory> directory;
@@ -301,7 +308,7 @@ TEST_F(CastRunnerIntegrationTest, BasicRequest) {
   const char kBlankAppId[] = "00000000";
   const char kBlankAppPath[] = "/defaultresponse";
   app_config_manager_.AddAppMapping(kBlankAppId,
-                                    test_server_.GetURL(kBlankAppPath));
+                                    test_server_.GetURL(kBlankAppPath), false);
 
   // Launch the test-app component.
   fuchsia::sys::ComponentControllerPtr component_controller =
@@ -348,7 +355,7 @@ TEST_F(CastRunnerIntegrationTest, ApiBindings) {
   const char kBlankAppId[] = "00000000";
   const char kBlankAppPath[] = "/echo.html";
   app_config_manager_.AddAppMapping(kBlankAppId,
-                                    test_server_.GetURL(kBlankAppPath));
+                                    test_server_.GetURL(kBlankAppPath), false);
 
   std::vector<chromium::cast::ApiBinding> binding_list;
   chromium::cast::ApiBinding echo_binding;
@@ -410,7 +417,7 @@ TEST_F(CastRunnerIntegrationTest, UrlRequestRewriteRulesProvider) {
   const char kEchoAppId[] = "00000000";
   const char kEchoAppPath[] = "/echoheader?Test";
   const GURL echo_app_url = test_server_.GetURL(kEchoAppPath);
-  app_config_manager_.AddAppMapping(kEchoAppId, echo_app_url);
+  app_config_manager_.AddAppMapping(kEchoAppId, echo_app_url, false);
 
   // Launch the test-app component.
   fuchsia::sys::ComponentControllerPtr component_controller =
@@ -448,8 +455,8 @@ TEST_F(CastRunnerIntegrationTest, UrlRequestRewriteRulesProvider) {
 TEST_F(CastRunnerIntegrationTest, ApplicationControllerBound) {
   const char kCastChannelAppId[] = "00000001";
   const char kCastChannelAppPath[] = "/defaultresponse";
-  app_config_manager_.AddAppMapping(kCastChannelAppId,
-                                    test_server_.GetURL(kCastChannelAppPath));
+  app_config_manager_.AddAppMapping(
+      kCastChannelAppId, test_server_.GetURL(kCastChannelAppPath), false);
 
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent(base::StringPrintf("cast:%s", kCastChannelAppId));
@@ -470,8 +477,8 @@ TEST_F(CastRunnerIntegrationTest, ApplicationControllerNotSupported) {
 
   provide_controller_receiver_ = false;
 
-  app_config_manager_.AddAppMapping(kCastChannelAppId,
-                                    test_server_.GetURL(kCastChannelAppPath));
+  app_config_manager_.AddAppMapping(
+      kCastChannelAppId, test_server_.GetURL(kCastChannelAppPath), false);
 
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent(base::StringPrintf("cast:%s", kCastChannelAppId));
@@ -492,8 +499,8 @@ TEST_F(CastRunnerIntegrationTest, Lifecycle) {
 
   provide_controller_receiver_ = false;
 
-  app_config_manager_.AddAppMapping(kCastChannelAppId,
-                                    test_server_.GetURL(kCastChannelAppPath));
+  app_config_manager_.AddAppMapping(
+      kCastChannelAppId, test_server_.GetURL(kCastChannelAppPath), false);
 
   fuchsia::sys::ComponentControllerPtr component_controller =
       StartCastComponent(base::StringPrintf("cast:%s", kCastChannelAppId));
@@ -510,4 +517,44 @@ TEST_F(CastRunnerIntegrationTest, Lifecycle) {
   run_loop.Run();
 }
 
+// Verify an App launched with remote debugging enabled is properly reachable.
+TEST_F(CastRunnerIntegrationTest, RemoteDebugging) {
+  const char kBlankAppId[] = "00000000";
+  const char kBlankAppPath[] = "/defaultresponse";
+  const GURL kBlankAppUrl = test_server_.GetURL(kBlankAppPath);
+
+  app_config_manager_.AddAppMapping(kBlankAppId, kBlankAppUrl, true);
+
+  // Launch the test-app component.
+  fuchsia::sys::ComponentControllerPtr component_controller =
+      StartCastComponent(base::StringPrintf("cast:%s", kBlankAppId));
+  component_controller.set_error_handler(&ComponentErrorHandler);
+
+  // Get the remote debugging port from the Context.
+  uint16_t remote_debugging_port = 0;
+  {
+    base::RunLoop run_loop;
+    cr_fuchsia::ResultReceiver<
+        fuchsia::web::Context_GetRemoteDebuggingPort_Result>
+        port_receiver(run_loop.QuitClosure());
+    cast_runner_->context()->GetRemoteDebuggingPort(
+        cr_fuchsia::CallbackToFitFunction(port_receiver.GetReceiveCallback()));
+    run_loop.Run();
+
+    ASSERT_TRUE(port_receiver->is_response());
+    remote_debugging_port = port_receiver->response().port;
+    ASSERT_TRUE(remote_debugging_port != 0);
+  }
+
+  // Connect to the debug service and ensure we get the proper response.
+  base::Value devtools_list =
+      cr_fuchsia::GetDevToolsListFromPort(remote_debugging_port);
+  ASSERT_TRUE(devtools_list.is_list());
+  EXPECT_EQ(devtools_list.GetList().size(), 1u);
+
+  base::Value* devtools_url = devtools_list.GetList()[0].FindPath("url");
+  ASSERT_TRUE(devtools_url->is_string());
+  EXPECT_EQ(devtools_url->GetString(), kBlankAppUrl.spec());
+}
+
 }  // namespace castrunner

fuchsia/runners/cast/fake_application_config_manager.cc

@@ -15,21 +15,27 @@ FakeApplicationConfigManager::~FakeApplicationConfigManager() = default;
 
 void FakeApplicationConfigManager::GetConfig(std::string id,
                                              GetConfigCallback callback) {
-  if (id_to_url_.find(id) == id_to_url_.end()) {
+  const auto iter = id_to_application_config_.find(id);
+  if (iter == id_to_application_config_.end()) {
     LOG(ERROR) << "Unknown Cast App ID: " << id;
     callback(chromium::cast::ApplicationConfig());
     return;
   }
 
   chromium::cast::ApplicationConfig app_config;
-  app_config.set_id(id);
-  app_config.set_display_name("Dummy test app");
-  app_config.set_web_url(id_to_url_[id].spec());
-
+  zx_status_t status = iter->second.Clone(&app_config);
+  DCHECK_EQ(status, ZX_OK);
   callback(std::move(app_config));
 }
 
 void FakeApplicationConfigManager::AddAppMapping(const std::string& id,
-                                                 const GURL& url) {
-  id_to_url_[id] = url;
+                                                 const GURL& url,
+                                                 bool enable_remote_debugging) {
+  chromium::cast::ApplicationConfig app_config;
+  app_config.set_id(id);
+  app_config.set_display_name("Dummy test app");
+  app_config.set_web_url(url.spec());
+  app_config.set_enable_remote_debugging(enable_remote_debugging);
+
+  id_to_application_config_[id] = std::move(app_config);
 }

fuchsia/runners/cast/fake_application_config_manager.h

@@ -22,13 +22,16 @@ class FakeApplicationConfigManager
 
   // Associates a Cast application |id| with a url, to be served from the
   // EmbeddedTestServer.
-  void AddAppMapping(const std::string& id, const GURL& url);
+  void AddAppMapping(const std::string& id,
+                     const GURL& url,
+                     bool enable_remote_debugging);
 
   // chromium::cast::ApplicationConfigManager interface.
   void GetConfig(std::string id, GetConfigCallback config_callback) override;
 
  private:
-  std::map<std::string, GURL> id_to_url_;
+  std::map<std::string, chromium::cast::ApplicationConfig>
+      id_to_application_config_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeApplicationConfigManager);
 };

fuchsia/runners/cast/main.cc

@@ -43,6 +43,9 @@ int main(int argc, char** argv) {
   create_context_params.set_user_agent_product("CrKey");
   create_context_params.set_user_agent_version("0");
 
+  const uint16_t kRemoteDebuggingPort = 9222;
+  create_context_params.set_remote_debugging_port(kRemoteDebuggingPort);
+
   CastRunner runner(
       base::fuchsia::ComponentContextForCurrentProcess()->outgoing().get(),
       WebContentRunner::CreateWebContext(std::move(create_context_params)));

fuchsia/runners/common/web_component.cc

@@ -42,9 +42,31 @@ WebComponent::WebComponent(
       DestroyComponent(0, fuchsia::sys::TerminationReason::UNKNOWN);
     });
   }
+}
+
+WebComponent::~WebComponent() {
+  // If Modular is available, request to be removed from the Story.
+  if (module_context_)
+    module_context_->RemoveSelfFromStory();
+
+  // Send process termination details to the client.
+  controller_binding_.events().OnTerminated(termination_exit_code_,
+                                            termination_reason_);
+}
+
+void WebComponent::EnableRemoteDebugging() {
+  DCHECK(!component_started_);
+  enable_remote_debugging_ = true;
+}
+
+void WebComponent::StartComponent() {
+  DCHECK(!component_started_);
 
   // Create the underlying Frame and get its NavigationController.
-  runner_->context()->CreateFrame(frame_.NewRequest());
+  fuchsia::web::CreateFrameParams create_params;
+  create_params.set_enable_remote_debugging(enable_remote_debugging_);
+  runner_->context()->CreateFrameWithParams(std::move(create_params),
+                                            frame_.NewRequest());
 
   // If the Frame unexpectedly disconnect us then tear-down this Component.
   frame_.set_error_handler([this](zx_status_t status) {
@@ -64,21 +86,14 @@ WebComponent::WebComponent(
         base::BindOnce(&WebComponent::Kill, base::Unretained(this)));
     startup_context()->ServeOutgoingDirectory();
   }
-}
 
-WebComponent::~WebComponent() {
-  // If Modular is available, request to be removed from the Story.
-  if (module_context_)
-    module_context_->RemoveSelfFromStory();
-
-  // Send process termination details to the client.
-  controller_binding_.events().OnTerminated(termination_exit_code_,
-                                            termination_reason_);
+  component_started_ = true;
 }
 
 void WebComponent::LoadUrl(
     const GURL& url,
     std::vector<fuchsia::net::http::Header> extra_headers) {
+  DCHECK(component_started_);
   DCHECK(url.is_valid());
   fuchsia::web::NavigationControllerPtr navigation_controller;
   frame()->GetNavigationController(navigation_controller.NewRequest());

fuchsia/runners/common/web_component.h

@@ -36,16 +36,24 @@ class WebComponent : public fuchsia::sys::ComponentController,
   // Creates a WebComponent encapsulating a web.Frame. A ViewProvider service
   // will be published to the service-directory specified by |startup_context|,
   // and if |controller_request| is valid then it will be bound to this
-  // component, and the componentconfigured to teardown if that channel closes.
+  // component, and the component configured to teardown if that channel closes.
   // |runner| must outlive this component.
   WebComponent(WebContentRunner* runner,
-               std::unique_ptr<base::fuchsia::StartupContext> startup_context,
+               std::unique_ptr<base::fuchsia::StartupContext> context,
                fidl::InterfaceRequest<fuchsia::sys::ComponentController>
                    controller_request);
 
   ~WebComponent() override;
 
+  // Enables remote debugging on this WebComponent. Must be called before
+  // StartComponent().
+  void EnableRemoteDebugging();
+
+  // Starts this component. Must be called before LoadUrl().
+  virtual void StartComponent();
+
   // Navigates this component's Frame to |url| and passes |extra_headers|.
+  // May not be called until after StartComponent().
   void LoadUrl(const GURL& url,
                std::vector<fuchsia::net::http::Header> extra_headers);
 
@@ -106,6 +114,9 @@ class WebComponent : public fuchsia::sys::ComponentController,
 
   bool view_is_bound_ = false;
 
+  bool component_started_ = false;
+  bool enable_remote_debugging_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(WebComponent);
 };
 

fuchsia/runners/common/web_content_runner.cc

@@ -18,7 +18,6 @@
 #include "base/fuchsia/scoped_service_binding.h"
 #include "base/fuchsia/startup_context.h"
 #include "base/logging.h"
-#include "fuchsia/runners/buildflags.h"
 #include "fuchsia/runners/common/web_component.h"
 #include "url/gurl.h"
 
@@ -72,12 +71,6 @@ fuchsia::web::CreateContextParams WebContentRunner::BuildCreateContextParams(
   if (data_directory)
     create_params.set_data_directory(std::move(data_directory));
 
-  // Set |remote_debugging_port| on the context, if set.
-  if (BUILDFLAG(ENABLE_REMOTE_DEBUGGING_ON_PORT) != 0) {
-    create_params.set_remote_debugging_port(
-        BUILDFLAG(ENABLE_REMOTE_DEBUGGING_ON_PORT));
-  }
-
   create_params.set_features(features);
 
   return create_params;
@@ -106,6 +99,7 @@ void WebContentRunner::StartComponent(
       this,
       std::make_unique<base::fuchsia::StartupContext>(std::move(startup_info)),
       std::move(controller_request));
+  component->StartComponent();
   component->LoadUrl(url, std::vector<fuchsia::net::http::Header>());
   RegisterComponent(std::move(component));
 }