[tracing] Remove unused background tracing category presets.

This CL removes unused background tracing category presets so we
don't have to unnecessarily maintain them. This reduces technical
debt.

Bug: 1157487
Change-Id: I3d2c2eda3fa2ea730cf0cac46936733ea5f19862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586105
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841096}

chrome/browser/tracing/background_tracing_field_trial_unittest.cc

@@ -40,7 +40,7 @@ const char kValidTracingConfig[] = R"(
     "scenario_name": "BrowserProcess",
     "configs": [
       {
-        "category": "BENCHMARK_NAVIGATION",
+        "custom_categories": "base,toplevel",
         "rule": "MONITOR_AND_DUMP_WHEN_SPECIFIC_HISTOGRAM_AND_VALUE",
         "histogram_name": "Omnibox.CharTypedToRepaintLatency.ToPaint",
         "histogram_lower_value": 1

chrome/browser/tracing/background_tracing_metrics_provider_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/tracing/background_tracing_metrics_provider.h"
 
 #include "base/bind.h"
+#include "components/tracing/common/trace_startup_config.h"
 #include "content/public/browser/background_tracing_config.h"
 #include "content/public/browser/background_tracing_manager.h"
 #include "content/public/test/browser_task_environment.h"
@@ -25,7 +26,8 @@ class BackgroundTracingMetricsProviderTest : public testing::Test {
     base::DictionaryValue dict;
 
     dict.SetString("mode", "REACTIVE_TRACING_MODE");
-    dict.SetString("category", "BENCHMARK");
+    dict.SetString("custom_categories",
+                   tracing::TraceStartupConfig::kDefaultStartupCategories);
 
     std::unique_ptr<base::ListValue> rules_list(new base::ListValue());
     {

chrome/browser/tracing/chrome_tracing_delegate_browsertest.cc

@@ -17,6 +17,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "components/tracing/common/trace_startup_config.h"
 #include "components/variations/variations_params_manager.h"
 #include "content/public/browser/background_tracing_config.h"
 #include "content/public/browser/background_tracing_manager.h"
@@ -50,7 +51,8 @@ class ChromeTracingDelegateBrowserTest : public InProcessBrowserTest {
     base::DictionaryValue dict;
 
     dict.SetString("mode", "PREEMPTIVE_TRACING_MODE");
-    dict.SetString("category", "BENCHMARK");
+    dict.SetString("custom_categories",
+                   tracing::TraceStartupConfig::kDefaultStartupCategories);
 
     std::unique_ptr<base::ListValue> rules_list(new base::ListValue());
     {
@@ -253,8 +255,8 @@ class ChromeTracingDelegateBrowserTestOnStartup
     // We need to replace the config JSON with the full one here, as we can't
     // pass JSON through the fieldtrial switch parsing.
     if (config_text == "default_config_for_testing") {
-      return "{\"mode\":\"PREEMPTIVE_TRACING_MODE\", \"category\": "
-             "\"BENCHMARK\",\"configs\": [{\"rule\": "
+      return "{\"mode\":\"PREEMPTIVE_TRACING_MODE\", \"custom_categories\": "
+             "\"base,toplevel\",\"configs\": [{\"rule\": "
              "\"MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED\",\"trigger_name\":"
              "\"test\"}]}";
     }

components/tracing/common/trace_startup_config.cc

@@ -37,16 +37,6 @@ const size_t kTraceConfigFileSizeLimit = 64 * 1024;
 #if defined(OS_ANDROID)
 const base::FilePath::CharType kAndroidTraceConfigFile[] =
     FILE_PATH_LITERAL("/data/local/chrome-trace-config.json");
-
-const char kDefaultStartupCategories[] =
-    "startup,browser,toplevel,toplevel.flow,ipc,EarlyJava,cc,Java,navigation,"
-    "loading,gpu,disabled-by-default-cpu_profiler,download_service,"
-    "disabled-by-default-histogram_samples,"
-    "disabled-by-default-user_action_samples,-*";
-#else
-const char kDefaultStartupCategories[] =
-    "benchmark,toplevel,startup,disabled-by-default-file,toplevel.flow,"
-    "download_service,-*";
 #endif
 
 // String parameters that can be used to parse the trace config file content.
@@ -57,6 +47,18 @@ const char kResultDirectoryParam[] = "result_directory";
 
 }  // namespace
 
+// static
+const char TraceStartupConfig::kDefaultStartupCategories[] =
+#if defined(OS_ANDROID)
+    "startup,browser,toplevel,toplevel.flow,ipc,EarlyJava,cc,Java,navigation,"
+    "loading,gpu,disabled-by-default-cpu_profiler,download_service,"
+    "disabled-by-default-histogram_samples,"
+    "disabled-by-default-user_action_samples,-*";
+#else
+    "benchmark,toplevel,startup,disabled-by-default-file,toplevel.flow,"
+    "download_service,-*";
+#endif
+
 // static
 TraceStartupConfig* TraceStartupConfig::GetInstance() {
   return base::Singleton<TraceStartupConfig, base::DefaultSingletonTraits<

components/tracing/common/trace_startup_config.h

@@ -99,6 +99,9 @@ class TRACING_EXPORT TraceStartupConfig {
     kProto,
   };
 
+  // Exposed for testing.
+  static const char kDefaultStartupCategories[];
+
   static TraceStartupConfig* GetInstance();
 
   // Default minimum startup trace config with enough events to debug issues.

content/browser/BUILD.gn

@@ -1724,8 +1724,6 @@ source_set("browser") {
     "storage_partition_impl_map.h",
     "theme_helper.cc",
     "theme_helper.h",
-    "tracing/background_memory_tracing_observer.cc",
-    "tracing/background_memory_tracing_observer.h",
     "tracing/background_startup_tracing_observer.cc",
     "tracing/background_startup_tracing_observer.h",
     "tracing/background_tracing_active_scenario.cc",

content/browser/tracing/background_memory_tracing_observer.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "content/browser/tracing/background_memory_tracing_observer.h"
-
-#include "base/trace_event/memory_dump_request_args.h"
-#include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
-
-namespace content {
-
-// static
-BackgroundMemoryTracingObserver*
-BackgroundMemoryTracingObserver::GetInstance() {
-  static auto* instance = new BackgroundMemoryTracingObserver();
-  return instance;
-}
-
-BackgroundMemoryTracingObserver::BackgroundMemoryTracingObserver() {}
-BackgroundMemoryTracingObserver::~BackgroundMemoryTracingObserver() {}
-
-void BackgroundMemoryTracingObserver::OnScenarioActivated(
-    const BackgroundTracingConfigImpl* config) {}
-
-void BackgroundMemoryTracingObserver::OnScenarioAborted() {}
-
-void BackgroundMemoryTracingObserver::OnTracingEnabled(
-    BackgroundTracingConfigImpl::CategoryPreset preset) {
-  if (preset !=
-      BackgroundTracingConfigImpl::CategoryPreset::BENCHMARK_MEMORY_LIGHT)
-    return;
-
-  memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDumpAndAppendToTrace(
-          base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
-          base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND,
-          base::trace_event::MemoryDumpDeterminism::NONE,
-          memory_instrumentation::MemoryInstrumentation::
-              RequestGlobalMemoryDumpAndAppendToTraceCallback());
-}
-
-}  // namespace content

content/browser/tracing/background_memory_tracing_observer.h

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CONTENT_BROWSER_TRACING_BACKGROUND_MEMORY_TRACING_OBSERVER_H_
-#define CONTENT_BROWSER_TRACING_BACKGROUND_MEMORY_TRACING_OBSERVER_H_
-
-#include "content/browser/tracing/background_tracing_manager_impl.h"
-
-namespace content {
-
-class CONTENT_EXPORT BackgroundMemoryTracingObserver
-    : public BackgroundTracingManagerImpl::EnabledStateObserver {
- public:
-  static BackgroundMemoryTracingObserver* GetInstance();
-
-  void OnScenarioActivated(const BackgroundTracingConfigImpl* config) override;
-  void OnScenarioAborted() override;
-  void OnTracingEnabled(
-      BackgroundTracingConfigImpl::CategoryPreset preset) override;
-
- private:
-  BackgroundMemoryTracingObserver();
-  ~BackgroundMemoryTracingObserver() override;
-
-  DISALLOW_COPY_AND_ASSIGN(BackgroundMemoryTracingObserver);
-};
-
-}  // namespace content
-
-#endif  // CONTENT_BROWSER_TRACING_BACKGROUND_MEMORY_TRACING_OBSERVER_H_

content/browser/tracing/background_reached_code_tracing_observer_android_unittest.cc

@@ -34,11 +34,11 @@ const BackgroundTracingRule* FindReachedCodeRuleInConfig(
   return nullptr;
 }
 
-std::unique_ptr<BackgroundTracingConfigImpl> GetGpuConfig() {
+std::unique_ptr<BackgroundTracingConfigImpl> GetStartupConfig() {
   auto rules_dict = std::make_unique<base::DictionaryValue>();
   rules_dict->SetString("rule", "MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED");
   rules_dict->SetString("trigger_name", "test");
-  rules_dict->SetString("category", "BENCHMARK_GPU");
+  rules_dict->SetString("category", "BENCHMARK_STARTUP");
   base::DictionaryValue dict;
   auto rules_list = std::make_unique<base::ListValue>();
   rules_list->Append(std::move(rules_dict));
@@ -80,15 +80,15 @@ void TestReachedCodeRuleExists(const BackgroundTracingConfigImpl& config,
   }
 }
 
-void TestGpuConfigExists(const BackgroundTracingConfigImpl& config) {
-  bool found_gpu = false;
+void TestStartupConfigExists(const BackgroundTracingConfigImpl& config) {
+  bool found_startup = false;
   for (const auto& rule : config.rules()) {
     if (rule->category_preset() ==
-        BackgroundTracingConfigImpl::CategoryPreset::BENCHMARK_GPU) {
-      found_gpu = true;
+        BackgroundTracingConfigImpl::CategoryPreset::BENCHMARK_STARTUP) {
+      found_startup = true;
     }
   }
-  EXPECT_TRUE(found_gpu);
+  EXPECT_TRUE(found_startup);
 }
 
 }  // namespace
@@ -106,9 +106,9 @@ TEST(BackgroundReachedCodeTracingObserverTest,
   EXPECT_FALSE(observer.enabled_in_current_session());
   EXPECT_FALSE(base::android::IsReachedCodeProfilerEnabled());
 
-  // A GPU config without preference set should not set preference and keep
+  // A startup config without preference set should not set preference and keep
   // config same.
-  config_impl = GetGpuConfig();
+  config_impl = GetStartupConfig();
   ASSERT_TRUE(config_impl);
 
   EXPECT_FALSE(base::android::IsReachedCodeProfilerEnabled());
@@ -118,7 +118,7 @@ TEST(BackgroundReachedCodeTracingObserverTest,
   EXPECT_FALSE(base::android::IsReachedCodeProfilerEnabled());
   EXPECT_EQ(1u, config_impl->rules().size());
   TestReachedCodeRuleExists(*config_impl, false);
-  TestGpuConfigExists(*config_impl);
+  TestStartupConfigExists(*config_impl);
 
   // A reached code config without profiler should stay config same.
   config_impl = GetReachedCodeConfig();
@@ -149,8 +149,8 @@ TEST(BackgroundReachedCodeTracingObserverTest,
             config_impl->tracing_mode());
   TestReachedCodeRuleExists(*config_impl, true);
 
-  // A GPU config with profiler on should not enabled reached code config.
-  config_impl = GetGpuConfig();
+  // A startup config with profiler on should not enabled reached code config.
+  config_impl = GetStartupConfig();
   config_impl =
       observer.IncludeReachedCodeConfigIfNeeded(std::move(config_impl));
   EXPECT_FALSE(observer.enabled_in_current_session());
@@ -161,7 +161,7 @@ TEST(BackgroundReachedCodeTracingObserverTest,
             config_impl->tracing_mode());
   TestReachedCodeRuleExists(*config_impl, false);
 
-  TestGpuConfigExists(*config_impl);
+  TestStartupConfigExists(*config_impl);
 }
 
 }  // namespace content

content/browser/tracing/background_startup_tracing_observer_unittest.cc

@@ -5,6 +5,7 @@
 #include "content/browser/tracing/background_startup_tracing_observer.h"
 
 #include "build/build_config.h"
+#include "components/tracing/common/trace_startup_config.h"
 #include "content/browser/tracing/background_tracing_rule.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -87,17 +88,18 @@ TEST(BackgroundStartupTracingObserverTest, IncludeStartupConfigIfNeeded) {
   EXPECT_EQ(1u, config_impl->rules().size());
   TestStartupRuleExists(*config_impl, true);
 
-  // A GPU config without preference set should not set preference and keep
+  // A custom config without preference set should not set preference and keep
   // config same.
   std::unique_ptr<base::DictionaryValue> rules_dict(
       new base::DictionaryValue());
   rules_dict->SetString("rule", "MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED");
   rules_dict->SetString("trigger_name", "test");
-  rules_dict->SetString("category", "BENCHMARK_GPU");
   base::DictionaryValue dict;
   std::unique_ptr<base::ListValue> rules_list(new base::ListValue());
   rules_list->Append(std::move(rules_dict));
   dict.Set("configs", std::move(rules_list));
+  dict.SetString("custom_categories",
+                 tracing::TraceStartupConfig::kDefaultStartupCategories);
   config_impl = BackgroundTracingConfigImpl::ReactiveFromDict(&dict);
   ASSERT_TRUE(config_impl);
 
@@ -108,8 +110,8 @@ TEST(BackgroundStartupTracingObserverTest, IncludeStartupConfigIfNeeded) {
   EXPECT_EQ(1u, config_impl->rules().size());
   TestStartupRuleExists(*config_impl, false);
 
-  // A GPU config with preference set should include startup config and disable
-  // preference.
+  // A custom config with preference set should include startup config and
+  // disable preference.
   preferences->SetBackgroundStartupTracingEnabled(true);
   config_impl = observer->IncludeStartupConfigIfNeeded(std::move(config_impl));
   EXPECT_TRUE(observer->enabled_in_current_session());
@@ -119,15 +121,10 @@ TEST(BackgroundStartupTracingObserverTest, IncludeStartupConfigIfNeeded) {
   EXPECT_EQ(BackgroundTracingConfig::TracingMode::REACTIVE,
             config_impl->tracing_mode());
   TestStartupRuleExists(*config_impl, true);
+  EXPECT_EQ(
+      config_impl->category_preset(),
+      BackgroundTracingConfigImpl::CategoryPreset::CUSTOM_CATEGORY_PRESET);
 
-  bool found_gpu = false;
-  for (const auto& rule : config_impl->rules()) {
-    if (rule->category_preset() ==
-        BackgroundTracingConfigImpl::CategoryPreset::BENCHMARK_GPU) {
-      found_gpu = true;
-    }
-  }
-  EXPECT_TRUE(found_gpu);
   preferences->SetBackgroundStartupTracingEnabled(false);
 }
 

content/browser/tracing/background_tracing_config_impl.h

@@ -33,20 +33,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
     CATEGORY_PRESET_UNSET,
     CUSTOM_CATEGORY_PRESET,
     CUSTOM_TRACE_CONFIG,
-    BENCHMARK,
-    BENCHMARK_DEEP,
-    BENCHMARK_GPU,
-    BENCHMARK_IPC,
     BENCHMARK_STARTUP,
-    BENCHMARK_BLINK_GC,
-    BENCHMARK_MEMORY_HEAVY,
-    BENCHMARK_MEMORY_LIGHT,
-    BENCHMARK_EXECUTION_METRIC,
-    BENCHMARK_NAVIGATION,
-    BENCHMARK_RENDERERS,
-    BENCHMARK_SERVICEWORKER,
-    BENCHMARK_POWER,
-    BLINK_STYLE,
   };
 
   CategoryPreset category_preset() const { return category_preset_; }
@@ -118,6 +105,8 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
   void SetBufferSizeLimits(const base::DictionaryValue* dict);
   int GetMaximumTraceBufferSizeKb() const;
 
+  // A trace config extracted from the "trace_config" field of the input
+  // dictionnary.
   base::trace_event::TraceConfig trace_config_;
   CategoryPreset category_preset_;
   std::vector<std::unique_ptr<BackgroundTracingRule>> rules_;

content/browser/tracing/background_tracing_manager_impl.cc

@@ -21,7 +21,6 @@
 #include "build/build_config.h"
 #include "components/tracing/common/trace_startup_config.h"
 #include "components/variations/variations_associated_data.h"
-#include "content/browser/tracing/background_memory_tracing_observer.h"
 #include "content/browser/tracing/background_startup_tracing_observer.h"
 #include "content/browser/tracing/background_tracing_active_scenario.h"
 #include "content/browser/tracing/background_tracing_agent_client_impl.h"
@@ -87,7 +86,6 @@ void BackgroundTracingManagerImpl::ActivateForProcess(
 BackgroundTracingManagerImpl::BackgroundTracingManagerImpl()
     : delegate_(GetContentClient()->browser()->GetTracingDelegate()),
       trigger_handle_ids_(0) {
-  AddEnabledStateObserver(BackgroundMemoryTracingObserver::GetInstance());
   AddEnabledStateObserver(BackgroundStartupTracingObserver::GetInstance());
 #if defined(OS_ANDROID)
   AddEnabledStateObserver(&BackgroundReachedCodeTracingObserver::GetInstance());

services/tracing/public/cpp/trace_event_args_allowlist.cc

@@ -70,7 +70,6 @@ const AllowlistEntry kEventArgsAllowlist[] = {
     {"base", "ScopedBlockingCall*", kScopedBlockingCallAllowedArgs},
     {"base", "ScopedMayLoadLibraryAtBackgroundPriority",
      kScopedBlockingCallAllowedArgs},
-    {"benchmark", "TestAllowlist*", nullptr},
     {"blink", "MemoryPressureListenerRegistry::onMemoryPressure",
      kMemoryPressureEventsAllowedArgs},
     {"browser", "KeyedServiceFactory::GetServiceForContext", nullptr},
@@ -88,7 +87,7 @@ const AllowlistEntry kEventArgsAllowlist[] = {
     {"renderer_host", "*", kRendererHostAllowedArgs},
     {"shutdown", "*", nullptr},
     {"startup", "PrefProvider::PrefProvider", nullptr},
-    {"task_scheduler", "*", nullptr},
+    {"startup", "TestAllowlist*", nullptr},
     {"toplevel", "*", nullptr},
     {"toplevel.ipc", "TaskAnnotator::RunTask", kTopLevelIpcRunTaskAllowedArgs},
     {TRACE_DISABLED_BY_DEFAULT("cpu_profiler"), "*", nullptr},