Record field trails to the SystemProfile in early startup.

In this CL, we record more information, field trails to the Persistent histogram
data in the reduced mode.

(cherry picked from commit 095f393)

Bug: 965482
Change-Id: I5a5221d2e4c9eeeaeb8f5ace8275921a9aa08b8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742367
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#685318}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746305
Reviewed-by: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/branch-heads/3878@{#3}
Cr-Branched-From: 5ccb96c-refs/heads/master@{#685164}

chrome/android/javatests/src/org/chromium/chrome/browser/ServicificationBackgroundServiceTest.java

@@ -19,7 +19,6 @@
 import org.chromium.base.Log;
 import org.chromium.base.StrictModeContext;
 import org.chromium.base.metrics.RecordHistogram;
-import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.browser.init.ServiceManagerStartupUtils;
@@ -119,7 +118,6 @@ private static void startServiceAndWaitForNative(
     @Test
     @LargeTest
     @Feature({"ServicificationStartup"})
-    @CommandLineFlags.Add({"enable-features=WriteBasicSystemProfileToPersistentHistogramsFile"})
     public void testHistogramsPersistedWithServiceManagerOnlyStart() {
         createBrowserMetricsSpareFile();
         Assert.assertTrue(mSpareFile.exists());

chrome/browser/android/servicification_background_service_jni.cc

@@ -9,10 +9,12 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/system/sys_info.h"
 #include "chrome/android/test_support_jni_headers/ServicificationBackgroundService_jni.h"
 #include "components/metrics/persistent_system_profile.h"
+#include "components/variations/active_field_trials.h"
 #include "third_party/metrics_proto/system_profile.pb.h"
 
 // Verifies that the memory-mapped file for persistent histograms data exists
@@ -70,5 +72,16 @@ JNI_ServicificationBackgroundService_TestPersistentHistogramsOnDiskSystemProfile
   if (!os.has_version())
     return false;
 
-  return base::SysInfo::OperatingSystemVersion().compare(os.version()) == 0;
+  if (base::SysInfo::OperatingSystemVersion().compare(os.version()) != 0)
+    return false;
+
+  std::vector<variations::ActiveGroupId> field_trial_ids;
+  variations::GetFieldTrialActiveGroupIds("", &field_trial_ids);
+
+  int expeceted_size = static_cast<int>(field_trial_ids.size());
+  // The active field trial "PersistentHistograms" is guaranteed in the list.
+  if (expeceted_size <= 0)
+    return false;
+
+  return system_profile_proto.field_trial_size() == expeceted_size;
 }

chrome/browser/metrics/field_trial_synchronizer.cc

@@ -16,6 +16,18 @@
 
 using content::BrowserThread;
 
+namespace {
+
+void AddFieldTrialToPersistentSystemProfile(const std::string& field_trial_name,
+                                            const std::string& group_name) {
+  // Note this in the persistent profile as it will take a while for a new
+  // "complete" profile to be generated.
+  metrics::GlobalPersistentSystemProfile::GetInstance()->AddFieldTrial(
+      field_trial_name, group_name);
+}
+
+}  // namespace
+
 FieldTrialSynchronizer::FieldTrialSynchronizer() {
   bool success = base::FieldTrialList::AddObserver(this);
   // Ensure the observer was actually registered.
@@ -29,10 +41,7 @@ void FieldTrialSynchronizer::NotifyAllRenderers(
   // need to be on the UI thread.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // Note this in the persistent profile as it will take a while for a new
-  // "complete" profile to be genereated.
-  metrics::GlobalPersistentSystemProfile::GetInstance()->AddFieldTrial(
-      field_trial_name, group_name);
+  AddFieldTrialToPersistentSystemProfile(field_trial_name, group_name);
 
   for (content::RenderProcessHost::iterator it(
           content::RenderProcessHost::AllHostsIterator());
@@ -53,9 +62,13 @@ void FieldTrialSynchronizer::OnFieldTrialGroupFinalized(
     const std::string& group_name) {
   // The FieldTrialSynchronizer may have been created before any BrowserThread
   // is created, so we don't need to synchronize with child processes in which
-  // case there are no child processes to notify yet.
-  if (!content::BrowserThread::IsThreadInitialized(BrowserThread::UI))
+  // case there are no child processes to notify yet. But we want to update the
+  // persistent system profile, thus the histogram data recorded in the reduced
+  // mode will be tagged to its corresponding field trial experiment.
+  if (!content::BrowserThread::IsThreadInitialized(BrowserThread::UI)) {
+    AddFieldTrialToPersistentSystemProfile(field_trial_name, group_name);
     return;
+  }
 
   base::PostTask(FROM_HERE, {BrowserThread::UI},
                  base::BindOnce(&FieldTrialSynchronizer::NotifyAllRenderers,

chrome/browser/startup_data.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/metrics/chrome_feature_list_creator.h"
 #include "chrome/browser/prefs/profile_pref_store_manager.h"
 #include "chrome/common/channel_info.h"
+#include "components/metrics/field_trials_provider.h"
 #include "components/metrics/metrics_log.h"
 #include "components/metrics/persistent_system_profile.h"
 #include "components/metrics/version_utils.h"
@@ -72,7 +73,13 @@ void StartupData::RecordCoreSystemProfile() {
       chrome_feature_list_creator_->actual_locale(),
       metrics::GetAppPackageName(), &system_profile);
 
-  // TODO(crbug.com/965482): Records other information, such as field trials.
+  // TODO(hanxi): Create SyntheticTrialRegistry and pass it to
+  // |field_trial_provider|.
+  variations::FieldTrialsProvider field_trial_provider(nullptr,
+                                                       base::StringPiece());
+  field_trial_provider.ProvideSystemProfileMetrics(&system_profile);
+
+  // TODO(crbug.com/965482): Records information from other providers.
   metrics::GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile(
       system_profile, /* complete */ false);
 }

components/metrics/field_trials_provider_unittest.cc

@@ -34,15 +34,21 @@ class TestProvider : public FieldTrialsProvider {
 
 // Check that the values in |system_values| correspond to the test data
 // defined at the top of this file.
-void CheckSystemProfile(const metrics::SystemProfileProto& system_profile) {
-  ASSERT_EQ(base::size(kFieldTrialIds) + base::size(kSyntheticTrials),
-            static_cast<size_t>(system_profile.field_trial_size()));
+void CheckFieldTrialsInSystemProfile(
+    const metrics::SystemProfileProto& system_profile) {
+  // Verify the right data is present for the field trials.
   for (size_t i = 0; i < base::size(kFieldTrialIds); ++i) {
     const metrics::SystemProfileProto::FieldTrial& field_trial =
         system_profile.field_trial(i);
     EXPECT_EQ(kFieldTrialIds[i].name, field_trial.name_id());
     EXPECT_EQ(kFieldTrialIds[i].group, field_trial.group_id());
   }
+}
+
+// Check that the values in |system_values| correspond to the test data
+// defined at the top of this file.
+void CheckSyntheticTrialsInSystemProfile(
+    const metrics::SystemProfileProto& system_profile) {
   // Verify the right data is present for the synthetic trials.
   for (size_t i = 0; i < base::size(kSyntheticTrials); ++i) {
     const metrics::SystemProfileProto::FieldTrial& field_trial =
@@ -98,7 +104,22 @@ TEST_F(FieldTrialsProviderTest, ProvideSyntheticTrials) {
 
   metrics::SystemProfileProto proto;
   provider.ProvideSystemProfileMetrics(&proto);
-  CheckSystemProfile(proto);
+
+  ASSERT_EQ(base::size(kFieldTrialIds) + base::size(kSyntheticTrials),
+            static_cast<size_t>(proto.field_trial_size()));
+  CheckFieldTrialsInSystemProfile(proto);
+  CheckSyntheticTrialsInSystemProfile(proto);
+}
+
+TEST_F(FieldTrialsProviderTest, NoSyntheticTrials) {
+  TestProvider provider(nullptr, base::StringPiece());
+
+  metrics::SystemProfileProto proto;
+  provider.ProvideSystemProfileMetrics(&proto);
+
+  ASSERT_EQ(base::size(kFieldTrialIds),
+            static_cast<size_t>(proto.field_trial_size()));
+  CheckFieldTrialsInSystemProfile(proto);
 }
 
 }  // namespace variations