Stop recording field trial data in activity trace.

The field trial data is exhausting the per-process user data stash.
This makes field trial data incomplete and therefore useless, while
also preventing the use of the per-process user data for other
instrumentation.

Bug: 1003391
Change-Id: I3837076763ba7672040d0223b78978565e472a8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115760Reviewed-by: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753282}

base/debug/activity_tracker.cc

@@ -1623,12 +1623,6 @@ void GlobalActivityTracker::RecordModuleInfo(const ModuleInfo& info) {
   modules_.emplace(info.file, record);
 }
 
-void GlobalActivityTracker::RecordFieldTrial(const std::string& trial_name,
-                                             StringPiece group_name) {
-  const std::string key = std::string("FieldTrial.") + trial_name;
-  process_data_.SetString(key, group_name);
-}
-
 void GlobalActivityTracker::RecordException(const void* pc,
                                             const void* origin,
                                             uint32_t code) {
@@ -1680,12 +1674,6 @@ GlobalActivityTracker::GlobalActivityTracker(
 
   // Note that this process has launched.
   SetProcessPhase(PROCESS_LAUNCHED);
-
-  // Fetch and record all activated field trials.
-  FieldTrial::ActiveGroups active_groups;
-  FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
-  for (auto& group : active_groups)
-    RecordFieldTrial(group.trial_name, group.group_name);
 }
 
 GlobalActivityTracker::~GlobalActivityTracker() {

base/debug/activity_tracker.h

@@ -1046,17 +1046,6 @@ class BASE_EXPORT GlobalActivityTracker {
       tracker->RecordModuleInfo(info);
   }
 
-  // Record field trial information. This call is thread-safe. In addition to
-  // this, construction of a GlobalActivityTracker will cause all existing
-  // active field trials to be fetched and recorded.
-  void RecordFieldTrial(const std::string& trial_name, StringPiece group_name);
-  static void RecordFieldTrialIfEnabled(const std::string& trial_name,
-                                        StringPiece group_name) {
-    GlobalActivityTracker* tracker = Get();
-    if (tracker)
-      tracker->RecordFieldTrial(trial_name, group_name);
-  }
-
   // Record exception information for the current thread.
   ALWAYS_INLINE
   void RecordException(const void* origin, uint32_t code) {

base/metrics/field_trial.cc

@@ -986,14 +986,6 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
     ActivateFieldTrialEntryWhileLocked(field_trial);
   }
 
-  // Recording for stability debugging has to be done inline as a task posted
-  // to an observer may not get executed before a crash.
-  debug::GlobalActivityTracker* tracker = debug::GlobalActivityTracker::Get();
-  if (tracker) {
-    tracker->RecordFieldTrial(field_trial->trial_name(),
-                              field_trial->group_name_internal());
-  }
-
   if (global_->synchronous_observer_) {
     global_->synchronous_observer_->OnFieldTrialGroupFinalized(
         field_trial->trial_name(), field_trial->group_name_internal());

components/browser_watcher/activity_report_extractor.cc

@@ -16,7 +16,6 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/browser_watcher/activity_data_names.h"
-#include "components/variations/active_field_trials.h"
 #include "third_party/crashpad/crashpad/util/misc/uuid.h"
 
 namespace browser_watcher {
@@ -30,8 +29,6 @@ using base::debug::ThreadActivityAnalyzer;
 
 namespace {
 
-const char kFieldTrialKeyPrefix[] = "FieldTrial.";
-
 // Collects stability user data from the recorded format to the collected
 // format.
 void CollectUserData(
@@ -65,20 +62,6 @@ void CollectUserData(
       }
       case ActivityUserData::STRING_VALUE: {
         base::StringPiece value = recorded_value.GetString();
-
-        if (report && base::StartsWith(key, kFieldTrialKeyPrefix,
-                                       base::CompareCase::SENSITIVE)) {
-          // This entry represents an active Field Trial.
-          std::string trial_name =
-              key.substr(std::strlen(kFieldTrialKeyPrefix));
-          variations::ActiveGroupId group_id =
-              variations::MakeActiveGroupId(trial_name, value.as_string());
-          FieldTrial* field_trial = report->add_field_trials();
-          field_trial->set_name_id(group_id.name);
-          field_trial->set_group_id(group_id.group);
-          continue;
-        }
-
         collected_value.set_string_value(value.data(), value.size());
 
         // Promote version information to the global key value store.
@@ -87,6 +70,9 @@ void CollectUserData(
               key == kActivityProduct || key == kActivityChannel ||
               key == kActivityPlatform || key == kActivityVersion;
           if (should_promote) {
+            // TODO(siggi): Copy the promoted data or perhaps remove this
+            //     promotion once the backend is able to display the per-process
+            //     data.
             (*report->mutable_global_data())[key].Swap(&collected_value);
             continue;
           }

components/browser_watcher/activity_report_extractor_unittest.cc

@@ -398,34 +398,6 @@ TEST_F(StabilityReportExtractorTest, ProcessUserDataCollection) {
   EXPECT_EQ(strlen(string2), static_cast<uint64_t>(sref.size()));
 }
 
-TEST_F(StabilityReportExtractorTest, FieldTrialCollection) {
-  // Record some data.
-  GlobalActivityTracker::CreateWithFile(debug_file_path(), kMemorySize, 0ULL,
-                                        "", 3);
-  ActivityUserData& process_data = GlobalActivityTracker::Get()->process_data();
-  process_data.SetString("string", "bar");
-  process_data.SetString("FieldTrial.string", "bar");
-  process_data.SetString("FieldTrial.foo", "bar");
-
-  // Collect the stability report.
-  StabilityReport report;
-  ASSERT_EQ(SUCCESS, Extract(CreateAnalyzer(), &report));
-  ASSERT_EQ(1, report.process_states_size());
-
-  // Validate the report's experiment and global data.
-  ASSERT_EQ(2, report.field_trials_size());
-  EXPECT_NE(0U, report.field_trials(0).name_id());
-  EXPECT_NE(0U, report.field_trials(0).group_id());
-  EXPECT_NE(0U, report.field_trials(1).name_id());
-  EXPECT_EQ(report.field_trials(0).group_id(),
-            report.field_trials(1).group_id());
-
-  // Expect 1 key/value pair.
-  const auto& collected_data = report.process_states(0).data();
-  EXPECT_EQ(kInternalProcessDatums + 1U, collected_data.size());
-  EXPECT_TRUE(base::Contains(collected_data, "string"));
-}
-
 TEST_F(StabilityReportExtractorTest, ModuleCollection) {
   // Record some module information.
   GlobalActivityTracker::CreateWithFile(debug_file_path(), kMemorySize, 0ULL,