Reland "Enable a new SYSTEM background tracing mode."

This reverts commit 94d22097.

Reason for revert: Tests on the pixel 2 GPU are now fixed.
See gpu-fyi-try-android-p-pixel-2-skv-32 trybot.

Changes:

1) We now call OnDisconnect if we were already connected when calling
   SetNewSocketForTesting() this fixes gpu-fyi-try-android-p-pixel-2-skv-32

2) GetDefaultTraceConfig() is now in a unnamed namespace in the .cc file rather
   then a member function. This was needed for ubsan_vptr as described in
   crbug/997318

Original change's description:
> Revert "Enable a new SYSTEM background tracing mode."
>
> This reverts commit 9c5df46a.
>
> Reason for revert: timing out tests
>
> Original change's description:
> > Enable a new SYSTEM background tracing mode.
> >
> > This mode will allow us to set up a background scenario and deliver
> > triggers to the system when something interesting is happening. Which
> > will allow us to only upload traces that are of interest.
> >
> > Bug: 979583
> > Change-Id: Ic48ea08e3f9fafccaf045847cc0ddfa76d11611c
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1760736
> > Commit-Queue: Avi Drissman <avi@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Reviewed-by: oysteine <oysteine@chromium.org>
> > Auto-Submit: Stephen Nusko <nuskos@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#689563}
>
> TBR=avi@chromium.org,oysteine@chromium.org,eseckler@chromium.org,nuskos@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 979583,997206
> Change-Id: I51889cecfdc1ac392fb199b1a190b14880801e6d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768981
> Reviewed-by: enne <enne@chromium.org>
> Commit-Queue: enne <enne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#690047}

TBR=avi@chromium.org,enne@chromium.org,oysteine@chromium.org,eseckler@chromium.org,nuskos@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 979583, 997206, 997318
Change-Id: Ib7052fbb7f2b67d85657494ab59d111a58213cdf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768768Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Stephen Nusko <nuskos@chromium.org>
Auto-Submit: Stephen Nusko <nuskos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690883}

content/browser/tracing/background_tracing_active_scenario.cc

@@ -34,7 +34,10 @@ class BackgroundTracingActiveScenario::TracingTimer {
  public:
   TracingTimer(BackgroundTracingActiveScenario* scenario,
                BackgroundTracingManager::StartedFinalizingCallback callback)
-      : scenario_(scenario), callback_(callback) {}
+      : scenario_(scenario), callback_(callback) {
+    DCHECK_NE(scenario->GetConfig()->tracing_mode(),
+              BackgroundTracingConfigImpl::SYSTEM);
+  }
   ~TracingTimer() = default;
 
   void StartTimer(int seconds) {
@@ -299,11 +302,13 @@ void BackgroundTracingActiveScenario::SetState(State new_state) {
     // which means that we're left in a state where the Mojo interface doesn't
     // think we're tracing but TraceLog is still enabled. If that's the case,
     // we abort tracing here.
+    DCHECK_NE(config_->tracing_mode(), BackgroundTracingConfigImpl::SYSTEM);
     base::trace_event::TraceLog::GetInstance()->SetDisabled(
         base::trace_event::TraceLog::GetInstance()->enabled_modes());
   }
 
   if (scenario_state_ == State::kAborted) {
+    DCHECK_NE(config_->tracing_mode(), BackgroundTracingConfigImpl::SYSTEM);
     tracing_session_.reset();
     std::move(on_aborted_callback_).Run();
   }
@@ -334,6 +339,7 @@ void BackgroundTracingActiveScenario::StartTracingIfConfigNeedsIt() {
 }
 
 bool BackgroundTracingActiveScenario::StartTracing() {
+  DCHECK_NE(config_->tracing_mode(), BackgroundTracingConfigImpl::SYSTEM);
   TraceConfig chrome_config = config_->GetTraceConfig();
 
   // If the tracing controller is tracing, i.e. DevTools or about://tracing,
@@ -368,6 +374,7 @@ bool BackgroundTracingActiveScenario::StartTracing() {
 
 void BackgroundTracingActiveScenario::BeginFinalizing(
     BackgroundTracingManager::StartedFinalizingCallback callback) {
+  DCHECK_NE(config_->tracing_mode(), BackgroundTracingConfigImpl::SYSTEM);
   triggered_named_event_handle_ = -1;
   tracing_timer_.reset();
 
@@ -468,6 +475,12 @@ void BackgroundTracingActiveScenario::AbortScenario() {
           }
         },
         weak_ptr_factory_.GetWeakPtr()));
+  } else if (config_->tracing_mode() == BackgroundTracingConfig::SYSTEM) {
+    // We can't 'abort' system tracing since we aren't the consumer. Instead we
+    // send a trigger into the system tracing so that we can tell the time the
+    // scenario stopped.
+    tracing::PerfettoTracedProcess::Get()->ActivateSystemTriggers(
+        {"org.chromium.bg_tracing.scenario_aborted"});
   } else {
     // Setting the kAborted state will cause |this| to be destroyed.
     SetState(State::kAborted);
@@ -528,41 +541,55 @@ void BackgroundTracingActiveScenario::OnRuleTriggered(
 
   int trace_delay = triggered_rule->GetTraceDelay();
 
-  if (config_->tracing_mode() == BackgroundTracingConfigImpl::REACTIVE) {
-    // In reactive mode, a trigger starts tracing, or finalizes tracing
-    // immediately if it's already running.
-    BackgroundTracingManagerImpl::RecordMetric(Metrics::REACTIVE_TRIGGERED);
+  switch (config_->tracing_mode()) {
+    case BackgroundTracingConfigImpl::REACTIVE:
+      // In reactive mode, a trigger starts tracing, or finalizes tracing
+      // immediately if it's already running.
+      BackgroundTracingManagerImpl::RecordMetric(Metrics::REACTIVE_TRIGGERED);
 
-    if (state() != State::kTracing) {
-      // It was not already tracing, start a new trace.
-      if (!StartTracing()) {
-        return;
-      }
-    } else {
-      // Some reactive configs that trigger again while tracing should just
-      // end right away (to not capture multiple navigations, for example).
-      // For others we just want to ignore the repeated trigger.
-      if (triggered_rule->stop_tracing_on_repeated_reactive()) {
-        trace_delay = -1;
+      if (state() != State::kTracing) {
+        // It was not already tracing, start a new trace.
+        if (!StartTracing()) {
+          return;
+        }
       } else {
+        // Some reactive configs that trigger again while tracing should just
+        // end right away (to not capture multiple navigations, for example).
+        // For others we just want to ignore the repeated trigger.
+        if (triggered_rule->stop_tracing_on_repeated_reactive()) {
+          trace_delay = -1;
+        } else {
+          if (!callback.is_null()) {
+            std::move(callback).Run(false);
+          }
+          return;
+        }
+      }
+      break;
+    case BackgroundTracingConfigImpl::SYSTEM:
+      BackgroundTracingManagerImpl::RecordMetric(Metrics::SYSTEM_TRIGGERED);
+      tracing::PerfettoTracedProcess::Get()->ActivateSystemTriggers(
+          {triggered_rule->rule_id()});
+      if (!rule_triggered_callback_for_testing_.is_null()) {
+        rule_triggered_callback_for_testing_.Run();
+      }
+      // We drop |callback| on the floor because we won't know when the system
+      // service starts finalizing the trace and the callback isn't relevant to
+      // this scenario.
+      return;
+    case BackgroundTracingConfigImpl::PREEMPTIVE:
+      // In preemptive mode, a trigger starts finalizing a trace if one is
+      // running and we haven't got a finalization timer running,
+      // otherwise we do nothing.
+      if ((state() != State::kTracing) || tracing_timer_) {
         if (!callback.is_null()) {
           std::move(callback).Run(false);
         }
         return;
       }
-    }
-  } else {
-    // In preemptive mode, a trigger starts finalizing a trace if one is
-    // running and we haven't got a finalization timer running,
-    // otherwise we do nothing.
-    if ((state() != State::kTracing) || tracing_timer_) {
-      if (!callback.is_null()) {
-        std::move(callback).Run(false);
-      }
-      return;
-    }
 
-    BackgroundTracingManagerImpl::RecordMetric(Metrics::PREEMPTIVE_TRIGGERED);
+      BackgroundTracingManagerImpl::RecordMetric(Metrics::PREEMPTIVE_TRIGGERED);
+      break;
   }
 
   if (trace_delay < 0) {

content/browser/tracing/background_tracing_config_impl.cc

@@ -29,6 +29,7 @@ const char kConfigsKey[] = "configs";
 const char kConfigModeKey[] = "mode";
 const char kConfigModePreemptive[] = "PREEMPTIVE_TRACING_MODE";
 const char kConfigModeReactive[] = "REACTIVE_TRACING_MODE";
+const char kConfigModeSystem[] = "SYSTEM_TRACING_MODE";
 
 const char kConfigScenarioName[] = "scenario_name";
 const char kConfigTraceBrowserProcessOnly[] = "trace_browser_process_only";
@@ -211,6 +212,9 @@ void BackgroundTracingConfigImpl::IntoDict(base::DictionaryValue* dict) {
     case BackgroundTracingConfigImpl::REACTIVE:
       dict->SetString(kConfigModeKey, kConfigModeReactive);
       break;
+    case BackgroundTracingConfigImpl::SYSTEM:
+      dict->SetString(kConfigModeKey, kConfigModeSystem);
+      break;
   }
 
   std::unique_ptr<base::ListValue> configs_list(new base::ListValue());
@@ -230,23 +234,23 @@ void BackgroundTracingConfigImpl::IntoDict(base::DictionaryValue* dict) {
 
 void BackgroundTracingConfigImpl::AddPreemptiveRule(
     const base::DictionaryValue* dict) {
-  std::unique_ptr<BackgroundTracingRule> rule =
-      BackgroundTracingRule::CreateRuleFromDict(dict);
-  if (rule)
-    rules_.push_back(std::move(rule));
+  AddRule(dict);
 }
 
 void BackgroundTracingConfigImpl::AddReactiveRule(
     const base::DictionaryValue* dict,
     BackgroundTracingConfigImpl::CategoryPreset category_preset) {
-  std::unique_ptr<BackgroundTracingRule> rule =
-      BackgroundTracingRule::CreateRuleFromDict(dict);
+  BackgroundTracingRule* rule = AddRule(dict);
   if (rule) {
     rule->set_category_preset(category_preset);
-    rules_.push_back(std::move(rule));
   }
 }
 
+void BackgroundTracingConfigImpl::AddSystemRule(
+    const base::DictionaryValue* dict) {
+  AddRule(dict);
+}
+
 TraceConfig BackgroundTracingConfigImpl::GetTraceConfig() const {
   base::trace_event::TraceRecordMode record_mode =
       (tracing_mode() == BackgroundTracingConfigImpl::REACTIVE)
@@ -317,6 +321,8 @@ BackgroundTracingConfigImpl::FromDict(const base::DictionaryValue* dict) {
     config = PreemptiveFromDict(dict);
   } else if (mode == kConfigModeReactive) {
     config = ReactiveFromDict(dict);
+  } else if (mode == kConfigModeSystem) {
+    config = SystemFromDict(dict);
   } else {
     return nullptr;
   }
@@ -434,6 +440,32 @@ BackgroundTracingConfigImpl::ReactiveFromDict(
   return config;
 }
 
+// static
+std::unique_ptr<BackgroundTracingConfigImpl>
+BackgroundTracingConfigImpl::SystemFromDict(const base::DictionaryValue* dict) {
+  DCHECK(dict);
+
+  auto config = std::make_unique<BackgroundTracingConfigImpl>(
+      BackgroundTracingConfigImpl::SYSTEM);
+
+  const base::ListValue* configs_list = nullptr;
+  if (!dict->GetList(kConfigsKey, &configs_list))
+    return nullptr;
+
+  for (const auto& it : *configs_list) {
+    const base::DictionaryValue* config_dict = nullptr;
+    if (!it.GetAsDictionary(&config_dict))
+      return nullptr;
+
+    config->AddSystemRule(config_dict);
+  }
+
+  if (config->rules().empty())
+    return nullptr;
+
+  return config;
+}
+
 // static
 TraceConfig BackgroundTracingConfigImpl::GetConfigForCategoryPreset(
     BackgroundTracingConfigImpl::CategoryPreset preset,
@@ -523,6 +555,17 @@ TraceConfig BackgroundTracingConfigImpl::GetConfigForCategoryPreset(
   return TraceConfig();
 }
 
+BackgroundTracingRule* BackgroundTracingConfigImpl::AddRule(
+    const base::DictionaryValue* dict) {
+  std::unique_ptr<BackgroundTracingRule> rule =
+      BackgroundTracingRule::CreateRuleFromDict(dict);
+  if (rule) {
+    rules_.push_back(std::move(rule));
+    return rules_.back().get();
+  }
+  return nullptr;
+}
+
 void BackgroundTracingConfigImpl::SetBufferSizeLimits(
     const base::DictionaryValue* dict) {
   int value = 0;

content/browser/tracing/background_tracing_config_impl.h

@@ -63,6 +63,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
   void AddReactiveRule(
       const base::DictionaryValue* dict,
       BackgroundTracingConfigImpl::CategoryPreset category_preset);
+  void AddSystemRule(const base::DictionaryValue* dict);
 
   base::trace_event::TraceConfig GetTraceConfig() const;
 
@@ -80,6 +81,8 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
       const base::DictionaryValue* dict);
   static std::unique_ptr<BackgroundTracingConfigImpl> ReactiveFromDict(
       const base::DictionaryValue* dict);
+  static std::unique_ptr<BackgroundTracingConfigImpl> SystemFromDict(
+      const base::DictionaryValue* dict);
 
   static std::unique_ptr<BackgroundTracingConfigImpl> FromDict(
       const base::DictionaryValue* dict);
@@ -108,6 +111,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
       BackgroundTracingConfigImpl::CategoryPreset,
       base::trace_event::TraceRecordMode);
 
+  BackgroundTracingRule* AddRule(const base::DictionaryValue* dict);
   void SetBufferSizeLimits(const base::DictionaryValue* dict);
   int GetMaximumTraceBufferSizeKb() const;
 

content/browser/tracing/background_tracing_manager_impl.cc

@@ -133,9 +133,12 @@ bool BackgroundTracingManagerImpl::SetActiveScenario(
     data_filtering = DataFiltering::ANONYMIZE_DATA;
     RecordMetric(Metrics::STARTUP_SCENARIO_TRIGGERED);
   } else {
-    // If startup config was not set and tracing was enabled, then do not set
-    // any scenario.
-    if (base::trace_event::TraceLog::GetInstance()->IsEnabled()) {
+    // If startup config was not set and we're not a SYSTEM scenario (system
+    // might already have started a trace in the background) but tracing was
+    // enabled, then do not set any scenario.
+    if (base::trace_event::TraceLog::GetInstance()->IsEnabled() &&
+        config_impl &&
+        config_impl->tracing_mode() != BackgroundTracingConfigImpl::SYSTEM) {
       return false;
     }
   }

content/browser/tracing/background_tracing_manager_impl.h

@@ -82,6 +82,7 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
     UPLOAD_SUCCEEDED = 11,
     STARTUP_SCENARIO_TRIGGERED = 12,
     LARGE_UPLOAD_WAITING_TO_RETRY = 13,
+    SYSTEM_TRIGGERED = 14,
     NUMBER_OF_BACKGROUND_TRACING_METRICS,
   };
   static void RecordMetric(Metrics metric);

content/browser/tracing/background_tracing_rule.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/statistics_recorder.h"
 #include "base/rand_util.h"
 #include "base/strings/safe_sprintf.h"
+#include "base/strings/strcat.h"
 #include "base/task/post_task.h"
 #include "base/timer/timer.h"
 #include "base/values.h"
@@ -30,6 +31,7 @@ const char kConfigRuleTriggerChance[] = "trigger_chance";
 const char kConfigRuleStopTracingOnRepeatedReactive[] =
     "stop_tracing_on_repeated_reactive";
 const char kConfigRuleArgsKey[] = "args";
+const char kConfigRuleIdKey[] = "rule_id";
 
 const char kConfigRuleHistogramNameKey[] = "histogram_name";
 const char kConfigRuleHistogramValueOldKey[] = "histogram_value";
@@ -86,6 +88,10 @@ int BackgroundTracingRule::GetTraceDelay() const {
   return trigger_delay_;
 }
 
+std::string BackgroundTracingRule::GetDefaultRuleId() const {
+  return "org.chromium.background_tracing.trigger";
+}
+
 void BackgroundTracingRule::IntoDict(base::DictionaryValue* dict) const {
   DCHECK(dict);
   if (trigger_chance_ < 1.0)
@@ -98,6 +104,9 @@ void BackgroundTracingRule::IntoDict(base::DictionaryValue* dict) const {
     dict->SetBoolean(kConfigRuleStopTracingOnRepeatedReactive,
                      stop_tracing_on_repeated_reactive_);
   }
+  if (rule_id_ != GetDefaultRuleId()) {
+    dict->SetString(kConfigRuleIdKey, rule_id_);
+  }
 
   if (category_preset_ != BackgroundTracingConfigImpl::CATEGORY_PRESET_UNSET) {
     dict->SetString(
@@ -114,6 +123,11 @@ void BackgroundTracingRule::Setup(const base::DictionaryValue* dict) {
   dict->GetInteger(kConfigRuleTriggerDelay, &trigger_delay_);
   dict->GetBoolean(kConfigRuleStopTracingOnRepeatedReactive,
                    &stop_tracing_on_repeated_reactive_);
+  if (dict->HasKey(kConfigRuleIdKey)) {
+    dict->GetString(kConfigRuleIdKey, &rule_id_);
+  } else {
+    rule_id_ = GetDefaultRuleId();
+  }
 }
 
 namespace {
@@ -162,6 +176,11 @@ class NamedTriggerRule : public BackgroundTracingRule {
     return named_event == named_event_;
   }
 
+ protected:
+  std::string GetDefaultRuleId() const override {
+    return base::StrCat({"org.chromium.backgroud_tracing.", named_event_});
+  }
+
  private:
   std::string named_event_;
 };
@@ -311,6 +330,11 @@ class HistogramRule : public BackgroundTracingRule,
     return named_event == histogram_name_;
   }
 
+ protected:
+  std::string GetDefaultRuleId() const override {
+    return base::StrCat({"org.chromium.backgroud_tracing.", histogram_name_});
+  }
+
  private:
   std::string histogram_name_;
   int histogram_lower_value_;
@@ -357,6 +381,11 @@ class TraceForNSOrTriggerOrFullRule : public BackgroundTracingRule {
     return named_event == named_event_;
   }
 
+ protected:
+  std::string GetDefaultRuleId() const override {
+    return base::StrCat({"org.chromium.backgroud_tracing.", named_event_});
+  }
+
  private:
   std::string named_event_;
 };

content/browser/tracing/background_tracing_rule.h

@@ -63,12 +63,18 @@ class CONTENT_EXPORT BackgroundTracingRule {
   }
   const base::DictionaryValue* args() const { return args_.get(); }
 
+  const std::string& rule_id() const { return rule_id_; }
+
+ protected:
+  virtual std::string GetDefaultRuleId() const;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BackgroundTracingRule);
 
   double trigger_chance_;
   int trigger_delay_;
   bool stop_tracing_on_repeated_reactive_;
+  std::string rule_id_;
   BackgroundTracingConfigImpl::CategoryPreset category_preset_;
   std::unique_ptr<base::DictionaryValue> args_;
 };

content/public/browser/background_tracing_config.h

@@ -25,6 +25,9 @@ class CONTENT_EXPORT BackgroundTracingConfig {
   enum TracingMode {
     PREEMPTIVE,
     REACTIVE,
+    // System means that we will inform the system service of triggered rules,
+    // but won't manage the trace ourselves.
+    SYSTEM,
   };
   TracingMode tracing_mode() const { return tracing_mode_; }
 

content/test/BUILD.gn

@@ -1329,6 +1329,7 @@ test("content_browsertests") {
       "//content/shell:content_shell_lib",
       "//content/shell/android:content_shell_assets",
       "//content/shell/android:content_shell_jni_headers",
+      "//services/tracing:test_utils",
       "//testing/android/native_test:native_test_support",
       "//ui/android:android",
       "//ui/touch_selection:touch_selection",

services/tracing/BUILD.gn

@@ -107,6 +107,31 @@ source_set("privacy_check") {
   ]
 }
 
+source_set("test_utils") {
+  testonly = true
+
+  sources = [
+    "perfetto/test_utils.cc",
+    "perfetto/test_utils.h",
+  ]
+
+  deps = [
+    ":lib",
+    "//testing/gtest",
+    "//third_party/perfetto/protos/perfetto/common:lite",
+    "//third_party/perfetto/protos/perfetto/trace:lite",
+  ]
+
+  if (is_android) {
+    sources += [
+      "perfetto/system_test_utils.cc",
+      "perfetto/system_test_utils.h",
+    ]
+
+    deps += [ "//third_party/perfetto/src/tracing:ipc" ]
+  }
+}
+
 source_set("tests") {
   testonly = true
 
@@ -119,8 +144,6 @@ source_set("tests") {
     "perfetto/json_trace_exporter_unittest.cc",
     "perfetto/perfetto_integration_unittest.cc",
     "perfetto/perfetto_tracing_coordinator_unittest.cc",
-    "perfetto/test_utils.cc",
-    "perfetto/test_utils.h",
     "perfetto/track_event_json_exporter_unittest.cc",
     "public/cpp/perfetto/task_runner_unittest.cc",
     "public/cpp/perfetto/trace_event_data_source_unittest.cc",
@@ -138,13 +161,13 @@ source_set("tests") {
 
   deps = [
     ":lib",
+    ":test_utils",
     "//base",
     "//base/test:test_support",
     "//mojo/public/cpp/bindings",
     "//services/service_manager/public/cpp",
     "//services/service_manager/public/cpp/test:test_support",
     "//services/service_manager/public/mojom",
-    "//services/tracing:lib",
     "//testing/gmock",
     "//testing/gtest",
     "//third_party/perfetto/include/perfetto/protozero:protozero",
@@ -156,12 +179,8 @@ source_set("tests") {
   ]
 
   if (is_android) {
-    sources += [
-      "perfetto/system_perfetto_unittest.cc",
-      "perfetto/system_test_utils.cc",
-      "perfetto/system_test_utils.h",
-    ]
-    deps += [ "//third_party/perfetto/src/tracing:ipc" ]
+    sources += [ "perfetto/system_perfetto_unittest.cc" ]
+    deps += [ "//third_party/perfetto/include/perfetto/ext/tracing/ipc" ]
 
     if (can_unwind_with_cfi_table && is_official_build) {
       sources +=

services/tracing/perfetto/system_test_utils.cc

@@ -6,6 +6,7 @@
 
 #include <cstdio>
 
+#include "base/files/scoped_temp_dir.h"
 #include "services/tracing/perfetto/test_utils.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/commit_data_request.h"
@@ -16,22 +17,57 @@ namespace tracing {
 
 MockSystemService::MockSystemService(const std::string& consumer_socket,
                                      const std::string& producer_socket)
-    : consumer_(consumer_socket),
+    : used_tmpdir_(false),
+      consumer_(consumer_socket),
       producer_(producer_socket),
       task_runner_(std::make_unique<PerfettoTaskRunner>(
           base::SequencedTaskRunnerHandle::Get())) {
-  service_ = perfetto::ServiceIPCHost::CreateInstance(task_runner_.get());
-  CHECK(service_);
-  unlink(producer_socket.c_str());
-  unlink(consumer_socket.c_str());
-  bool succeeded = service_->Start(producer_.c_str(), consumer_.c_str());
-  CHECK(succeeded);
+  StartService();
+}
+
+MockSystemService::MockSystemService(const base::ScopedTempDir& tmp_dir)
+    : used_tmpdir_(true),
+      task_runner_(std::make_unique<PerfettoTaskRunner>(
+          base::SequencedTaskRunnerHandle::Get())) {
+  // We need to set TMPDIR environment variable because when a new producer
+  // connects to the perfetto service it needs to create a memmap'd file for
+  // the shared memory buffer. Setting TMPDIR allows the service to know
+  // where this should be.
+  //
+  // Finally since environment variables are leaked into other tests if
+  // multiple tests run we need to restore the value so each test is
+  // hermetic.
+
+  old_tmpdir_ = getenv("TMPDIR");
+  setenv("TMPDIR", tmp_dir.GetPath().value().c_str(), true);
+  // Set up the system socket locations in a valid tmp directory.
+  producer_ = tmp_dir.GetPath().Append(FILE_PATH_LITERAL("producer")).value();
+  consumer_ = tmp_dir.GetPath().Append(FILE_PATH_LITERAL("consumer")).value();
+  StartService();
 }
 
 MockSystemService::~MockSystemService() {
   service_.reset();
   remove(producer().c_str());
   remove(consumer().c_str());
+  if (used_tmpdir_) {
+    if (old_tmpdir_) {
+      // Restore the old value back to its initial value.
+      setenv("TMPDIR", old_tmpdir_, true);
+    } else {
+      // TMPDIR wasn't set originally so unset it.
+      unsetenv("TMPDIR");
+    }
+  }
+}
+
+void MockSystemService::StartService() {
+  service_ = perfetto::ServiceIPCHost::CreateInstance(task_runner_.get());
+  CHECK(service_);
+  unlink(producer_.c_str());
+  unlink(consumer_.c_str());
+  bool succeeded = service_->Start(producer_.c_str(), consumer_.c_str());
+  CHECK(succeeded);
 }
 
 const std::string& MockSystemService::consumer() const {

services/tracing/perfetto/system_test_utils.h

@@ -9,6 +9,10 @@
 
 #include "services/tracing/public/cpp/perfetto/android_system_producer.h"
 
+namespace base {
+class ScopedTempDir;
+}
+
 namespace perfetto {
 class ServiceIPCHost;
 class TracingService;
@@ -23,6 +27,7 @@ class MockSystemService {
  public:
   MockSystemService(const std::string& consumer_socket,
                     const std::string& producer_socket);
+  MockSystemService(const base::ScopedTempDir& tmp_dir);
   ~MockSystemService();
 
   perfetto::TracingService* GetService();
@@ -30,8 +35,12 @@ class MockSystemService {
   const std::string& producer() const;
 
  private:
-  const std::string consumer_;
-  const std::string producer_;
+  void StartService();
+
+  const bool used_tmpdir_;
+  const char* old_tmpdir_ = nullptr;
+  std::string consumer_;
+  std::string producer_;
   std::unique_ptr<perfetto::ServiceIPCHost> service_;
   std::unique_ptr<perfetto::base::TaskRunner> task_runner_;
 };

services/tracing/perfetto/test_utils.cc

@@ -18,7 +18,19 @@
 #include "third_party/perfetto/protos/perfetto/trace/trace_packet.pbzero.h"
 
 namespace tracing {
-
+namespace {
+perfetto::TraceConfig GetDefaultTraceConfig(
+    const std::vector<std::string>& data_sources) {
+  perfetto::TraceConfig trace_config;
+  trace_config.add_buffers()->set_size_kb(1024 * 32);
+  for (const auto& data_source : data_sources) {
+    auto* ds_config = trace_config.add_data_sources()->mutable_config();
+    ds_config->set_name(data_source);
+    ds_config->set_target_buffer(0);
+  }
+  return trace_config;
+}
+}  // namespace
 // static
 std::unique_ptr<TestDataSource> TestDataSource::CreateAndRegisterDataSource(
     const std::string& data_source_name,
@@ -163,7 +175,17 @@ void MockProducerClient::SetAgentDisabledCallback(
 MockConsumer::MockConsumer(std::vector<std::string> data_source_names,
                            perfetto::TracingService* service,
                            PacketReceivedCallback packet_received_callback)
-    : packet_received_callback_(packet_received_callback) {
+    : MockConsumer(data_source_names,
+                   service,
+                   std::move(packet_received_callback),
+                   GetDefaultTraceConfig(data_source_names)) {}
+
+MockConsumer::MockConsumer(std::vector<std::string> data_source_names,
+                           perfetto::TracingService* service,
+                           PacketReceivedCallback packet_received_callback,
+                           const perfetto::TraceConfig& config)
+    : packet_received_callback_(packet_received_callback),
+      trace_config_(config) {
   for (const auto& source : data_source_names) {
     data_sources_.emplace_back(DataSourceStatus{
         source,
@@ -189,16 +211,8 @@ void MockConsumer::StopTracing() {
 }
 
 void MockConsumer::StartTracing() {
-  perfetto::TraceConfig trace_config;
-  trace_config.add_buffers()->set_size_kb(1024 * 32);
-  for (const auto& data_source : data_sources_) {
-    auto* ds_config = trace_config.add_data_sources()->mutable_config();
-    ds_config->set_name(data_source.name);
-    ds_config->set_target_buffer(0);
-  }
-
   CHECK(consumer_endpoint_);
-  consumer_endpoint_->EnableTracing(trace_config);
+  consumer_endpoint_->EnableTracing(trace_config_);
 }
 
 void MockConsumer::FreeBuffers() {
@@ -219,8 +233,9 @@ void MockConsumer::OnTraceData(std::vector<perfetto::TracePacket> packets,
   for (auto& encoded_packet : packets) {
     perfetto::protos::TracePacket packet;
     EXPECT_TRUE(encoded_packet.Decode(&packet));
+    ++received_packets_;
     if (packet.for_testing().str() == kPerfettoTestString) {
-      received_test_packets_++;
+      ++received_test_packets_;
     }
   }
 

services/tracing/perfetto/test_utils.h

@@ -14,6 +14,7 @@
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
 #include "services/tracing/public/cpp/perfetto/producer_client.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/consumer.h"
+#include "third_party/perfetto/include/perfetto/tracing/core/trace_config.h"
 #include "third_party/perfetto/protos/perfetto/common/observable_events.pb.h"
 
 namespace base {
@@ -104,6 +105,10 @@ class MockConsumer : public perfetto::Consumer {
   MockConsumer(std::vector<std::string> data_source_names,
                perfetto::TracingService* service,
                PacketReceivedCallback packet_received_callback);
+  MockConsumer(std::vector<std::string> data_source_names,
+               perfetto::TracingService* service,
+               PacketReceivedCallback packet_received_callback,
+               const perfetto::TraceConfig& config);
   ~MockConsumer() override;
 
   void ReadBuffers();
@@ -114,6 +119,7 @@ class MockConsumer : public perfetto::Consumer {
 
   void FreeBuffers();
 
+  size_t received_packets() const { return received_packets_; }
   size_t received_test_packets() const { return received_test_packets_; }
 
   // perfetto::Consumer implementation
@@ -143,11 +149,13 @@ class MockConsumer : public perfetto::Consumer {
 
   std::unique_ptr<perfetto::TracingService::ConsumerEndpoint>
       consumer_endpoint_;
+  size_t received_packets_ = 0;
   size_t received_test_packets_ = 0;
   PacketReceivedCallback packet_received_callback_;
   std::vector<DataSourceStatus> data_sources_;
   base::RunLoop* on_started_runloop_ = nullptr;
   base::RunLoop* on_stopped_runloop_ = nullptr;
+  perfetto::TraceConfig trace_config_;
 };
 
 class MockProducerHost : public ProducerHost {

services/tracing/public/cpp/perfetto/android_system_producer.cc

@@ -40,7 +40,29 @@ AndroidSystemProducer::~AndroidSystemProducer() {
 }
 
 void AndroidSystemProducer::SetDisallowPreAndroidPieForTesting(bool disallow) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   disallow_pre_android_pie = disallow;
+  if (!disallow && state_ == State::kUninitialized) {
+    // If previously we would not have connected, we now attempt to connect
+    // since we are now skipping a check.
+    Connect();
+  }
+}
+
+void AndroidSystemProducer::SetNewSocketForTesting(const char* socket) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  socket_name_ = socket;
+  if (state_ == State::kConnected) {
+    // If we are fully connected we need to reset the service before we
+    // reconnect.
+    DisconnectWithReply(base::BindOnce(&AndroidSystemProducer::OnDisconnect,
+                                       base::Unretained(this)));
+  } else {
+    // In any other case we just need to do a normal disconnect and
+    // DisconnectWithReply will ensure we set up the retries on the new
+    // |socket|.
+    DisconnectWithReply(base::OnceClosure());
+  }
 }
 
 bool AndroidSystemProducer::IsTracingActive() {
@@ -64,18 +86,6 @@ void AndroidSystemProducer::NewDataSourceAdded(
 void AndroidSystemProducer::DisconnectWithReply(
     base::OnceClosure on_disconnect_complete) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // If we are tracing we need to wait until we're fully disconnected
-  // to run the callback, otherwise we run it immediately (we will
-  // still unregister the data sources but that can happen async in
-  // the background).
-  if (!on_disconnect_complete.is_null()) {
-    if (IsTracingActive() || !on_disconnect_callbacks_.empty()) {
-      on_disconnect_callbacks_.push_back(std::move(on_disconnect_complete));
-    } else {
-      std::move(on_disconnect_complete).Run();
-    }
-  }
-
   if (state_ == State::kConnected) {
     // We are connected and need to unregister the DataSources to
     // inform the service these data sources are going away. If we
@@ -92,6 +102,17 @@ void AndroidSystemProducer::DisconnectWithReply(
       service_->UnregisterDataSource(data_source->name());
     }
   }
+  // If we are tracing we need to wait until we're fully disconnected
+  // to run the callback, otherwise we run it immediately (we will
+  // still unregister the data sources but that can happen async in
+  // the background).
+  if (!on_disconnect_complete.is_null()) {
+    if (IsTracingActive() || !on_disconnect_callbacks_.empty()) {
+      on_disconnect_callbacks_.push_back(std::move(on_disconnect_complete));
+    } else {
+      std::move(on_disconnect_complete).Run();
+    }
+  }
   DelayedReconnect();
 }
 
@@ -328,9 +349,12 @@ AndroidSystemProducer::GetInProcessShmemArbiter() {
   return GetSharedMemoryArbiter();
 }
 
-void AndroidSystemProducer::ActivateTriggers(const std::vector<std::string>&) {
-  // Never called by SharedMemoryArbiter/TraceWriter.
-  NOTREACHED();
+void AndroidSystemProducer::ActivateTriggers(
+    const std::vector<std::string>& triggers) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (state_ == State::kConnected) {
+    service_->ActivateTriggers(triggers);
+  }
 }
 
 void AndroidSystemProducer::ConnectSocket() {

services/tracing/public/cpp/perfetto/android_system_producer.h

@@ -47,6 +47,9 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
   // TODO(nuskos): We need to make this possible for telemetry as well, since
   // they might have side loaded the app.
   void SetDisallowPreAndroidPieForTesting(bool disallow);
+  // |socket| must remain alive as long as AndroidSystemProducer is around
+  // trying to connect to it.
+  void SetNewSocketForTesting(const char* socket);
 
   // PerfettoProducer implementation.
   bool IsTracingActive() override;

services/tracing/public/cpp/perfetto/perfetto_traced_process.cc

@@ -219,6 +219,19 @@ bool PerfettoTracedProcess::CanStartTracing(
   return true;
 }
 
+void PerfettoTracedProcess::ActivateSystemTriggers(
+    const std::vector<std::string>& triggers) {
+  DCHECK(system_producer_endpoint_.get());
+  if (!GetTaskRunner()->GetOrCreateTaskRunner()->RunsTasksInCurrentSequence()) {
+    GetTaskRunner()->GetOrCreateTaskRunner()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&PerfettoTracedProcess::ActivateSystemTriggers,
+                       base::Unretained(this), triggers));
+    return;
+  }
+  system_producer_endpoint_->ActivateTriggers(triggers);
+}
+
 ProducerClient* PerfettoTracedProcess::producer_client() {
   return producer_client_.get();
 }

services/tracing/public/cpp/perfetto/perfetto_traced_process.h

@@ -102,6 +102,8 @@ class COMPONENT_EXPORT(TRACING_CPP) PerfettoTracedProcess final {
   bool CanStartTracing(PerfettoProducer* producer,
                        base::OnceCallback<void()> start_tracing);
 
+  void ActivateSystemTriggers(const std::vector<std::string>& triggers);
+
   // Be careful when using ResetTaskRunnerForTesting. There is a PostTask in the
   // constructor of PerfettoTracedProcess, so before this class is constructed
   // is the only safe time to call this.