[fuchsia] Connect the Agent's MetricsRecorder to WebEngine.

Allows the WebEngine to connect to an Agent-published MetricsRecorder
service.

Some simplification of CastRunner code, eliminating the need for an
alternate constructor for "isolated runners" by decoupling service
publishing from runner construction.

Added an integration test.

Bug: 1060768
Change-Id: Ia36a13cbd9a68565bd80bb32facf393b23d3cfb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2133056
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758107}

fuchsia/engine/BUILD.gn

@@ -82,6 +82,7 @@ component("web_engine_core") {
     "//content/public/common",
     "//content/public/renderer",
     "//fuchsia/base",
+    "//fuchsia/base:legacymetrics",
     "//fuchsia/base:message_port",
     "//fuchsia/base:modular",
     "//gpu/command_buffer/service",

fuchsia/engine/browser/web_engine_browser_main_parts.cc

@@ -13,6 +13,7 @@
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/common/main_function_params.h"
+#include "fuchsia/base/legacymetrics_client.h"
 #include "fuchsia/engine/browser/context_impl.h"
 #include "fuchsia/engine/browser/web_engine_browser_context.h"
 #include "fuchsia/engine/browser/web_engine_devtools_controller.h"
@@ -23,6 +24,13 @@
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/ozone/public/ozone_switches.h"
 
+namespace {
+
+constexpr base::TimeDelta kMetricsReportingInterval =
+    base::TimeDelta::FromMinutes(1);
+
+}  // namespace
+
 WebEngineBrowserMainParts::WebEngineBrowserMainParts(
     const content::MainFunctionParams& parameters,
     fidl::InterfaceRequest<fuchsia::web::Context> request)
@@ -62,6 +70,13 @@ void WebEngineBrowserMainParts::PreMainMessageLoopRun() {
   context_binding_ = std::make_unique<fidl::Binding<fuchsia::web::Context>>(
       context_service_.get(), std::move(request_));
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUseLegacyMetricsService)) {
+    legacy_metrics_client_ =
+        std::make_unique<cr_fuchsia::LegacyMetricsClient>();
+    legacy_metrics_client_->Start(kMetricsReportingInterval);
+  }
+
   // Quit the browser main loop when the Context connection is dropped.
   context_binding_->set_error_handler([this](zx_status_t status) {
     ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
@@ -104,6 +119,7 @@ void WebEngineBrowserMainParts::PostMainMessageLoopRun() {
   // These resources must be freed while a MessageLoop is still available, so
   // that they may post cleanup tasks during teardown.
   // NOTE: Please destroy objects in the reverse order of their creation.
+  legacy_metrics_client_.reset();
   context_binding_.reset();
   browser_context_.reset();
   screen_.reset();

fuchsia/engine/browser/web_engine_browser_main_parts.h

@@ -23,6 +23,10 @@ namespace content {
 struct MainFunctionParams;
 }
 
+namespace cr_fuchsia {
+class LegacyMetricsClient;
+}
+
 class WebEngineBrowserMainParts : public content::BrowserMainParts {
  public:
   explicit WebEngineBrowserMainParts(
@@ -55,6 +59,7 @@ class WebEngineBrowserMainParts : public content::BrowserMainParts {
   std::unique_ptr<ContextImpl> context_service_;
   std::unique_ptr<fidl::Binding<fuchsia::web::Context>> context_binding_;
   std::unique_ptr<WebEngineDevToolsController> devtools_controller_;
+  std::unique_ptr<cr_fuchsia::LegacyMetricsClient> legacy_metrics_client_;
 
   bool run_message_loop_ = true;
   base::OnceClosure quit_closure_;

fuchsia/engine/context_provider_impl.cc

@@ -397,6 +397,11 @@ void ContextProviderImpl::Create(
     launch_command.AppendSwitch(switches::kHeadless);
   }
 
+  if ((features & fuchsia::web::ContextFeatureFlags::LEGACYMETRICS) ==
+      fuchsia::web::ContextFeatureFlags::LEGACYMETRICS) {
+    launch_command.AppendSwitch(switches::kUseLegacyMetricsService);
+  }
+
   bool enable_vulkan = (features & fuchsia::web::ContextFeatureFlags::VULKAN) ==
                        fuchsia::web::ContextFeatureFlags::VULKAN;
   bool enable_widevine =

fuchsia/engine/switches.cc

@@ -14,5 +14,6 @@ const char kPlayreadyKeySystem[] = "playready-key-system";
 const char kRemoteDebuggerHandles[] = "remote-debugger-handles";
 const char kUserAgentProductAndVersion[] = "user-agent-product";
 const char kAllowRunningInsecureContent[] = "allow-running-insecure-content";
+const char kUseLegacyMetricsService[] = "use-legacy-metrics-service";
 
 }  // namespace switches

fuchsia/engine/switches.h

@@ -38,6 +38,10 @@ extern const char kUserAgentProductAndVersion[];
 // necessary.
 extern const char kAllowRunningInsecureContent[];
 
+// Enables use of the fuchsia.legacymetrics.MetricsRecorder service for
+// telemetry.
+extern const char kUseLegacyMetricsService[];
+
 }  // namespace switches
 
 #endif  // FUCHSIA_ENGINE_SWITCHES_H_

fuchsia/runners/BUILD.gn

@@ -76,6 +76,7 @@ source_set("cast_runner_core") {
   public_deps = [
     ":common",
     "//fuchsia:cast_fidl",
+    "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.legacymetrics",
     "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.web",
   ]
   visibility = [ ":*" ]

fuchsia/runners/cast/cast_runner.cc

@@ -43,8 +43,9 @@ static constexpr const char* kServices[] = {
     "fuchsia.ui.scenic.Scenic",
     "fuchsia.vulkan.loader.Loader",
 
-    // Redirected to the agent.
-    // fuchsia.media.Audio
+    // These services are redirected to the Agent:
+    // * fuchsia.media.Audio
+    // * fuchsia.legacymetrics.MetricsRecorder
 };
 
 bool AreCastComponentParamsValid(
@@ -68,7 +69,8 @@ fuchsia::web::CreateContextParams BuildCreateContextParamsForIsolatedRunners(
 
   // Isolated contexts receive only a limited set of features.
   fuchsia::web::ContextFeatureFlags features =
-      fuchsia::web::ContextFeatureFlags::AUDIO;
+      fuchsia::web::ContextFeatureFlags::AUDIO |
+      fuchsia::web::ContextFeatureFlags::LEGACYMETRICS;
 
   if ((create_context_params.features() &
        fuchsia::web::ContextFeatureFlags::HEADLESS) ==
@@ -303,6 +305,17 @@ CastRunner::CreateServiceDirectory() {
       }),
       fuchsia::media::Audio::Name_);
 
+  // Proxy fuchsia.legacymetrics.MetricsRecorder connection requests to the
+  // Agent.
+  service_directory->outgoing_directory()->AddPublicService(
+      std::make_unique<vfs::Service>(
+          [this](zx::channel channel, async_dispatcher_t*) {
+            this->ConnectMetricsRecorderProtocol(
+                fidl::InterfaceRequest<fuchsia::legacymetrics::MetricsRecorder>(
+                    std::move(channel)));
+          }),
+      fuchsia::legacymetrics::MetricsRecorder::Name_);
+
   return service_directory;
 }
 
@@ -315,7 +328,7 @@ void CastRunner::MaybeStartComponent(
   CastRunner* component_owner = this;
   if (pending_component_params->app_config
           .has_content_directories_for_isolated_application()) {
-    // Create a isolated, isolated CastRunner instance which will own the
+    // Create an isolated CastRunner instance which will own the
     // CastComponent.
     component_owner =
         CreateChildRunnerForIsolatedComponent(pending_component_params);
@@ -362,12 +375,6 @@ CastRunner* CastRunner::CreateChildRunnerForIsolatedComponent(
       BuildCreateContextParamsForIsolatedRunners(
           get_context_params_callback_.Run());
 
-  // Service redirection is not necessary for isolated context. Pass default
-  // /svc as is, without overriding any services.
-  isolated_context_params.set_service_directory(base::fuchsia::OpenDirectory(
-      base::FilePath(base::fuchsia::kServiceDirectoryPath)));
-  DCHECK(isolated_context_params.service_directory());
-
   isolated_context_params.set_content_directories(
       std::move(*component_params->app_config
                      .mutable_content_directories_for_isolated_application()));
@@ -411,6 +418,16 @@ void CastRunner::ConnectAudioProtocol(
       std::move(request));
 }
 
+void CastRunner::ConnectMetricsRecorderProtocol(
+    fidl::InterfaceRequest<fuchsia::legacymetrics::MetricsRecorder> request) {
+  CastComponent* component =
+      reinterpret_cast<CastComponent*>(GetAnyComponent());
+  DCHECK(component);
+
+  component->agent_manager()->ConnectToAgentService(
+      component->application_config().agent_url(), std::move(request));
+}
+
 fuchsia::web::CreateContextParams CastRunner::GetMainContextParams() {
   fuchsia::web::CreateContextParams create_context_params =
       get_context_params_callback_.Run();

fuchsia/runners/cast/cast_runner.h

@@ -5,6 +5,7 @@
 #ifndef FUCHSIA_RUNNERS_CAST_CAST_RUNNER_H_
 #define FUCHSIA_RUNNERS_CAST_CAST_RUNNER_H_
 
+#include <fuchsia/legacymetrics/cpp/fidl.h>
 #include <fuchsia/sys/cpp/fidl.h>
 #include <fuchsia/web/cpp/fidl.h>
 #include <memory>
@@ -90,6 +91,11 @@ class CastRunner : public WebContentRunner {
   void ConnectAudioProtocol(
       fidl::InterfaceRequest<fuchsia::media::Audio> request);
 
+  // Handler for fuchsia.legacymetrics.MetricsRecorder requests in
+  // |service_directory_|.
+  void ConnectMetricsRecorderProtocol(
+      fidl::InterfaceRequest<fuchsia::legacymetrics::MetricsRecorder> request);
+
   // Returns the parameters with which the main context should be created.
   fuchsia::web::CreateContextParams GetMainContextParams();
 

fuchsia/runners/cast/cast_runner_integration_test.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <fuchsia/legacymetrics/cpp/fidl.h>
+#include <fuchsia/legacymetrics/cpp/fidl_test_base.h>
 #include <fuchsia/modular/cpp/fidl.h>
 #include <lib/fdio/directory.h>
 #include <lib/fidl/cpp/binding.h>
@@ -221,11 +223,13 @@ class CastRunnerIntegrationTest : public testing::Test {
 
     // Create the CastRunner, published into |outgoing_directory_|.
     fuchsia::web::ContextFeatureFlags feature_flags =
-        fuchsia::web::ContextFeatureFlags::NETWORK;
+        fuchsia::web::ContextFeatureFlags::NETWORK |
+        fuchsia::web::ContextFeatureFlags::LEGACYMETRICS;
     if (headless) {
       feature_flags =
           feature_flags | fuchsia::web::ContextFeatureFlags::HEADLESS;
     }
+
     WebContentRunner::GetContextParamsCallback get_context_params =
         base::BindLambdaForTesting([feature_flags]() {
           fuchsia::web::CreateContextParams create_context_params;
@@ -442,13 +446,18 @@ class CastRunnerIntegrationTest : public testing::Test {
     base::RunLoop state_loop;
     component_state_->set_on_delete(state_loop.QuitClosure());
 
+    base::RunLoop controller_loop;
     if (component_controller_) {
-      base::RunLoop controller_loop;
       component_controller_.set_error_handler(
           [&controller_loop](zx_status_t status) {
             EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
             controller_loop.Quit();
           });
+    }
+
+    web_engine_controller_->Kill();
+
+    if (component_controller_) {
       controller_loop.Run();
     }
 
@@ -814,3 +823,33 @@ TEST_F(HeadlessCastRunnerIntegrationTest, IsolatedAndHeadless) {
   CreateComponentContextAndStartComponent();
   CheckAppUrl(kContentDirectoryUrl);
 }
+
+// Verifies that the Context can establish a connection to the Agent's
+// MetricsRecorder service.
+TEST_F(CastRunnerIntegrationTest, LegacyMetricsRedirect) {
+  GURL app_url = test_server_.GetURL(kBlankAppUrl);
+  app_config_manager_.AddApp(kTestAppId, app_url);
+
+  auto component_url = base::StringPrintf("cast:%s", kTestAppId);
+  CreateComponentContext(component_url);
+  EXPECT_NE(component_context_, nullptr);
+
+  StartCastComponent(component_url);
+
+  // Wait until we see the CastRunner connect to the LegacyMetrics service.
+  base::RunLoop run_loop;
+  component_state_created_callback_ = base::BindOnce(
+      [](FakeComponentState** component_state,
+         base::RepeatingClosure quit_closure) {
+        (*component_state)
+            ->outgoing_directory()
+            ->AddPublicService(
+                std::make_unique<vfs::Service>(
+                    [quit_closure](zx::channel, async_dispatcher_t*) {
+                      quit_closure.Run();
+                    }),
+                fuchsia::legacymetrics::MetricsRecorder::Name_);
+      },
+      base::Unretained(&component_state_), run_loop.QuitClosure());
+  run_loop.Run();
+}

fuchsia/runners/cast/main.cc

@@ -38,7 +38,8 @@ fuchsia::web::CreateContextParams CreateMainContextParams() {
   fuchsia::web::ContextFeatureFlags features =
       fuchsia::web::ContextFeatureFlags::NETWORK |
       fuchsia::web::ContextFeatureFlags::AUDIO |
-      fuchsia::web::ContextFeatureFlags::WIDEVINE_CDM;
+      fuchsia::web::ContextFeatureFlags::WIDEVINE_CDM |
+      fuchsia::web::ContextFeatureFlags::LEGACYMETRICS;
 
   if (IsHeadless()) {
     LOG(WARNING) << "Running in headless mode.";
@@ -52,10 +53,6 @@ fuchsia::web::CreateContextParams CreateMainContextParams() {
   fuchsia::web::CreateContextParams create_context_params;
   create_context_params.set_features(features);
 
-  create_context_params.set_service_directory(base::fuchsia::OpenDirectory(
-      base::FilePath(base::fuchsia::kServiceDirectoryPath)));
-  CHECK(create_context_params.service_directory());
-
   const char kCastPlayreadyKeySystem[] = "com.chromecast.playready";
   create_context_params.set_playready_key_system(kCastPlayreadyKeySystem);
 

fuchsia/runners/common/web_content_runner.cc

@@ -86,6 +86,13 @@ void WebContentRunner::StartComponent(
   RegisterComponent(std::move(component));
 }
 
+WebComponent* WebContentRunner::GetAnyComponent() {
+  if (components_.empty())
+    return nullptr;
+
+  return components_.begin()->get();
+}
+
 void WebContentRunner::DestroyComponent(WebComponent* component) {
   components_.erase(components_.find(component));
 }

fuchsia/runners/common/web_content_runner.h

@@ -56,6 +56,11 @@ class WebContentRunner : public fuchsia::sys::Runner {
   // Sets a callback that's called when the context is lost.
   void SetOnContextLostCallback(base::OnceClosure callback);
 
+ protected:
+  // Returns a pointer to any currently running component, or nullptr if no
+  // components are currently running.
+  WebComponent* GetAnyComponent();
+
  private:
   const GetContextParamsCallback get_context_params_callback_;