AW UMA: componentize more metrics things for webview/layer

No change to logic. This moves version/channel info and MetricsProvider
registration into AndroidMetricsServiceClient, to avoid copying in both
WebView/WebLayer.

Bug: 1015655
Test: manual - trigger UMA upload in WebView, inspect proto, verify
Test: version and channel are correct
Test: run_webview_instrumentation_test_apk -f AwMetricsIntegrationTest.*
Test: run_components_unittests -f AndroidMetricsServiceClientTest.*
Change-Id: I108ca9de3c37a8d72c566c8bb290009502a90bd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049207Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740482}

android_webview/browser/metrics/BUILD.gn

@@ -15,11 +15,7 @@ source_set("metrics") {
     "//base",
     "//components/embedder_support/android/metrics",
     "//components/metrics",
-    "//components/metrics:gpu",
-    "//components/metrics:net",
-    "//components/metrics:ui",
     "//components/prefs",
-    "//components/version_info",
     "//components/version_info/android:channel_getter",
     "//content/public/browser",
   ]

android_webview/browser/metrics/aw_metrics_service_client.cc

@@ -12,21 +12,13 @@
 #include "android_webview/common/aw_features.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
-#include "base/base_paths_android.h"
 #include "base/feature_list.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/no_destructor.h"
-#include "components/metrics/android_metrics_provider.h"
-#include "components/metrics/call_stack_profile_metrics_provider.h"
-#include "components/metrics/drive_metrics_provider.h"
-#include "components/metrics/entropy_state_provider.h"
-#include "components/metrics/gpu/gpu_metrics_provider.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
-#include "components/metrics/version_utils.h"
 #include "components/prefs/pref_service.h"
 #include "components/version_info/android/channel_getter.h"
-#include "components/version_info/version_info.h"
 
 namespace android_webview {
 
@@ -123,14 +115,6 @@ int32_t AwMetricsServiceClient::GetProduct() {
   return metrics::ChromeUserMetricsExtension::ANDROID_WEBVIEW;
 }
 
-metrics::SystemProfileProto::Channel AwMetricsServiceClient::GetChannel() {
-  return metrics::AsProtobufChannel(version_info::android::GetChannel());
-}
-
-std::string AwMetricsServiceClient::GetVersionString() {
-  return version_info::GetVersionNumber();
-}
-
 double AwMetricsServiceClient::GetSampleRate() {
   // Down-sample unknown channel as a precaution in case it ends up being
   // shipped to Stable users.
@@ -163,13 +147,6 @@ void AwMetricsServiceClient::RegisterAdditionalMetricsProviders(
   service->RegisterMetricsProvider(
       std::make_unique<android_webview::AwStabilityMetricsProvider>(
           pref_service()));
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::AndroidMetricsProvider>());
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::DriveMetricsProvider>(
-          base::DIR_ANDROID_APP_DATA));
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::GPUMetricsProvider>());
 }
 
 std::string AwMetricsServiceClient::GetAppPackageNameInternal() {

android_webview/browser/metrics/aw_metrics_service_client.h

@@ -99,8 +99,6 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient {
 
   // metrics::MetricsServiceClient
   int32_t GetProduct() override;
-  metrics::SystemProfileProto::Channel GetChannel() override;
-  std::string GetVersionString() override;
 
   // metrics::AndroidMetricsServiceClient:
   void InitInternal() override;

components/BUILD.gn

@@ -313,6 +313,7 @@ test("components_unittests") {
       "//components/crash/android:java",
       "//components/crash/android:unit_tests",
       "//components/download/internal/common:internal_java",
+      "//components/embedder_support/android/metrics:test_support_java",
       "//components/embedder_support/android/metrics:unit_tests",
       "//components/gcm_driver/instance_id:test_support",
       "//components/gcm_driver/instance_id/android:instance_id_driver_java",

components/embedder_support/android/metrics/BUILD.gn

@@ -21,9 +21,12 @@ static_library("metrics") {
   deps = [
     ":jni",
     "//components/metrics",
+    "//components/metrics:gpu",
     "//components/metrics:net",
     "//components/metrics:ui",
     "//components/prefs",
+    "//components/version_info",
+    "//components/version_info/android:channel_getter",
     "//content/public/browser",
     "//services/resource_coordinator/public/cpp/memory_instrumentation:browser",
   ]
@@ -58,3 +61,11 @@ source_set("unit_tests") {
     "//testing/gtest",
   ]
 }
+
+# Runtime dependencies of the unit_tests target, accessed over JNI. This cannot
+# be pulled in via unit_tests deps, but should be pulled into the top-level test
+# target.
+java_group("test_support_java") {
+  testonly = true
+  deps = [ "//components/version_info/android:version_constants_java" ]
+}

components/embedder_support/android/metrics/DEPS

 include_rules = [
   "+components/metrics",
   "+components/prefs",
+  "+components/version_info",
   "+content/public/browser",
   "+services/resource_coordinator/public/cpp/memory_instrumentation",
 ]

components/embedder_support/android/metrics/android_metrics_service_client.cc

@@ -6,11 +6,15 @@
 
 #include <cstdint>
 
+#include "base/base_paths_android.h"
 #include "base/i18n/rtl.h"
 #include "build/build_config.h"
 #include "components/embedder_support/android/metrics/android_metrics_log_uploader.h"
+#include "components/metrics/android_metrics_provider.h"
 #include "components/metrics/call_stack_profile_metrics_provider.h"
 #include "components/metrics/cpu_metrics_provider.h"
+#include "components/metrics/drive_metrics_provider.h"
+#include "components/metrics/gpu/gpu_metrics_provider.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics/metrics_state_manager.h"
@@ -18,6 +22,7 @@
 #include "components/metrics/net/network_metrics_provider.h"
 #include "components/metrics/stability_metrics_helper.h"
 #include "components/metrics/ui/screen_info_metrics_provider.h"
+#include "components/metrics/version_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/notification_service.h"
@@ -121,6 +126,13 @@ AndroidMetricsServiceClient::CreateMetricsService(
       std::make_unique<ScreenInfoMetricsProvider>());
   service->RegisterMetricsProvider(
       std::make_unique<CallStackProfileMetricsProvider>());
+  service->RegisterMetricsProvider(
+      std::make_unique<metrics::AndroidMetricsProvider>());
+  service->RegisterMetricsProvider(
+      std::make_unique<metrics::DriveMetricsProvider>(
+          base::DIR_ANDROID_APP_DATA));
+  service->RegisterMetricsProvider(
+      std::make_unique<metrics::GPUMetricsProvider>());
   RegisterAdditionalMetricsProviders(service.get());
   service->InitializeMetricsRecordingState();
   return service;
@@ -200,6 +212,14 @@ bool AndroidMetricsServiceClient::GetBrand(std::string* brand_code) {
   return false;
 }
 
+SystemProfileProto::Channel AndroidMetricsServiceClient::GetChannel() {
+  return AsProtobufChannel(version_info::android::GetChannel());
+}
+
+std::string AndroidMetricsServiceClient::GetVersionString() {
+  return version_info::GetVersionNumber();
+}
+
 void AndroidMetricsServiceClient::CollectFinalMetricsForLog(
     base::OnceClosure done_callback) {
   std::move(done_callback).Run();
@@ -284,6 +304,9 @@ bool AndroidMetricsServiceClient::IsInPackageNameSample(uint32_t value) {
   return UintFallsInBottomPercentOfValues(value, GetPackageNameLimitRate());
 }
 
+void AndroidMetricsServiceClient::RegisterAdditionalMetricsProviders(
+    MetricsService* service) {}
+
 std::string AndroidMetricsServiceClient::GetAppPackageName() {
   if (IsInPackageNameSample() && CanRecordPackageNameForAppType())
     return GetAppPackageNameInternal();

components/embedder_support/android/metrics/android_metrics_service_client.h

@@ -14,6 +14,8 @@
 #include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_log_uploader.h"
 #include "components/metrics/metrics_service_client.h"
+#include "components/version_info/android/channel_getter.h"
+#include "components/version_info/version_info.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
@@ -106,6 +108,8 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
   void SetMetricsClientId(const std::string& client_id) override;
   std::string GetApplicationLocale() override;
   bool GetBrand(std::string* brand_code) override;
+  SystemProfileProto::Channel GetChannel() override;
+  std::string GetVersionString() override;
   void CollectFinalMetricsForLog(
       const base::OnceClosure done_callback) override;
   std::unique_ptr<MetricsLogUploader> CreateUploader(
@@ -174,8 +178,8 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
   virtual bool ShouldWakeMetricsService() = 0;
 
   // Called by CreateMetricsService, allows the embedder to register additional
-  // MetricsProviders.
-  virtual void RegisterAdditionalMetricsProviders(MetricsService* service) = 0;
+  // MetricsProviders. Does nothing by default.
+  virtual void RegisterAdditionalMetricsProviders(MetricsService* service);
 
   // Returns the embedding application's package name.
   virtual std::string GetAppPackageNameInternal() = 0;

components/embedder_support/android/metrics/android_metrics_service_client_unittest.cc

@@ -83,12 +83,6 @@ class TestClient : public AndroidMetricsServiceClient {
   bool IsInPackageNameSample() override { return in_package_name_sample_; }
 
   // AndroidMetricsServiceClient:
-  SystemProfileProto::Channel GetChannel() override {
-    return SystemProfileProto::CHANNEL_BETA;
-  }
-
-  std::string GetVersionString() override { return "1.1.1.1"; }
-
   int32_t GetProduct() override {
     return metrics::ChromeUserMetricsExtension::CHROME;
   }

weblayer/BUILD.gn

@@ -352,10 +352,6 @@ jumbo_static_library("weblayer_lib") {
   if (is_android) {
     deps += [
       "//components/embedder_support/android:web_contents_delegate",
-      "//components/metrics:gpu",
-      "//components/metrics:net",
-      "//components/metrics:ui",
-      "//components/version_info",
       "//services/resource_coordinator/public/cpp/memory_instrumentation:browser",
       "//ui/android",
       "//weblayer/browser/java:jni",

weblayer/browser/android/metrics/weblayer_metrics_service_client.cc

@@ -10,15 +10,9 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
-#include "base/base_paths_android.h"
 #include "base/no_destructor.h"
-#include "components/metrics/android_metrics_provider.h"
-#include "components/metrics/drive_metrics_provider.h"
-#include "components/metrics/gpu/gpu_metrics_provider.h"
 #include "components/metrics/metrics_service.h"
-#include "components/metrics/version_utils.h"
 #include "components/version_info/android/channel_getter.h"
-#include "components/version_info/version_info.h"
 #include "weblayer/browser/java/jni/MetricsServiceClient_jni.h"
 
 namespace weblayer {
@@ -60,15 +54,6 @@ int32_t WebLayerMetricsServiceClient::GetProduct() {
   return metrics::ChromeUserMetricsExtension::ANDROID_WEBLAYER;
 }
 
-metrics::SystemProfileProto::Channel
-WebLayerMetricsServiceClient::GetChannel() {
-  return metrics::AsProtobufChannel(version_info::android::GetChannel());
-}
-
-std::string WebLayerMetricsServiceClient::GetVersionString() {
-  return version_info::GetVersionNumber();
-}
-
 std::string WebLayerMetricsServiceClient::GetAppPackageNameInternal() {
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jstring> j_app_name =
@@ -99,17 +84,6 @@ bool WebLayerMetricsServiceClient::ShouldWakeMetricsService() {
   return true;
 }
 
-void WebLayerMetricsServiceClient::RegisterAdditionalMetricsProviders(
-    metrics::MetricsService* service) {
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::AndroidMetricsProvider>());
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::DriveMetricsProvider>(
-          base::DIR_ANDROID_APP_DATA));
-  service->RegisterMetricsProvider(
-      std::make_unique<metrics::GPUMetricsProvider>());
-}
-
 bool WebLayerMetricsServiceClient::CanRecordPackageNameForAppType() {
   // Check with Java side, to see if it's OK to log the package name for this
   // type of app (see Java side for the specific requirements).

weblayer/browser/android/metrics/weblayer_metrics_service_client.h

@@ -14,7 +14,6 @@
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
 #include "components/embedder_support/android/metrics/android_metrics_service_client.h"
-#include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_log_uploader.h"
 #include "components/metrics/metrics_service_client.h"
 #include "content/public/browser/notification_observer.h"
@@ -34,17 +33,13 @@ class WebLayerMetricsServiceClient
 
   // metrics::WebLayerMetricsServiceClient
   int32_t GetProduct() override;
-  metrics::SystemProfileProto::Channel GetChannel() override;
   double GetSampleRate() override;
-  std::string GetVersionString() override;
 
   // metrics::MobileWebLayerMetricsServiceClient:
   void InitInternal() override;
   void OnMetricsStart() override;
   double GetPackageNameLimitRate() override;
   bool ShouldWakeMetricsService() override;
-  void RegisterAdditionalMetricsProviders(
-      metrics::MetricsService* service) override;
   bool CanRecordPackageNameForAppType() override;
   std::string GetAppPackageNameInternal() override;