wv/wl: make AndroidMetricsServiceClient always create MetricsService

I had wanted to register the providers as well, but that is problematic.
In particular, Initialize() is called before consent has been determined
(it's async for WV/WL). As some providers are configured differently
depending upon whether reporting is enabled, configuring before reporting
is enabled is problematic.

I moved registering VariationsIdsProvider to where it happens on the
desktop.

Another possibility is to keep a pref of the last value and use that
when initializing. That way MetricsService and providers could be created
immediately. This route has the possibility for uploading some data when
we shouldn't. This change would also be trickier (bigger), and I'm hoping to
merge this to 88.

Lastly, AndroidMetricsServiceClient is initialized earlier then
happens on the desktop. In fact it's before threads have been
initialized. This means some shuffling around of creation order would
have to happen should we want to initialize all the providers (because
current place triggers DCHECKs if it does try to create providers).

BUG=1148351
TEST=MetricsServiceCreatedFromInitialize

Change-Id: Ia82225cb237da38c3bef56815382dc597155fac0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551292
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833373}

components/embedder_support/android/metrics/android_metrics_service_client.cc

@@ -248,6 +248,16 @@ void AndroidMetricsServiceClient::Initialize(PrefService* pref_service) {
                                   base::BindRepeating(&LoadClientInfo));
 
   init_finished_ = true;
+
+  // Create the MetricsService immediately so that other code can make use of
+  // it. Chrome always creates the MetricsService as well.
+  metrics_service_ = std::make_unique<MetricsService>(
+      metrics_state_manager_.get(), this, pref_service_);
+
+  // Registration of providers has to wait until consent is determined. To
+  // do otherwise means the providers would always be configured with reporting
+  // disabled (because when this is called in production consent hasn't been
+  // determined). If consent has not been determined, this does nothing.
   MaybeStartMetrics();
 }
 
@@ -260,8 +270,7 @@ void AndroidMetricsServiceClient::MaybeStartMetrics() {
   bool user_consent_or_flag = user_consent_ || IsMetricsReportingForceEnabled();
   if (IsConsentDetermined()) {
     if (app_consent_ && user_consent_or_flag) {
-      metrics_service_ = std::make_unique<MetricsService>(
-          metrics_state_manager_.get(), this, pref_service_);
+      did_start_metrics_ = true;
       // Make GetSampleBucketValue() work properly.
       metrics_state_manager_->ForceClientIdCreation();
       is_client_id_forced_ = true;
@@ -309,7 +318,7 @@ void AndroidMetricsServiceClient::RegisterMetricsProvidersAndInitState() {
       std::make_unique<metrics::GPUMetricsProvider>());
   RegisterAdditionalMetricsProviders(metrics_service_.get());
 
-  // The file metrics provider makes IO.
+  // The file metrics provider performs IO.
   base::ScopedAllowBlocking allow_io;
   metrics_service_->InitializeMetricsRecordingState();
 }
@@ -419,10 +428,14 @@ bool AndroidMetricsServiceClient::IsReportingEnabled() const {
          (EnabledStateProvider::IsReportingEnabled() && IsInSample());
 }
 
+MetricsService* AndroidMetricsServiceClient::GetMetricsServiceIfStarted() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return did_start_metrics_ ? metrics_service_.get() : nullptr;
+}
+
 MetricsService* AndroidMetricsServiceClient::GetMetricsService() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // This will be null if initialization hasn't finished, or if metrics
-  // collection is disabled.
+  // This will be null if initialization hasn't finished.
   return metrics_service_.get();
 }
 

components/embedder_support/android/metrics/android_metrics_service_client.h

@@ -87,6 +87,9 @@ extern const char kCrashpadHistogramAllocatorName[];
 // the client ID (generating a new ID if there was none). If this client is in
 // the sample, it then calls MetricsService::Start(). If consent was not
 // granted, MaybeStartMetrics() instead clears the client ID, if any.
+//
+// To match chrome on other platforms (including android), the MetricsService is
+// always created.
 class AndroidMetricsServiceClient : public MetricsServiceClient,
                                     public EnabledStateProvider,
                                     public content::NotificationObserver {
@@ -125,6 +128,10 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
   bool IsConsentGiven() const override;
   bool IsReportingEnabled() const override;
 
+  // Returns the MetricService only if it has been started (which means consent
+  // was given).
+  MetricsService* GetMetricsServiceIfStarted();
+
   // MetricsServiceClient
   MetricsService* GetMetricsService() override;
   ukm::UkmService* GetUkmService() override;
@@ -247,6 +254,7 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
   bool app_consent_ = false;
   bool is_client_id_forced_ = false;
   bool fast_startup_for_testing_ = false;
+  bool did_start_metrics_ = false;
 
   // When non-zero, this overrides the default value in
   // GetStandardUploadInterval().

components/embedder_support/android/metrics/android_metrics_service_client_unittest.cc

@@ -427,4 +427,24 @@ TEST_F(AndroidMetricsServiceClientTest,
   EXPECT_TRUE(base::PathExists(upload_dir));
 }
 
+TEST_F(AndroidMetricsServiceClientTest,
+       MetricsServiceCreatedFromInitializeWithNoConsent) {
+  auto prefs = CreateTestPrefs();
+  auto client = std::make_unique<TestClient>();
+  client->Initialize(prefs.get());
+  EXPECT_FALSE(client->IsReportingEnabled());
+  EXPECT_TRUE(client->GetMetricsService());
+}
+
+TEST_F(AndroidMetricsServiceClientTest, GetMetricsServiceIfStarted) {
+  auto prefs = CreateTestPrefs();
+  auto client = std::make_unique<TestClient>();
+  client->SetInSample(true);
+  client->Initialize(prefs.get());
+  EXPECT_EQ(nullptr, client->GetMetricsServiceIfStarted());
+  client->SetHaveMetricsConsent(/* user_consent= */ true,
+                                /* app_consent= */ true);
+  EXPECT_TRUE(client->GetMetricsServiceIfStarted());
+}
+
 }  // namespace metrics

components/metrics/metrics_service.cc

@@ -393,6 +393,7 @@ void MetricsService::RecordCompletedSessionEnd() {
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
 void MetricsService::OnAppEnterBackground(bool keep_recording_in_background) {
+  is_in_foreground_ = false;
   if (!keep_recording_in_background) {
     rotation_scheduler_->Stop();
     reporting_service_.Stop();
@@ -419,6 +420,7 @@ void MetricsService::OnAppEnterBackground(bool keep_recording_in_background) {
 }
 
 void MetricsService::OnAppEnterForeground(bool force_open_new_log) {
+  is_in_foreground_ = true;
   state_manager_->clean_exit_beacon()->WriteBeaconValue(false);
   StartSchedulerIfNecessary();
 

components/metrics/metrics_service.h

@@ -186,6 +186,10 @@ class MetricsService : public base::HistogramFlattener {
     return &delegating_provider_;
   }
 
+#if defined(OS_ANDROID) || defined(OS_IOS)
+  bool IsInForegroundForTesting() const { return is_in_foreground_; }
+#endif
+
  protected:
   // Sets the persistent system profile. Virtual for tests.
   virtual void SetPersistentSystemProfile(const std::string& serialized_proto,
@@ -398,6 +402,12 @@ class MetricsService : public base::HistogramFlattener {
   // Indicates if loading of independent metrics is currently active.
   bool independent_loader_active_ = false;
 
+#if defined(OS_ANDROID) || defined(OS_IOS)
+  // Indicates whether OnAppEnterForeground() (true) or OnAppEnterBackground
+  // (false) was called.
+  bool is_in_foreground_ = false;
+#endif
+
   // Redundant marker to check that we completed our shutdown, and set the
   // exited-cleanly bit in the prefs.
   static ShutdownCleanliness clean_shutdown_status_;

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/RegisterExternalExperimentIdsTest.java

@@ -23,12 +23,9 @@ import org.chromium.weblayer.shell.InstrumentationActivity;
  * Assertions for WebLayer.registerExternalExperimentIDs().
  */
 // The flags are necessary for the following reasons:
-// force-enable-metrics-reporting: forces metrics to be enabled. Without this, whether metrics are
-// enabled depends upon the android version and system. The test needs metrics to be enabled.
 // host-resolver-rules: to make 'google.com' redirect to the port created by TestWebServer.
 // ignore-certificate-errors: TestWebServer doesn't have a real cert.
-@CommandLineFlags.Add({"force-enable-metrics-reporting", "host-resolver-rules='MAP * 127.0.0.1'",
-        "ignore-certificate-errors"})
+@CommandLineFlags.Add({"host-resolver-rules='MAP * 127.0.0.1'", "ignore-certificate-errors"})
 @RunWith(WebLayerJUnit4ClassRunner.class)
 public class RegisterExternalExperimentIdsTest {
     @Rule

weblayer/browser/android/metrics/metrics_browsertest.cc

@@ -11,11 +11,13 @@
 #include "base/test/bind.h"
 #include "components/metrics/log_decoder.h"
 #include "components/metrics/metrics_log_uploader.h"
+#include "components/metrics/metrics_service.h"
 #include "components/metrics/metrics_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h"
 #include "weblayer/browser/android/metrics/metrics_test_helper.h"
 #include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
+#include "weblayer/browser/browser_list.h"
 #include "weblayer/browser/profile_impl.h"
 #include "weblayer/public/navigation_controller.h"
 #include "weblayer/public/profile.h"
@@ -46,7 +48,7 @@ class MetricsBrowserTest : public WebLayerBrowserTest {
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitch(metrics::switches::kForceEnableMetricsReporting);
 
-    InstallTestGmsBridge(HasUserConsent(),
+    InstallTestGmsBridge(GetConsentType(),
                          base::BindRepeating(&MetricsBrowserTest::OnLogMetrics,
                                              base::Unretained(this)));
     WebLayerMetricsServiceClient::GetInstance()->SetFastStartupForTesting(true);
@@ -80,7 +82,7 @@ class MetricsBrowserTest : public WebLayerBrowserTest {
 
   size_t GetNumLogs() const { return metrics_logs_.size(); }
 
-  virtual bool HasUserConsent() { return true; }
+  virtual ConsentType GetConsentType() { return ConsentType::kConsent; }
 
  private:
   std::unique_ptr<Profile> profile_;
@@ -197,7 +199,7 @@ IN_PROC_BROWSER_TEST_F(MetricsBrowserTest, RendererHistograms) {
 }
 
 class MetricsBrowserTestWithUserOptOut : public MetricsBrowserTest {
-  bool HasUserConsent() override { return false; }
+  ConsentType GetConsentType() override { return ConsentType::kNoConsent; }
 };
 
 IN_PROC_BROWSER_TEST_F(MetricsBrowserTestWithUserOptOut, MetricsNotRecorded) {
@@ -205,4 +207,22 @@ IN_PROC_BROWSER_TEST_F(MetricsBrowserTestWithUserOptOut, MetricsNotRecorded) {
   ASSERT_EQ(0u, GetNumLogs());
 }
 
+class MetricsBrowserTestWithConfigurableConsent : public MetricsBrowserTest {
+  ConsentType GetConsentType() override { return ConsentType::kDelayConsent; }
+};
+
+IN_PROC_BROWSER_TEST_F(MetricsBrowserTestWithConfigurableConsent,
+                       IsInForegroundWhenConsentGiven) {
+  // There should be at least one browser which is resumed. This is the trigger
+  // for whether the MetricsService is considered in the foreground.
+  EXPECT_TRUE(BrowserList::GetInstance()->HasAtLeastOneResumedBrowser());
+  RunConsentCallback(true);
+  // RunConsentCallback() should trigger the MetricsService to start.
+  EXPECT_TRUE(WebLayerMetricsServiceClient::GetInstance()
+                  ->GetMetricsServiceIfStarted());
+  EXPECT_TRUE(WebLayerMetricsServiceClient::GetInstance()
+                  ->GetMetricsService()
+                  ->IsInForegroundForTesting());
+}
+
 }  // namespace weblayer

weblayer/browser/android/metrics/metrics_test_helper.cc

@@ -31,11 +31,11 @@ ProfileImpl* GetProfileByName(const std::string& name) {
 
 }  // namespace
 
-void InstallTestGmsBridge(bool has_user_consent,
+void InstallTestGmsBridge(ConsentType consent_type,
                           const OnLogsMetricsCallback on_log_metrics) {
   GetOnLogMetricsCallback() = on_log_metrics;
   Java_MetricsTestHelper_installTestGmsBridge(
-      base::android::AttachCurrentThread(), has_user_consent);
+      base::android::AttachCurrentThread(), static_cast<int>(consent_type));
 }
 
 void RemoveTestGmsBridge() {
@@ -44,6 +44,11 @@ void RemoveTestGmsBridge() {
   GetOnLogMetricsCallback().Reset();
 }
 
+void RunConsentCallback(bool has_consent) {
+  Java_MetricsTestHelper_runConsentCallback(
+      base::android::AttachCurrentThread(), has_consent);
+}
+
 ProfileImpl* CreateProfile(const std::string& name) {
   DCHECK(!GetProfileByName(name));
   JNIEnv* env = base::android::AttachCurrentThread();

weblayer/browser/android/metrics/metrics_test_helper.h

@@ -18,15 +18,29 @@ class ProfileImpl;
 using OnLogsMetricsCallback =
     base::RepeatingCallback<void(metrics::ChromeUserMetricsExtension)>;
 
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.weblayer_private
+// GENERATED_JAVA_CLASS_NAME_OVERRIDE: ConsentType
+enum class ConsentType {
+  kConsent,
+  kNoConsent,
+  // If this is supplied to InstallTestGmsBridge(), the callback used to
+  // determine if consent is given is not automatically called. Use
+  // RunConsentCallback() to apply consent.
+  kDelayConsent,
+};
+
 // Call this in the SetUp() test harness method to install the test
 // GmsBridge and to set the metrics user consent state.
 void InstallTestGmsBridge(
-    bool has_user_consent,
+    ConsentType consent_type,
     const OnLogsMetricsCallback on_log_metrics = OnLogsMetricsCallback());
 
 // Call this in the TearDown() test harness method to remove the GmsBridge.
 void RemoveTestGmsBridge();
 
+// See comment for kDelayConsent.
+void RunConsentCallback(bool has_consent);
+
 // See Profile::Create()'s comments for the semantics of |name|.
 ProfileImpl* CreateProfile(const std::string& name);
 

weblayer/browser/android/metrics/ukm_browsertest.cc

@@ -17,7 +17,8 @@ namespace weblayer {
 class UkmBrowserTest : public WebLayerBrowserTest {
  public:
   void SetUp() override {
-    InstallTestGmsBridge(user_consent_);
+    InstallTestGmsBridge(user_consent_ ? ConsentType::kConsent
+                                       : ConsentType::kNoConsent);
 
     WebLayerBrowserTest::SetUp();
   }

weblayer/browser/android/metrics/weblayer_metrics_service_client.cc

@@ -15,11 +15,13 @@
 #include "components/metrics/metrics_provider.h"
 #include "components/metrics/metrics_service.h"
 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
+#include "components/ukm/ukm_service.h"
 #include "components/variations/variations_ids_provider.h"
 #include "components/version_info/android/channel_getter.h"
 #include "content/public/browser/browser_context.h"
 #include "google_apis/google_api_keys.h"
 #include "weblayer/browser/browser_context_impl.h"
+#include "weblayer/browser/browser_list.h"
 #include "weblayer/browser/java/jni/MetricsServiceClient_jni.h"
 #include "weblayer/browser/system_network_context_manager.h"
 #include "weblayer/browser/tab_impl.h"
@@ -80,9 +82,11 @@ WebLayerMetricsServiceClient* WebLayerMetricsServiceClient::GetInstance() {
 
 WebLayerMetricsServiceClient::WebLayerMetricsServiceClient() {
   ProfileImpl::AddProfileObserver(this);
+  BrowserList::GetInstance()->AddObserver(this);
 }
 
 WebLayerMetricsServiceClient::~WebLayerMetricsServiceClient() {
+  BrowserList::GetInstance()->RemoveObserver(this);
   ProfileImpl::RemoveProfileObserver(this);
 }
 
@@ -141,8 +145,6 @@ void WebLayerMetricsServiceClient::OnMetricsStart() {
     std::move(task).Run();
   }
   post_start_tasks_.clear();
-  GetMetricsService()->synthetic_trial_registry()->AddSyntheticTrialObserver(
-      variations::VariationsIdsProvider::GetInstance());
 }
 
 void WebLayerMetricsServiceClient::OnMetricsNotStarted() {
@@ -176,6 +178,42 @@ WebLayerMetricsServiceClient::GetURLLoaderFactory() {
       ->GetSharedURLLoaderFactory();
 }
 
+void WebLayerMetricsServiceClient::ApplyConsent(bool user_consent,
+                                                bool app_consent) {
+  // TODO(https://crbug.com/1155163): update this once consent can be
+  // dynamically changed.
+  // It is expected this function is called only once, and that prior to this
+  // function the metrics service has not been started. The reason the metric
+  // service should not have been started prior to this function is that the
+  // metrics service is only started if consent is given, and this function is
+  // responsible for setting consent.
+  DCHECK(!GetMetricsServiceIfStarted());
+  // UkmService is only created if consent is given.
+  DCHECK(!GetUkmService());
+
+  SetHaveMetricsConsent(user_consent, app_consent);
+  ApplyForegroundStateToServices();
+}
+
+void WebLayerMetricsServiceClient::ApplyForegroundStateToServices() {
+  const bool is_in_foreground =
+      BrowserList::GetInstance()->HasAtLeastOneResumedBrowser();
+  if (auto* metrics_service = WebLayerMetricsServiceClient::GetInstance()
+                                  ->GetMetricsServiceIfStarted()) {
+    if (is_in_foreground)
+      metrics_service->OnAppEnterForeground();
+    else
+      metrics_service->OnAppEnterBackground();
+  }
+
+  if (auto* ukm_service = GetUkmService()) {
+    if (is_in_foreground)
+      ukm_service->OnAppEnterForeground();
+    else
+      ukm_service->OnAppEnterBackground();
+  }
+}
+
 void WebLayerMetricsServiceClient::ProfileCreated(ProfileImpl* profile) {
   UpdateUkmService();
 }
@@ -184,12 +222,21 @@ void WebLayerMetricsServiceClient::ProfileDestroyed(ProfileImpl* profile) {
   UpdateUkmService();
 }
 
+void WebLayerMetricsServiceClient::OnHasAtLeastOneResumedBrowserStateChanged(
+    bool new_value) {
+  ApplyForegroundStateToServices();
+}
+
+void JNI_ApplyConsentHelper(bool user_consent, bool app_consent) {
+  WebLayerMetricsServiceClient::GetInstance()->ApplyConsent(user_consent,
+                                                            app_consent);
+}
+
 // static
 void JNI_MetricsServiceClient_SetHaveMetricsConsent(JNIEnv* env,
                                                     jboolean user_consent,
                                                     jboolean app_consent) {
-  WebLayerMetricsServiceClient::GetInstance()->SetHaveMetricsConsent(
-      user_consent, app_consent);
+  JNI_ApplyConsentHelper(user_consent, app_consent);
 }
 
 }  // namespace weblayer

weblayer/browser/android/metrics/weblayer_metrics_service_client.h

@@ -18,13 +18,15 @@
 #include "components/metrics/metrics_service_client.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "weblayer/browser/browser_list_observer.h"
 #include "weblayer/browser/profile_impl.h"
 
 namespace weblayer {
 
 class WebLayerMetricsServiceClient
     : public ::metrics::AndroidMetricsServiceClient,
-      public ProfileImpl::ProfileObserver {
+      public ProfileImpl::ProfileObserver,
+      public BrowserListObserver {
   friend class base::NoDestructor<WebLayerMetricsServiceClient>;
 
  public:
@@ -52,10 +54,21 @@ class WebLayerMetricsServiceClient
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
 
  private:
+  friend void JNI_ApplyConsentHelper(bool user_consent, bool app_consent);
+
+  // Called once when consent has been determined.
+  void ApplyConsent(bool user_consent, bool app_consent);
+
+  // Updates the services based on the foreground state.
+  void ApplyForegroundStateToServices();
+
   // ProfileImpl::ProfileObserver:
   void ProfileCreated(ProfileImpl* profile) override;
   void ProfileDestroyed(ProfileImpl* profile) override;
 
+  // BrowserListObserver:
+  void OnHasAtLeastOneResumedBrowserStateChanged(bool new_value) override;
+
   std::vector<base::OnceClosure> post_start_tasks_;
 
   DISALLOW_COPY_AND_ASSIGN(WebLayerMetricsServiceClient);

weblayer/browser/browser_impl.cc

@@ -31,9 +31,6 @@
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/json/json_writer.h"
-#include "components/metrics/metrics_service.h"
-#include "components/ukm/ukm_service.h"
-#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
 #include "weblayer/browser/browser_process.h"
 #include "weblayer/browser/java/jni/BrowserImpl_jni.h"
 #endif
@@ -46,41 +43,6 @@ using base::android::ScopedJavaLocalRef;
 
 namespace weblayer {
 
-namespace {
-
-#if defined(OS_ANDROID)
-void UpdateMetricsService() {
-  static bool s_foreground = false;
-  // TODO(sky): convert this to observer.
-  bool foreground = BrowserList::GetInstance()->HasAtLeastOneResumedBrowser();
-
-  if (foreground == s_foreground)
-    return;
-
-  s_foreground = foreground;
-
-  auto* metrics_service =
-      WebLayerMetricsServiceClient::GetInstance()->GetMetricsService();
-  if (metrics_service) {
-    if (foreground)
-      metrics_service->OnAppEnterForeground();
-    else
-      metrics_service->OnAppEnterBackground();
-  }
-
-  auto* ukm_service =
-      WebLayerMetricsServiceClient::GetInstance()->GetUkmService();
-  if (ukm_service) {
-    if (foreground)
-      ukm_service->OnAppEnterForeground();
-    else
-      ukm_service->OnAppEnterBackground();
-  }
-}
-#endif  // defined(OS_ANDROID)
-
-}  // namespace
-
 // static
 constexpr char BrowserImpl::kPersistenceFilePrefix[];
 
@@ -443,7 +405,6 @@ void BrowserImpl::UpdateFragmentResumedState(bool state) {
   const bool old_has_at_least_one_active_browser =
       BrowserList::GetInstance()->HasAtLeastOneResumedBrowser();
   fragment_resumed_ = state;
-  UpdateMetricsService();
   if (old_has_at_least_one_active_browser !=
       BrowserList::GetInstance()->HasAtLeastOneResumedBrowser()) {
     BrowserList::GetInstance()->NotifyHasAtLeastOneResumedBrowserChanged();

weblayer/browser/browser_list_observer.h

@@ -17,7 +17,7 @@ class BrowserListObserver : public base::CheckedObserver {
 #if defined(OS_ANDROID)
   // Called when the value of BrowserList::HasAtLeastOneResumedBrowser()
   // changes.
-  void OnHasAtLeastOneResumedBrowserStateChanged(bool new_value) {}
+  virtual void OnHasAtLeastOneResumedBrowserStateChanged(bool new_value) {}
 #endif
 
   virtual void OnBrowserCreated(Browser* browser) {}

weblayer/browser/browser_main_parts_impl.cc

@@ -49,11 +49,14 @@
 #include "components/crash/core/common/crash_key.h"
 #include "components/javascript_dialogs/android/app_modal_dialog_view_android.h"  // nogncheck
 #include "components/javascript_dialogs/app_modal_dialog_manager.h"  // nogncheck
+#include "components/metrics/metrics_service.h"
+#include "components/variations/variations_ids_provider.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "net/android/network_change_notifier_factory_android.h"
 #include "net/base/network_change_notifier.h"
 #include "weblayer/browser/android/metrics/uma_utils.h"
+#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
 #include "weblayer/browser/java/jni/MojoInterfaceRegistrar_jni.h"
 #include "weblayer/browser/media/local_presentation_manager_factory.h"
 #include "weblayer/browser/media/media_router_factory.h"
@@ -161,6 +164,18 @@ int BrowserMainPartsImpl::PreCreateThreads() {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         ::switches::kDisableMediaSessionAPI);
   }
+
+  // WebLayer initializes the MetricsService once consent is determined.
+  // Determining consent is async and potentially slow. VariationsIdsProvider
+  // is responsible for updating the X-Client-Data header. To ensure the header
+  // is always provided, VariationsIdsProvider is registered now.
+  //
+  // Chrome registers the VariationsIdsProvider from PreCreateThreads() as well.
+  auto* metrics_client = WebLayerMetricsServiceClient::GetInstance();
+  metrics_client->GetMetricsService()
+      ->synthetic_trial_registry()
+      ->AddSyntheticTrialObserver(
+          variations::VariationsIdsProvider::GetInstance());
 #endif
 
   return content::RESULT_CODE_NORMAL_EXIT;

weblayer/browser/feature_list_creator.cc

@@ -16,10 +16,13 @@
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/common/content_switch_dependent_feature_overrides.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
 #include "weblayer/browser/system_network_context_manager.h"
 #include "weblayer/browser/weblayer_variations_service_client.h"
 
+#if defined(OS_ANDROID)
+#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
+#endif
+
 #if defined(OS_ANDROID)
 namespace switches {
 const char kDisableBackgroundNetworking[] = "disable-background-networking";

weblayer/browser/no_state_prefetch/no_state_prefetch_browsertest.cc

@@ -38,7 +38,7 @@ class NoStatePrefetchBrowserTest : public WebLayerBrowserTest {
  public:
 #if defined(OS_ANDROID)
   void SetUp() override {
-    InstallTestGmsBridge(/* user_consent= */ true);
+    InstallTestGmsBridge(ConsentType::kConsent);
 
     WebLayerBrowserTest::SetUp();
   }

weblayer/browser/profile_impl.cc

@@ -32,7 +32,6 @@
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
-#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
 #include "weblayer/browser/browser_context_impl.h"
 #include "weblayer/browser/browser_impl.h"
 #include "weblayer/browser/browser_list.h"
@@ -52,6 +51,7 @@
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/unified_consent/pref_names.h"
 #include "ui/gfx/android/java_bitmap.h"
+#include "weblayer/browser/android/metrics/weblayer_metrics_service_client.h"
 #include "weblayer/browser/browser_process.h"
 #include "weblayer/browser/java/jni/ProfileImpl_jni.h"
 #include "weblayer/browser/safe_browsing/safe_browsing_service.h"

weblayer/shell/android/browsertests_apk/src/org/chromium/weblayer_private/MetricsTestHelper.java

@@ -21,10 +21,13 @@ import org.chromium.weblayer.WebLayer;
 @JNINamespace("weblayer")
 class MetricsTestHelper {
     private static class TestGmsBridge extends GmsBridge {
-        private final boolean mUserConsent;
+        private final @ConsentType int mConsentType;
+        private Callback<Boolean> mConsentCallback;
+        public static TestGmsBridge sInstance;
 
-        public TestGmsBridge(boolean userConsent) {
-            mUserConsent = userConsent;
+        public TestGmsBridge(@ConsentType int consentType) {
+            sInstance = this;
+            mConsentType = consentType;
         }
 
         @Override
@@ -40,7 +43,11 @@ class MetricsTestHelper {
         @Override
         public void queryMetricsSetting(Callback<Boolean> callback) {
             ThreadUtils.assertOnUiThread();
-            callback.onResult(mUserConsent);
+            if (mConsentType == ConsentType.DELAY_CONSENT) {
+                mConsentCallback = callback;
+            } else {
+                callback.onResult(mConsentType == ConsentType.CONSENT);
+            }
         }
 
         @Override
@@ -50,8 +57,15 @@ class MetricsTestHelper {
     }
 
     @CalledByNative
-    private static void installTestGmsBridge(boolean userConsent) {
-        GmsBridge.injectInstance(new TestGmsBridge(userConsent));
+    private static void installTestGmsBridge(@ConsentType int consentType) {
+        GmsBridge.injectInstance(new TestGmsBridge(consentType));
+    }
+
+    @CalledByNative
+    private static void runConsentCallback(boolean hasConsent) {
+        assert TestGmsBridge.sInstance != null;
+        assert TestGmsBridge.sInstance.mConsentCallback != null;
+        TestGmsBridge.sInstance.mConsentCallback.onResult(hasConsent);
     }
 
     @CalledByNative

weblayer/test/BUILD.gn

@@ -42,6 +42,7 @@ if (is_android) {
       "//content/public/test/android:content_java_test_support",
       "//testing/android/native_test:native_test_java",
       "//third_party/android_deps:android_support_v4_java",
+      "//third_party/android_deps:androidx_annotation_annotation_java",
       "//third_party/android_deps:androidx_core_core_java",
       "//ui/android:ui_java",
       "//weblayer/browser/java",
@@ -49,6 +50,7 @@ if (is_android) {
       "//weblayer/public/java",
       "//weblayer/public/javatestutil:test_java",
     ]
+    srcjar_deps = [ ":generated_enums" ]
 
     annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
   }
@@ -70,6 +72,10 @@ if (is_android) {
       deps += [ "//v8:v8_external_startup_data_assets" ]
     }
   }
+
+  java_cpp_enum("generated_enums") {
+    sources = [ "//weblayer/browser/android/metrics/metrics_test_helper.h" ]
+  }
 }
 
 test("weblayer_browsertests") {