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/+/1768981Reviewed-by: enne <enne@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690047}

content/browser/tracing/background_tracing_active_scenario.cc

@@ -34,10 +34,7 @@ class BackgroundTracingActiveScenario::TracingTimer {
  public:
   TracingTimer(BackgroundTracingActiveScenario* scenario,
                BackgroundTracingManager::StartedFinalizingCallback callback)
-      : scenario_(scenario), callback_(callback) {
-    DCHECK_NE(scenario->GetConfig()->tracing_mode(),
-              BackgroundTracingConfigImpl::SYSTEM);
-  }
+      : scenario_(scenario), callback_(callback) {}
   ~TracingTimer() = default;
 
   void StartTimer(int seconds) {
@@ -302,13 +299,11 @@ 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();
   }
@@ -339,7 +334,6 @@ 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,
@@ -383,7 +377,6 @@ bool BackgroundTracingActiveScenario::StartTracing() {
 
 void BackgroundTracingActiveScenario::BeginFinalizing(
     BackgroundTracingManager::StartedFinalizingCallback callback) {
-  DCHECK_NE(config_->tracing_mode(), BackgroundTracingConfigImpl::SYSTEM);
   triggered_named_event_handle_ = -1;
   tracing_timer_.reset();
 
@@ -484,12 +477,6 @@ 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);
@@ -550,55 +537,41 @@ void BackgroundTracingActiveScenario::OnRuleTriggered(
 
   int trace_delay = triggered_rule->GetTraceDelay();
 
-  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 (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);
 
-      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;
-        }
+    if (state() != State::kTracing) {
+      // It was not already tracing, start a new trace.
+      if (!StartTracing()) {
+        return;
       }
-      break;
-    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_) {
+    } 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;
       }
-
-      BackgroundTracingManagerImpl::RecordMetric(Metrics::PREEMPTIVE_TRIGGERED);
-      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();
+    }
+  } 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);
       }
-      // 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;
+    }
+
+    BackgroundTracingManagerImpl::RecordMetric(Metrics::PREEMPTIVE_TRIGGERED);
   }
 
   if (trace_delay < 0) {

content/browser/tracing/background_tracing_config_impl.cc

@@ -29,7 +29,6 @@ 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";
@@ -212,9 +211,6 @@ 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());
@@ -234,23 +230,23 @@ void BackgroundTracingConfigImpl::IntoDict(base::DictionaryValue* dict) {
 
 void BackgroundTracingConfigImpl::AddPreemptiveRule(
     const base::DictionaryValue* dict) {
-  AddRule(dict);
+  std::unique_ptr<BackgroundTracingRule> rule =
+      BackgroundTracingRule::CreateRuleFromDict(dict);
+  if (rule)
+    rules_.push_back(std::move(rule));
 }
 
 void BackgroundTracingConfigImpl::AddReactiveRule(
     const base::DictionaryValue* dict,
     BackgroundTracingConfigImpl::CategoryPreset category_preset) {
-  BackgroundTracingRule* rule = AddRule(dict);
+  std::unique_ptr<BackgroundTracingRule> rule =
+      BackgroundTracingRule::CreateRuleFromDict(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)
@@ -321,8 +317,6 @@ 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;
   }
@@ -440,32 +434,6 @@ 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,
@@ -555,17 +523,6 @@ 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,7 +63,6 @@ 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;
 
@@ -81,8 +80,6 @@ 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);
@@ -111,7 +108,6 @@ 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,12 +133,9 @@ bool BackgroundTracingManagerImpl::SetActiveScenario(
     data_filtering = DataFiltering::ANONYMIZE_DATA;
     RecordMetric(Metrics::STARTUP_SCENARIO_TRIGGERED);
   } else {
-    // 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) {
+    // If startup config was not set and tracing was enabled, then do not set
+    // any scenario.
+    if (base::trace_event::TraceLog::GetInstance()->IsEnabled()) {
       return false;
     }
   }

content/browser/tracing/background_tracing_manager_impl.h

@@ -82,7 +82,6 @@ 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,7 +12,6 @@
 #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"
@@ -31,7 +30,6 @@ 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";
@@ -88,10 +86,6 @@ 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)
@@ -104,9 +98,6 @@ 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(
@@ -123,11 +114,6 @@ 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 {
@@ -176,11 +162,6 @@ 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_;
 };
@@ -330,11 +311,6 @@ 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_;
@@ -381,11 +357,6 @@ 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,18 +63,12 @@ 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,9 +25,6 @@ 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

@@ -1326,7 +1326,6 @@ 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,31 +107,6 @@ 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
 
@@ -144,6 +119,8 @@ 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",
@@ -161,13 +138,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",
@@ -179,8 +156,12 @@ source_set("tests") {
   ]
 
   if (is_android) {
-    sources += [ "perfetto/system_perfetto_unittest.cc" ]
-    deps += [ "//third_party/perfetto/include/perfetto/ext/tracing/ipc" ]
+    sources += [
+      "perfetto/system_perfetto_unittest.cc",
+      "perfetto/system_test_utils.cc",
+      "perfetto/system_test_utils.h",
+    ]
+    deps += [ "//third_party/perfetto/src/tracing:ipc" ]
 
     if (can_unwind_with_cfi_table && is_official_build) {
       sources +=

services/tracing/perfetto/system_test_utils.cc

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

services/tracing/perfetto/test_utils.cc

@@ -163,17 +163,7 @@ void MockProducerClient::SetAgentDisabledCallback(
 MockConsumer::MockConsumer(std::vector<std::string> data_source_names,
                            perfetto::TracingService* service,
                            PacketReceivedCallback 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) {
+    : packet_received_callback_(packet_received_callback) {
   for (const auto& source : data_source_names) {
     data_sources_.emplace_back(DataSourceStatus{
         source,
@@ -199,8 +189,16 @@ 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() {
@@ -221,9 +219,8 @@ 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_++;
     }
   }
 
@@ -295,18 +292,6 @@ void MockConsumer::CheckForAllDataSourcesStopped() {
   }
 }
 
-perfetto::TraceConfig MockConsumer::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;
-}
-
 MockProducerHost::MockProducerHost(
     const std::string& producer_name,
     const std::string& data_source_name,

services/tracing/perfetto/test_utils.h

@@ -14,7 +14,6 @@
 #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 {
@@ -105,10 +104,6 @@ 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();
@@ -119,7 +114,6 @@ 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
@@ -146,18 +140,14 @@ class MockConsumer : public perfetto::Consumer {
 
   void CheckForAllDataSourcesStarted();
   void CheckForAllDataSourcesStopped();
-  perfetto::TraceConfig GetDefaultTraceConfig(
-      const std::vector<std::string>& data_sources);
 
   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,21 +40,7 @@ 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;
-  // Disconnect if we are connected and switch sockets.
-  DisconnectWithReply(base::BindOnce(&AndroidSystemProducer::DelayedReconnect,
-                                     base::Unretained(this)));
 }
 
 bool AndroidSystemProducer::IsTracingActive() {
@@ -342,12 +328,9 @@ AndroidSystemProducer::GetInProcessShmemArbiter() {
   return GetSharedMemoryArbiter();
 }
 
-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::ActivateTriggers(const std::vector<std::string>&) {
+  // Never called by SharedMemoryArbiter/TraceWriter.
+  NOTREACHED();
 }
 
 void AndroidSystemProducer::ConnectSocket() {

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

@@ -47,9 +47,6 @@ 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,19 +219,6 @@ 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,8 +102,6 @@ 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.