metrics/weblayer: move ChromeStabilityMetricsProvider

to components/metrics/content and renames
ContentStabilityMetricsProvider. I'm doing this to share
with WebLayer.

Having components depend upon extensions seems bad (and has
compile errors). So, I had to refactor that dependency out.

I will sort out unit-test next.

BUG=1120537
TEST=pure rename.

Change-Id: I4fd9907fa5b52c3a06222b96bb7550f02a12ab95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372779
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801425}

chrome/browser/BUILD.gn

@@ -776,14 +776,14 @@ static_library("browser") {
     "metrics/chrome_browser_main_extra_parts_metrics.h",
     "metrics/chrome_feature_list_creator.cc",
     "metrics/chrome_feature_list_creator.h",
+    "metrics/chrome_metrics_extensions_helper.cc",
+    "metrics/chrome_metrics_extensions_helper.h",
     "metrics/chrome_metrics_service_accessor.cc",
     "metrics/chrome_metrics_service_accessor.h",
     "metrics/chrome_metrics_service_client.cc",
     "metrics/chrome_metrics_service_client.h",
     "metrics/chrome_metrics_services_manager_client.cc",
     "metrics/chrome_metrics_services_manager_client.h",
-    "metrics/chrome_stability_metrics_provider.cc",
-    "metrics/chrome_stability_metrics_provider.h",
     "metrics/https_engagement_metrics_provider.cc",
     "metrics/https_engagement_metrics_provider.h",
     "metrics/incognito_observer.cc",

chrome/browser/metrics/chrome_metrics_extensions_helper.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 "chrome/browser/metrics/chrome_metrics_extensions_helper.h"
+
+#include "content/public/browser/render_process_host.h"
+#include "extensions/buildflags/buildflags.h"
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "extensions/browser/process_map.h"
+#endif
+
+ChromeMetricsExtensionsHelper::ChromeMetricsExtensionsHelper() = default;
+ChromeMetricsExtensionsHelper::~ChromeMetricsExtensionsHelper() = default;
+
+bool ChromeMetricsExtensionsHelper::IsExtensionProcess(
+    content::RenderProcessHost* render_process_host) {
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  return extensions::ProcessMap::Get(render_process_host->GetBrowserContext())
+      ->Contains(render_process_host->GetID());
+#else
+  return false;
+#endif
+}

chrome/browser/metrics/chrome_metrics_extensions_helper.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 CHROME_BROWSER_METRICS_CHROME_METRICS_EXTENSIONS_HELPER_H_
+#define CHROME_BROWSER_METRICS_CHROME_METRICS_EXTENSIONS_HELPER_H_
+
+#include "components/metrics/content/extensions_helper.h"
+
+class ChromeMetricsExtensionsHelper : public metrics::ExtensionsHelper {
+ public:
+  ChromeMetricsExtensionsHelper();
+  ChromeMetricsExtensionsHelper(const ChromeMetricsExtensionsHelper&) = delete;
+  ChromeMetricsExtensionsHelper& operator=(
+      const ChromeMetricsExtensionsHelper&) = delete;
+  ~ChromeMetricsExtensionsHelper() override;
+
+  // metrics::ExtensionsHelper:
+  bool IsExtensionProcess(
+      content::RenderProcessHost* render_process_host) override;
+};
+
+#endif  // CHROME_BROWSER_METRICS_CHROME_METRICS_EXTENSIONS_HELPER_H_

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -42,8 +42,8 @@
 #include "chrome/browser/google/google_brand.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/metrics/cached_metrics_profile.h"
+#include "chrome/browser/metrics/chrome_metrics_extensions_helper.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
-#include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
 #include "chrome/browser/metrics/desktop_platform_features_metrics_provider.h"
 #include "chrome/browser/metrics/desktop_session_duration/desktop_profile_session_durations_service_factory.h"
 #include "chrome/browser/metrics/https_engagement_metrics_provider.h"
@@ -68,6 +68,7 @@
 #include "components/history/core/browser/history_service.h"
 #include "components/metrics/call_stack_profile_metrics_provider.h"
 #include "components/metrics/component_metrics_provider.h"
+#include "components/metrics/content/content_stability_metrics_provider.h"
 #include "components/metrics/content/gpu_metrics_provider.h"
 #include "components/metrics/content/rendering_perf_metrics_provider.h"
 #include "components/metrics/content/subprocess_metrics_provider.h"
@@ -627,7 +628,8 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
       std::make_unique<OmniboxMetricsProvider>());
 
   metrics_service_->RegisterMetricsProvider(
-      std::make_unique<ChromeStabilityMetricsProvider>(local_state));
+      std::make_unique<metrics::ContentStabilityMetricsProvider>(
+          local_state, std::make_unique<ChromeMetricsExtensionsHelper>()));
 
   metrics_service_->RegisterMetricsProvider(
       std::make_unique<metrics::GPUMetricsProvider>());

chrome/browser/metrics/chrome_stability_metrics_provider_unittest.cc

@@ -2,11 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
+#include "components/metrics/content/content_stability_metrics_provider.h"
 
 #include "base/macros.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
+#include "chrome/browser/metrics/chrome_metrics_extensions_helper.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
@@ -38,6 +39,8 @@ namespace {
 const char kTestGpuProcessName[] = "content_gpu";
 const char kTestUtilityProcessName[] = "test_utility_process";
 
+}  // namespace
+
 class ChromeStabilityMetricsProviderTest : public testing::Test {
  protected:
   ChromeStabilityMetricsProviderTest() : prefs_(new TestingPrefServiceSimple) {
@@ -53,11 +56,9 @@ class ChromeStabilityMetricsProviderTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(ChromeStabilityMetricsProviderTest);
 };
 
-}  // namespace
-
 TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverGpu) {
   base::HistogramTester histogram_tester;
-  ChromeStabilityMetricsProvider provider(prefs());
+  metrics::ContentStabilityMetricsProvider provider(prefs(), nullptr);
 
   content::ChildProcessData child_process_data(content::PROCESS_TYPE_GPU);
   child_process_data.metrics_name = kTestGpuProcessName;
@@ -89,7 +90,7 @@ TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverGpu) {
 
 TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverUtility) {
   base::HistogramTester histogram_tester;
-  ChromeStabilityMetricsProvider provider(prefs());
+  metrics::ContentStabilityMetricsProvider provider(prefs(), nullptr);
 
   content::ChildProcessData child_process_data(content::PROCESS_TYPE_UTILITY);
   child_process_data.metrics_name = kTestUtilityProcessName;
@@ -131,7 +132,8 @@ TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverUtility) {
 
 TEST_F(ChromeStabilityMetricsProviderTest, NotificationObserver) {
   base::HistogramTester histogram_tester;
-  ChromeStabilityMetricsProvider provider(prefs());
+  metrics::ContentStabilityMetricsProvider provider(
+      prefs(), std::make_unique<ChromeMetricsExtensionsHelper>());
   std::unique_ptr<TestingProfileManager> profile_manager(
       new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
   EXPECT_TRUE(profile_manager->SetUp());

components/metrics/BUILD.gn

@@ -217,6 +217,9 @@ static_library("component_metrics") {
 if (!is_ios) {
   static_library("content") {
     sources = [
+      "content/content_stability_metrics_provider.cc",
+      "content/content_stability_metrics_provider.h",
+      "content/extensions_helper.h",
       "content/gpu_metrics_provider.cc",
       "content/gpu_metrics_provider.h",
       "content/rendering_perf_metrics_provider.cc",
@@ -228,8 +231,12 @@ if (!is_ios) {
     deps = [
       "//base",
       "//content/public/browser",
+      "//extensions/buildflags",
       "//gpu/config",
     ]
+    if (is_android) {
+      deps += [ "//components/crash/content/browser" ]
+    }
   }
 }
 

components/metrics/content/DEPS

 include_rules = [
+  "+components/crash",
   "+content/public/browser",
+  "+content/public/common",
   "+gpu/config",
+  "+ppapi/buildflags/buildflags.h",
 ]
 
 specific_include_rules = {

chrome/browser/metrics/chrome_stability_metrics_provider.cc → components/metrics/content/content_stability_metrics_provider.cc

@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
+#include "components/metrics/content/content_stability_metrics_provider.h"
 
 #include <vector>
 
 #include "base/check.h"
 #include "base/notreached.h"
 #include "build/build_config.h"
+#include "components/metrics/content/extensions_helper.h"
 #include "content/public/browser/browser_child_process_observer.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/child_process_termination_info.h"
@@ -16,28 +17,23 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/process_type.h"
-#include "extensions/buildflags/buildflags.h"
 #include "ppapi/buildflags/buildflags.h"
 
 #if defined(OS_ANDROID)
 #include "components/crash/content/browser/crash_metrics_reporter_android.h"
 #endif
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "extensions/browser/process_map.h"
-#endif
-
-#if BUILDFLAG(ENABLE_PLUGINS)
-#include "chrome/browser/metrics/plugin_metrics_provider.h"
-#endif
+namespace metrics {
 
-ChromeStabilityMetricsProvider::ChromeStabilityMetricsProvider(
-    PrefService* local_state)
+ContentStabilityMetricsProvider::ContentStabilityMetricsProvider(
+    PrefService* local_state,
+    std::unique_ptr<ExtensionsHelper> extensions_helper)
     :
 #if defined(OS_ANDROID)
       scoped_observer_(this),
 #endif  // defined(OS_ANDROID)
-      helper_(local_state) {
+      helper_(local_state),
+      extensions_helper_(std::move(extensions_helper)) {
   BrowserChildProcessObserver::Add(this);
 
   registrar_.Add(this, content::NOTIFICATION_LOAD_START,
@@ -56,27 +52,25 @@ ChromeStabilityMetricsProvider::ChromeStabilityMetricsProvider(
 #endif  // defined(OS_ANDROID)
 }
 
-ChromeStabilityMetricsProvider::~ChromeStabilityMetricsProvider() {
+ContentStabilityMetricsProvider::~ContentStabilityMetricsProvider() {
   registrar_.RemoveAll();
   BrowserChildProcessObserver::Remove(this);
 }
 
-void ChromeStabilityMetricsProvider::OnRecordingEnabled() {
-}
+void ContentStabilityMetricsProvider::OnRecordingEnabled() {}
 
-void ChromeStabilityMetricsProvider::OnRecordingDisabled() {
-}
+void ContentStabilityMetricsProvider::OnRecordingDisabled() {}
 
-void ChromeStabilityMetricsProvider::ProvideStabilityMetrics(
-    metrics::SystemProfileProto* system_profile_proto) {
+void ContentStabilityMetricsProvider::ProvideStabilityMetrics(
+    SystemProfileProto* system_profile_proto) {
   helper_.ProvideStabilityMetrics(system_profile_proto);
 }
 
-void ChromeStabilityMetricsProvider::ClearSavedStabilityMetrics() {
+void ContentStabilityMetricsProvider::ClearSavedStabilityMetrics() {
   helper_.ClearSavedStabilityMetrics();
 }
 
-void ChromeStabilityMetricsProvider::Observe(
+void ContentStabilityMetricsProvider::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
@@ -88,15 +82,10 @@ void ChromeStabilityMetricsProvider::Observe(
     case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
       content::ChildProcessTerminationInfo* process_info =
           content::Details<content::ChildProcessTerminationInfo>(details).ptr();
-      bool was_extension_process = false;
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-      content::RenderProcessHost* host =
-          content::Source<content::RenderProcessHost>(source).ptr();
-      if (extensions::ProcessMap::Get(host->GetBrowserContext())
-              ->Contains(host->GetID())) {
-        was_extension_process = true;
-      }
-#endif
+      bool was_extension_process =
+          extensions_helper_ &&
+          extensions_helper_->IsExtensionProcess(
+              content::Source<content::RenderProcessHost>(source).ptr());
       helper_.LogRendererCrash(was_extension_process, process_info->status,
                                process_info->exit_code);
       break;
@@ -107,15 +96,10 @@ void ChromeStabilityMetricsProvider::Observe(
       break;
 
     case content::NOTIFICATION_RENDERER_PROCESS_CREATED: {
-      bool was_extension_process = false;
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-      content::RenderProcessHost* host =
-          content::Source<content::RenderProcessHost>(source).ptr();
-      if (extensions::ProcessMap::Get(host->GetBrowserContext())
-              ->Contains(host->GetID())) {
-        was_extension_process = true;
-      }
-#endif
+      bool was_extension_process =
+          extensions_helper_ &&
+          extensions_helper_->IsExtensionProcess(
+              content::Source<content::RenderProcessHost>(source).ptr());
       helper_.LogRendererLaunched(was_extension_process);
       break;
     }
@@ -126,15 +110,17 @@ void ChromeStabilityMetricsProvider::Observe(
   }
 }
 
-void ChromeStabilityMetricsProvider::BrowserChildProcessCrashed(
+void ContentStabilityMetricsProvider::BrowserChildProcessCrashed(
     const content::ChildProcessData& data,
     const content::ChildProcessTerminationInfo& info) {
   DCHECK(!data.metrics_name.empty());
 #if BUILDFLAG(ENABLE_PLUGINS)
   // Exclude plugin crashes from the count below because we report them via
   // a separate UMA metric.
-  if (PluginMetricsProvider::IsPluginProcess(data.process_type))
+  if (data.process_type == content::PROCESS_TYPE_PPAPI_PLUGIN ||
+      data.process_type == content::PROCESS_TYPE_PPAPI_BROKER) {
     return;
+  }
 #endif
 
   if (data.process_type == content::PROCESS_TYPE_UTILITY)
@@ -142,7 +128,7 @@ void ChromeStabilityMetricsProvider::BrowserChildProcessCrashed(
   helper_.BrowserChildProcessCrashed();
 }
 
-void ChromeStabilityMetricsProvider::BrowserChildProcessLaunchedAndConnected(
+void ContentStabilityMetricsProvider::BrowserChildProcessLaunchedAndConnected(
     const content::ChildProcessData& data) {
   DCHECK(!data.metrics_name.empty());
   if (data.process_type == content::PROCESS_TYPE_UTILITY)
@@ -150,7 +136,7 @@ void ChromeStabilityMetricsProvider::BrowserChildProcessLaunchedAndConnected(
 }
 
 #if defined(OS_ANDROID)
-void ChromeStabilityMetricsProvider::OnCrashDumpProcessed(
+void ContentStabilityMetricsProvider::OnCrashDumpProcessed(
     int rph_id,
     const crash_reporter::CrashMetricsReporter::ReportedCrashTypeSet&
         reported_counts) {
@@ -163,5 +149,6 @@ void ChromeStabilityMetricsProvider::OnCrashDumpProcessed(
     helper_.IncreaseGpuCrashCount();
   }
 }
-
 #endif  // defined(OS_ANDROID)
+
+}  // namespace metrics

chrome/browser/metrics/chrome_stability_metrics_provider.h → components/metrics/content/content_stability_metrics_provider.h

@@ -2,11 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_METRICS_CHROME_STABILITY_METRICS_PROVIDER_H_
-#define CHROME_BROWSER_METRICS_CHROME_STABILITY_METRICS_PROVIDER_H_
+#ifndef COMPONENTS_METRICS_CONTENT_CONTENT_STABILITY_METRICS_PROVIDER_H_
+#define COMPONENTS_METRICS_CONTENT_CONTENT_STABILITY_METRICS_PROVIDER_H_
+
+#include <memory>
 
 #include "base/gtest_prod_util.h"
-#include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "build/build_config.h"
 #include "components/metrics/metrics_provider.h"
@@ -21,32 +22,51 @@
 
 class PrefService;
 
-// ChromeStabilityMetricsProvider gathers and logs Chrome-specific stability-
+FORWARD_DECLARE_TEST(ChromeStabilityMetricsProviderTest,
+                     BrowserChildProcessObserverGpu);
+FORWARD_DECLARE_TEST(ChromeStabilityMetricsProviderTest,
+                     BrowserChildProcessObserverUtility);
+FORWARD_DECLARE_TEST(ChromeStabilityMetricsProviderTest, NotificationObserver);
+
+namespace metrics {
+
+class ExtensionsHelper;
+
+// ContentStabilityMetricsProvider gathers and logs Chrome-specific stability-
 // related metrics.
-class ChromeStabilityMetricsProvider
-    : public metrics::MetricsProvider,
+class ContentStabilityMetricsProvider
+    : public MetricsProvider,
       public content::BrowserChildProcessObserver,
 #if defined(OS_ANDROID)
       public crash_reporter::CrashMetricsReporter::Observer,
 #endif
       public content::NotificationObserver {
  public:
-  explicit ChromeStabilityMetricsProvider(PrefService* local_state);
-  ~ChromeStabilityMetricsProvider() override;
+  // |extensions_helper| is used to determine if a process corresponds to an
+  // extension and is optional. If an ExtensionsHelper is not supplied it is
+  // assumed the process does not correspond to an extension.
+  ContentStabilityMetricsProvider(
+      PrefService* local_state,
+      std::unique_ptr<ExtensionsHelper> extensions_helper);
+  ContentStabilityMetricsProvider(const ContentStabilityMetricsProvider&) =
+      delete;
+  ContentStabilityMetricsProvider& operator=(
+      const ContentStabilityMetricsProvider&) = delete;
+  ~ContentStabilityMetricsProvider() override;
 
-  // metrics::MetricsDataProvider:
+  // MetricsDataProvider:
   void OnRecordingEnabled() override;
   void OnRecordingDisabled() override;
   void ProvideStabilityMetrics(
-      metrics::SystemProfileProto* system_profile_proto) override;
+      SystemProfileProto* system_profile_proto) override;
   void ClearSavedStabilityMetrics() override;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(ChromeStabilityMetricsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(::ChromeStabilityMetricsProviderTest,
                            BrowserChildProcessObserverGpu);
-  FRIEND_TEST_ALL_PREFIXES(ChromeStabilityMetricsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(::ChromeStabilityMetricsProviderTest,
                            BrowserChildProcessObserverUtility);
-  FRIEND_TEST_ALL_PREFIXES(ChromeStabilityMetricsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(::ChromeStabilityMetricsProviderTest,
                            NotificationObserver);
 
   // content::NotificationObserver:
@@ -73,12 +93,14 @@ class ChromeStabilityMetricsProvider
       scoped_observer_;
 #endif  // defined(OS_ANDROID)
 
-  metrics::StabilityMetricsHelper helper_;
+  StabilityMetricsHelper helper_;
 
   // Registrar for receiving stability-related notifications.
   content::NotificationRegistrar registrar_;
 
-  DISALLOW_COPY_AND_ASSIGN(ChromeStabilityMetricsProvider);
+  std::unique_ptr<ExtensionsHelper> extensions_helper_;
 };
 
-#endif  // CHROME_BROWSER_METRICS_CHROME_STABILITY_METRICS_PROVIDER_H_
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_CONTENT_CONTENT_STABILITY_METRICS_PROVIDER_H_

components/metrics/content/extensions_helper.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 COMPONENTS_METRICS_CONTENT_EXTENSIONS_HELPER_H_
+#define COMPONENTS_METRICS_CONTENT_EXTENSIONS_HELPER_H_
+
+namespace content {
+class RenderProcessHost;
+}
+
+namespace metrics {
+
+// ExtensionsHelper is used by ContentStabilityMetricsProvider to determine
+// if a RenderProcessHost hosts an extension. This is separate from
+// ContentStabilityMetricsProvider to avoid this code depending directly on
+// extensions.
+class ExtensionsHelper {
+ public:
+  virtual ~ExtensionsHelper() = default;
+
+  // Returns true if |render_process_host| is hosting an extension.
+  virtual bool IsExtensionProcess(
+      content::RenderProcessHost* render_process_host) = 0;
+};
+
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_CONTENT_EXTENSIONS_HELPER_H_