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.

Bug: 965482
Change-Id: I5a5221d2e4c9eeeaeb8f5ace8275921a9aa08b8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742367Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685318}

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

@@ -19,7 +19,6 @@ import org.chromium.base.ContextUtils;
 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 @@ public final class ServicificationBackgroundServiceTest {
     @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

@@ -8,6 +8,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"
@@ -73,7 +74,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