[tracing] Move bg tracing config reading into //content

This CL moves functions related to getting background tracing
configuration from field trials out of //chrome into
//content/public/browser (BackgroundTracingManager) so that they can be
reused by other embedders (in this case, android_webview).

Bug: b/170511922
Change-Id: Ia4ffc5f84472d9af00a3b84460d3815b360593eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2473057
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821363}

chrome/browser/tracing/background_tracing_field_trial.cc

@@ -19,7 +19,6 @@
 #include "chrome/browser/tracing/crash_service_uploader.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/tracing/common/tracing_switches.h"
-#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/background_tracing_config.h"
 #include "content/public/browser/background_tracing_manager.h"
 #include "content/public/browser/browser_thread.h"
@@ -32,12 +31,6 @@ namespace tracing {
 
 namespace {
 
-const char kBackgroundTracingFieldTrial[] = "BackgroundTracing";
-const char kBackgroundTracingConfig[] = "config";
-const char kBackgroundTracingUploadUrl[] = "upload_url";
-
-ConfigTextFilterForTesting g_config_text_filter_for_testing = nullptr;
-
 void OnBackgroundTracingUploadComplete(
     TraceCrashServiceUploader* uploader,
     content::BackgroundTracingManager::FinishedProcessingCallback done_callback,
@@ -76,28 +69,6 @@ void BackgroundTracingUploadCallback(
                      std::move(callback)));
 }
 
-std::unique_ptr<content::BackgroundTracingConfig> GetBackgroundTracingConfig() {
-  std::string config_text = variations::GetVariationParamValue(
-      kBackgroundTracingFieldTrial, kBackgroundTracingConfig);
-
-  if (config_text.empty())
-    return nullptr;
-
-  if (g_config_text_filter_for_testing)
-    (*g_config_text_filter_for_testing)(&config_text);
-
-  std::unique_ptr<base::Value> value =
-      base::JSONReader::ReadDeprecated(config_text);
-  if (!value)
-    return nullptr;
-
-  const base::DictionaryValue* dict = nullptr;
-  if (!value->GetAsDictionary(&dict))
-    return nullptr;
-
-  return content::BackgroundTracingConfig::FromDict(dict);
-}
-
 void SetupBackgroundTracingFromConfigFile(const base::FilePath& config_file,
                                           const std::string& upload_url) {
   std::string config_text;
@@ -128,10 +99,6 @@ void SetupBackgroundTracingFromConfigFile(const base::FilePath& config_file,
 
 }  // namespace
 
-void SetConfigTextFilterForTesting(ConfigTextFilterForTesting predicate) {
-  g_config_text_filter_for_testing = predicate;
-}
-
 void SetupBackgroundTracingFieldTrial() {
   auto* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kEnableBackgroundTracing) &&
@@ -143,13 +110,14 @@ void SetupBackgroundTracingFieldTrial() {
   }
 
   std::unique_ptr<content::BackgroundTracingConfig> config =
-      GetBackgroundTracingConfig();
+      content::BackgroundTracingManager::GetInstance()
+          ->GetBackgroundTracingConfig();
 
-  std::string upload_url = variations::GetVariationParamValue(
-      kBackgroundTracingFieldTrial, kBackgroundTracingUploadUrl);
   content::BackgroundTracingManager::GetInstance()->SetActiveScenario(
       std::move(config),
-      base::BindRepeating(&BackgroundTracingUploadCallback, upload_url),
+      base::BindRepeating(&BackgroundTracingUploadCallback,
+                          content::BackgroundTracingManager::GetInstance()
+                              ->GetBackgroundTracingUploadUrl()),
       content::BackgroundTracingManager::ANONYMIZE_DATA);
 }
 

chrome/browser/tracing/background_tracing_field_trial.h

@@ -5,15 +5,10 @@
 #ifndef CHROME_BROWSER_TRACING_BACKGROUND_TRACING_FIELD_TRIAL_H_
 #define CHROME_BROWSER_TRACING_BACKGROUND_TRACING_FIELD_TRIAL_H_
 
-#include <string>
-
 namespace tracing {
 
 void SetupBackgroundTracingFieldTrial();
 
-typedef void (*ConfigTextFilterForTesting)(std::string*);
-void SetConfigTextFilterForTesting(ConfigTextFilterForTesting predicate);
-
 }  // namespace tracing
 
 #endif  // CHROME_BROWSER_TRACING_BACKGROUND_TRACING_FIELD_TRIAL_H_

chrome/browser/tracing/background_tracing_field_trial_unittest.cc

@@ -50,9 +50,10 @@ const char kValidTracingConfig[] = R"(
   }
 )";
 
-void CheckConfig(std::string* config) {
-  if (*config == kTestConfig)
+std::string CheckConfig(const std::string& config) {
+  if (config == kTestConfig)
     g_test_config_loaded = true;
+  return config;
 }
 
 }  // namespace
@@ -71,7 +72,8 @@ TEST_F(BackgroundTracingTest, SetupBackgroundTracingFieldTrial) {
   // In case it is already set at previous test run.
   g_test_config_loaded = false;
 
-  tracing::SetConfigTextFilterForTesting(&CheckConfig);
+  content::BackgroundTracingManager::GetInstance()
+      ->SetConfigTextFilterForTesting(base::BindRepeating(&CheckConfig));
 
   tracing::SetupBackgroundTracingFieldTrial();
   EXPECT_TRUE(g_test_config_loaded);

chrome/browser/tracing/chrome_tracing_delegate.cc

@@ -70,7 +70,11 @@ void ChromeTracingDelegate::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 ChromeTracingDelegate::ChromeTracingDelegate() : incognito_launched_(false) {
-  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  // Ensure that this code is called on the UI thread, except for
+  // tests where a UI thread might not have been initialized at this point.
+  DCHECK(
+      content::BrowserThread::CurrentlyOn(content::BrowserThread::UI) ||
+      !content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI));
 #if !defined(OS_ANDROID)
   BrowserList::AddObserver(this);
 #else

chrome/browser/tracing/chrome_tracing_delegate_browsertest.cc

@@ -10,7 +10,6 @@
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/tracing/background_tracing_field_trial.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/pref_names.h"
@@ -248,25 +247,30 @@ class ChromeTracingDelegateBrowserTestOnStartup
  protected:
   ChromeTracingDelegateBrowserTestOnStartup() {}
 
-  static void FieldTrialConfigTextFilter(std::string* config_text) {
-    ASSERT_TRUE(config_text);
+  static std::string FieldTrialConfigTextFilter(
+      const std::string& config_text) {
     // 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") {
-      *config_text =
-          "{\"mode\":\"PREEMPTIVE_TRACING_MODE\", \"category\": "
-          "\"BENCHMARK\",\"configs\": [{\"rule\": "
-          "\"MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED\",\"trigger_name\":"
-          "\"test\"}]}";
+    if (config_text == "default_config_for_testing") {
+      return "{\"mode\":\"PREEMPTIVE_TRACING_MODE\", \"category\": "
+             "\"BENCHMARK\",\"configs\": [{\"rule\": "
+             "\"MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED\",\"trigger_name\":"
+             "\"test\"}]}";
     }
+    return config_text;
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     variations::testing::VariationParamsManager::AppendVariationParams(
         "BackgroundTracing", "TestGroup",
         {{"config", "default_config_for_testing"}}, command_line);
+  }
 
-    tracing::SetConfigTextFilterForTesting(&FieldTrialConfigTextFilter);
+  void CreatedBrowserMainParts(
+      content::BrowserMainParts* browser_main_parts) override {
+    content::BackgroundTracingManager::GetInstance()
+        ->SetConfigTextFilterForTesting(
+            base::BindRepeating(&FieldTrialConfigTextFilter));
   }
 };
 

content/browser/tracing/background_tracing_manager_impl.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/macros.h"
@@ -19,6 +20,7 @@
 #include "base/values.h"
 #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"
@@ -46,6 +48,10 @@
 
 namespace content {
 
+const char kBackgroundTracingFieldTrial[] = "BackgroundTracing";
+const char kBackgroundTracingConfig[] = "config";
+const char kBackgroundTracingUploadUrl[] = "upload_url";
+
 // static
 void BackgroundTracingManagerImpl::RecordMetric(Metrics metric) {
   UMA_HISTOGRAM_ENUMERATION("Tracing.Background.ScenarioState", metric,
@@ -237,7 +243,11 @@ std::string BackgroundTracingManagerImpl::GetLatestTraceToUpload() {
 
 void BackgroundTracingManagerImpl::AddEnabledStateObserver(
     EnabledStateObserver* observer) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Ensure that this code is called on the UI thread, except for
+  // tests where a UI thread might not have been initialized at this point.
+  DCHECK(
+      content::BrowserThread::CurrentlyOn(content::BrowserThread::UI) ||
+      !content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI));
   background_tracing_observers_.insert(observer);
 }
 
@@ -304,6 +314,11 @@ void BackgroundTracingManagerImpl::SetTraceToUploadForTesting(
   SetTraceToUpload(std::move(trace_data));
 }
 
+void BackgroundTracingManagerImpl::SetConfigTextFilterForTesting(
+    ConfigTextFilterForTesting predicate) {
+  config_text_filter_for_testing_ = std::move(predicate);
+}
+
 void BackgroundTracingManagerImpl::SetTraceToUpload(
     std::unique_ptr<std::string> trace_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -314,6 +329,33 @@ void BackgroundTracingManagerImpl::SetTraceToUpload(
   }
 }
 
+std::string BackgroundTracingManagerImpl::GetBackgroundTracingUploadUrl() {
+  return variations::GetVariationParamValue(kBackgroundTracingFieldTrial,
+                                            kBackgroundTracingUploadUrl);
+}
+
+std::unique_ptr<content::BackgroundTracingConfig>
+BackgroundTracingManagerImpl::GetBackgroundTracingConfig() {
+  std::string config_text = variations::GetVariationParamValue(
+      kBackgroundTracingFieldTrial, kBackgroundTracingConfig);
+  if (config_text.empty())
+    return nullptr;
+
+  if (config_text_filter_for_testing_)
+    config_text = config_text_filter_for_testing_.Run(config_text);
+
+  auto value = base::JSONReader::Read(config_text);
+  if (!value)
+    return nullptr;
+
+  // TODO(crbug.com/646113): use the new base::Value API.
+  const base::DictionaryValue* dict = nullptr;
+  if (!value->GetAsDictionary(&dict))
+    return nullptr;
+
+  return BackgroundTracingConfig::FromDict(dict);
+}
+
 void BackgroundTracingManagerImpl::ValidateStartupScenario() {
   if (!active_scenario_ || !delegate_) {
     return;

content/browser/tracing/background_tracing_manager_impl.h

@@ -115,6 +115,9 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
   bool HasTraceToUpload() override;
   std::string GetLatestTraceToUpload() override;
   void SetTraceToUpload(std::unique_ptr<std::string> trace_data);
+  std::string GetBackgroundTracingUploadUrl() override;
+  std::unique_ptr<BackgroundTracingConfig> GetBackgroundTracingConfig()
+      override;
 
   // Add/remove EnabledStateObserver.
   CONTENT_EXPORT void AddEnabledStateObserver(EnabledStateObserver* observer);
@@ -142,6 +145,8 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
   CONTENT_EXPORT void AbortScenarioForTesting() override;
   CONTENT_EXPORT void SetTraceToUploadForTesting(
       std::unique_ptr<std::string> trace_data) override;
+  void SetConfigTextFilterForTesting(
+      ConfigTextFilterForTesting predicate) override;
 
  private:
   friend class base::NoDestructor<BackgroundTracingManagerImpl>;
@@ -185,6 +190,9 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
   // This field contains serialized trace log proto.
   std::string trace_to_upload_;
 
+  // Callback to override the background tracing config for testing.
+  ConfigTextFilterForTesting config_text_filter_for_testing_;
+
   DISALLOW_COPY_AND_ASSIGN(BackgroundTracingManagerImpl);
 };
 

content/public/browser/background_tracing_manager.h

@@ -100,11 +100,25 @@ class BackgroundTracingManager {
   // the serialized trace proto.
   virtual std::string GetLatestTraceToUpload() = 0;
 
+  // Returns the URL to upload the trace files to.
+  virtual std::string GetBackgroundTracingUploadUrl() = 0;
+
+  // Returns the configuration for background tracing.
+  virtual std::unique_ptr<BackgroundTracingConfig>
+  GetBackgroundTracingConfig() = 0;
+
   // For tests
   virtual void AbortScenarioForTesting() = 0;
   virtual void SetTraceToUploadForTesting(
       std::unique_ptr<std::string> trace_data) = 0;
 
+  using ConfigTextFilterForTesting =
+      base::RepeatingCallback<std::string(const std::string&)>;
+
+  // Sets a callback to override the background tracing config for testing.
+  virtual void SetConfigTextFilterForTesting(
+      ConfigTextFilterForTesting predicate) = 0;
+
  protected:
   virtual ~BackgroundTracingManager() {}
 };