Revert "[tracing] Align --trace-startup and --enable-tracing."

This reverts commit 0823dbae.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 839992 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMDgyM2RiYWVhZDc0YjM0NmE3NWQ2NTNlNjVhYjkwYjAyNzc5ZDNhNgw

Sample Failed Build: https://ci.chromium.org/b/8859069977390233904

Sample Failed Step: content_browsertests

Sample Flaky Test: All/StartupTracingTest.TestEnableTracing/6

Original change's description:
> [tracing] Align --trace-startup and --enable-tracing.
> 
> Use StartupTracingController both for --trace-startup (primary use case:
> browser startup perf profiling) and --enable-tracing (primary use case:
> browser test tracing).
> 
> Due to historical reasons both flags are kept, but now their behaviour
> is mostly the same. If both are present, --trace-startup takes
> precedence.
> 
> Differences:
> - `trace-startup` finishes after 5 seconds by default, while `enable-tracing`
>   continues until the browser is closed. (--trace-startup-duration overrides
>   both).
> - `trace-startup-file` always treats the path as absolute, while if
>   `enable-tracing-output` ends with /, it uses the provided value as the
>   directory and generates the base name based on the current runtime
>   metadata.
> 
> R=skyostil@chromium.org,eseckler@chromium.org
> BUG=1157954
> 
> Change-Id: Ia76316aa8927b431784e2fb94c8e42e4e2c0b479
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589834
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Commit-Queue: Alexander Timin <altimin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#839992}


No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=1157954

Change-Id: I089e53e58934bb503f087b1c8e3ef42c6d3df694
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2609585
Cr-Commit-Position: refs/heads/master@{#839997}

components/tracing/common/trace_startup_config.cc

@@ -88,11 +88,11 @@ TraceStartupConfig::TraceStartupConfig() {
     DCHECK(IsEnabled());
     DCHECK(!IsTracingStartupForDuration());
     DCHECK_EQ(SessionOwner::kBackgroundTracing, session_owner_);
-    CHECK(GetResultFile().empty());
+    CHECK(!ShouldTraceToResultFile());
   } else if (EnableFromATrace()) {
     DCHECK(IsEnabled());
     DCHECK_EQ(SessionOwner::kSystemTracing, session_owner_);
-    CHECK(GetResultFile().empty());
+    CHECK(!ShouldTraceToResultFile());
   }
 }
 
@@ -126,11 +126,20 @@ TraceStartupConfig::OutputFormat TraceStartupConfig::GetOutputFormat() const {
   return output_format_;
 }
 
+bool TraceStartupConfig::ShouldTraceToResultFile() const {
+  return IsEnabled() && should_trace_to_result_file_;
+}
+
 base::FilePath TraceStartupConfig::GetResultFile() const {
   DCHECK(IsEnabled());
+  DCHECK(ShouldTraceToResultFile());
   return result_file_;
 }
 
+void TraceStartupConfig::OnTraceToResultFileFinished() {
+  finished_writing_to_file_ = true;
+}
+
 void TraceStartupConfig::SetBackgroundStartupTracingEnabled(bool enabled) {
 #if defined(OS_ANDROID)
   base::android::SetBackgroundStartupTracingFlag(enabled);
@@ -163,41 +172,25 @@ bool TraceStartupConfig::EnableFromCommandLine() {
                     << "=" << startup_duration_str << " defaulting to 5 (secs)";
       startup_duration_in_seconds_ = kDefaultStartupDurationInSeconds;
     }
-  } else if (command_line->HasSwitch(switches::kEnableTracing)) {
-    // For --enable-tracing, tracing should last until browser shutdown.
-    startup_duration_in_seconds_ = 0;
   }
 
-  if (command_line->HasSwitch(switches::kTraceStartupFormat)) {
-    if (command_line->GetSwitchValueASCII(switches::kTraceStartupFormat) ==
-        "proto") {
-      // Default is "json".
-      output_format_ = OutputFormat::kProto;
-    }
-  } else if (command_line->GetSwitchValueASCII(
-                 switches::kEnableTracingFormat) == "proto") {
+  if (command_line->GetSwitchValueASCII(switches::kTraceStartupFormat) ==
+      "proto") {
+    // Default is "json".
     output_format_ = OutputFormat::kProto;
   }
 
-  if (!command_line->HasSwitch(switches::kTraceStartup) &&
-      !command_line->HasSwitch(switches::kEnableTracing)) {
+  if (!command_line->HasSwitch(switches::kTraceStartup))
     return false;
-  }
-
-  std::string categories;
-  if (command_line->HasSwitch(switches::kTraceStartup)) {
-    categories = command_line->GetSwitchValueASCII(switches::kTraceStartup);
-  } else {
-    categories = command_line->GetSwitchValueASCII(switches::kEnableTracing);
-  }
 
   trace_config_ = base::trace_event::TraceConfig(
-      categories,
+      command_line->GetSwitchValueASCII(switches::kTraceStartup),
       command_line->GetSwitchValueASCII(switches::kTraceStartupRecordMode));
 
   result_file_ = command_line->GetSwitchValuePath(switches::kTraceStartupFile);
 
   is_enabled_ = true;
+  should_trace_to_result_file_ = true;
   return true;
 }
 
@@ -232,6 +225,7 @@ bool TraceStartupConfig::EnableFromConfigFile() {
 
   if (trace_config_file.empty()) {
     is_enabled_ = true;
+    should_trace_to_result_file_ = true;
     DLOG(WARNING) << "Use default trace config.";
     return true;
   }
@@ -251,6 +245,7 @@ bool TraceStartupConfig::EnableFromConfigFile() {
   is_enabled_ = ParseTraceConfigFileContent(trace_config_file_content);
   if (!is_enabled_)
     DLOG(WARNING) << "Cannot parse the trace config file correctly.";
+  should_trace_to_result_file_ = is_enabled_;
   return is_enabled_;
 }
 
@@ -272,6 +267,7 @@ bool TraceStartupConfig::EnableFromBackgroundTracing() {
 
   is_enabled_ = true;
   session_owner_ = SessionOwner::kBackgroundTracing;
+  should_trace_to_result_file_ = false;
   // Set startup duration to 0 since background tracing config will configure
   // the durations later.
   startup_duration_in_seconds_ = 0;

components/tracing/common/trace_startup_config.h

@@ -123,12 +123,21 @@ class TRACING_EXPORT TraceStartupConfig {
   base::trace_event::TraceConfig GetTraceConfig() const;
   int GetStartupDuration() const;
 
-  // Returns the name of the file to write the trace result into.
+  // Returns true while startup tracing is not finished, if trace should be
+  // saved to result file.
+  bool ShouldTraceToResultFile() const;
   base::FilePath GetResultFile() const;
+  void OnTraceToResultFileFinished();
 
   // Set the background tracing config in preferences for the next session.
   void SetBackgroundStartupTracingEnabled(bool enabled);
 
+  // Returns when the startup tracing is finished and written to file, false on
+  // all other cases.
+  bool finished_writing_to_file_for_testing() const {
+    return finished_writing_to_file_;
+  }
+
   SessionOwner GetSessionOwner() const;
 
   OutputFormat GetOutputFormat() const;
@@ -161,7 +170,9 @@ class TRACING_EXPORT TraceStartupConfig {
   bool enable_background_tracing_for_testing_ = false;
   base::trace_event::TraceConfig trace_config_;
   int startup_duration_in_seconds_ = kDefaultStartupDurationInSeconds;
+  bool should_trace_to_result_file_ = false;
   base::FilePath result_file_;
+  bool finished_writing_to_file_ = false;
   SessionOwner session_owner_ = SessionOwner::kTracingController;
   bool session_adopted_ = false;
   OutputFormat output_format_ = OutputFormat::kLegacyJSON;

components/tracing/common/tracing_switches.cc

@@ -18,25 +18,14 @@ const char kTraceConfigFile[]               = "trace-config-file";
 // specify the specific trace categories to include (e.g.
 // --trace-startup=base,net) otherwise, all events are recorded. Setting this
 // flag results in the first call to BeginTracing() to receive all trace events
-// since startup.
-//
-// Historically, --trace-startup was used for browser startup profiling and
-// --enable-tracing was used for browsertest tracing. Now they are share the
-// same implementation, but both are still supported to avoid disrupting
-// existing workflows. The only difference between them is the default duration
-// (5 seconds for trace-startup, unlimited for enable-tracing). If both are
-// specified, 'trace-startup' takes precedence.
-//
-// In Chrome, you may find --trace-startup-file and
+// since startup. In Chrome, you may find --trace-startup-file and
 // --trace-startup-duration to control the auto-saving of the trace (not
 // supported in the base-only TraceLog component).
-const char kTraceStartup[] = "trace-startup";
-const char kEnableTracing[] = "enable-tracing";
+const char kTraceStartup[]                  = "trace-startup";
 
-// Sets the time in seconds until startup tracing ends. If omitted:
-// - if --trace-startup is specified, a default of 5 seconds is used.
-// - if --enable-tracing is specified, tracing lasts until the browser is
-// closed. Has no effect otherwise.
+// Sets the time in seconds until startup tracing ends. If omitted a default of
+// 5 seconds is used. Has no effect without --trace-startup, or if
+// --startup-trace-file=none was supplied.
 const char kTraceStartupDuration[]          = "trace-startup-duration";
 
 // If supplied, sets the file which startup tracing will be stored into, if
@@ -46,16 +35,7 @@ const char kTraceStartupDuration[]          = "trace-startup-duration";
 // As a special case, can be set to 'none' - this disables automatically saving
 // the result to a file and the first manually recorded trace will then receive
 // all events since startup.
-const char kTraceStartupFile[] = "trace-startup-file";
-
-// Similar to the flag above, with the following differences:
-// - A more detailed basename will be generated.
-// - If the value is empty or ends with path separator, the provided directory
-// will be used (with empty standing for current directory) and a detailed
-// basename file will be generated.
-//
-// It is ignored if --trace-startup-file is specified.
-const char kEnableTracingOutput[] = "enable-tracing-output";
+const char kTraceStartupFile[]              = "trace-startup-file";
 
 // Sets the output format for the trace, valid values are "json" and "proto".
 // If not set, the current default is "json".
@@ -64,7 +44,6 @@ const char kEnableTracingOutput[] = "enable-tracing-output";
 // unexpectedly terminates.
 // Ignored if "trace-startup-owner" is not "controller".
 const char kTraceStartupFormat[] = "trace-startup-format";
-const char kEnableTracingFormat[] = "enable-tracing-format";
 
 // If supplied, sets the tracing record mode and options; otherwise, the default
 // "record-until-full" mode will be used.

components/tracing/common/tracing_switches.h

@@ -12,13 +12,10 @@ namespace switches {
 TRACING_EXPORT extern const char kEnableBackgroundTracing[];
 TRACING_EXPORT extern const char kTraceConfigFile[];
 TRACING_EXPORT extern const char kTraceStartup[];
-TRACING_EXPORT extern const char kEnableTracing[];
 TRACING_EXPORT extern const char kTraceStartupDuration[];
 TRACING_EXPORT extern const char kTraceStartupFile[];
-TRACING_EXPORT extern const char kEnableTracingOutput[];
 TRACING_EXPORT extern const char kTraceStartupRecordMode[];
 TRACING_EXPORT extern const char kTraceStartupFormat[];
-TRACING_EXPORT extern const char kEnableTracingFormat[];
 TRACING_EXPORT extern const char kTraceStartupOwner[];
 TRACING_EXPORT extern const char kTraceStartupEnablePrivacyFiltering[];
 TRACING_EXPORT extern const char kPerfettoDisableInterning[];

content/browser/tracing/startup_tracing_browsertest.cc

@@ -12,7 +12,6 @@
 #include "build/build_config.h"
 #include "components/tracing/common/trace_startup_config.h"
 #include "components/tracing/common/tracing_switches.h"
-#include "content/browser/tracing/startup_tracing_controller.h"
 #include "content/browser/tracing/tracing_controller_impl.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/content_browser_test.h"
@@ -43,6 +42,52 @@ void WaitForCondition(base::RepeatingCallback<bool()> condition,
 
 }  // namespace
 
+class CommandlineStartupTracingTest : public ContentBrowserTest {
+ public:
+  CommandlineStartupTracingTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    base::CreateTemporaryFile(&temp_file_path_);
+    command_line->AppendSwitch(switches::kTraceStartup);
+    command_line->AppendSwitchASCII(switches::kTraceStartupDuration, "3");
+    command_line->AppendSwitchASCII(switches::kTraceStartupFile,
+                                    temp_file_path_.AsUTF8Unsafe());
+  }
+
+ protected:
+  base::FilePath temp_file_path_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CommandlineStartupTracingTest);
+};
+
+// Failing on Android/Win ASAN, Linux TSAN. crbug.com/1041392
+#if (defined(OS_ANDROID) && defined(ADDRESS_SANITIZER)) || \
+    (defined(OS_WIN) && defined(ADDRESS_SANITIZER)) ||     \
+    ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(THREAD_SANITIZER))
+#define MAYBE_TestStartupTracing DISABLED_TestStartupTracing
+#else
+#define MAYBE_TestStartupTracing TestStartupTracing
+#endif
+IN_PROC_BROWSER_TEST_F(CommandlineStartupTracingTest,
+                       MAYBE_TestStartupTracing) {
+  EXPECT_TRUE(NavigateToURL(shell(), GetTestUrl("", "title1.html")));
+  WaitForCondition(base::BindRepeating([]() {
+                     return tracing::TraceStartupConfig::GetInstance()
+                         ->finished_writing_to_file_for_testing();
+                   }),
+                   "finish file write");
+
+  std::string trace;
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  ASSERT_TRUE(base::ReadFileToString(temp_file_path_, &trace));
+  EXPECT_TRUE(base::JSONReader::Read(trace));
+  EXPECT_TRUE(trace.find("StartupTracingController::Start") !=
+              std::string::npos);
+}
+
+#undef MAYBE_TestStartupTracing
+
 class StartupTracingInProcessTest : public ContentBrowserTest {
  public:
   StartupTracingInProcessTest() {
@@ -109,210 +154,4 @@ IN_PROC_BROWSER_TEST_F(StartupTracingInProcessTest, TestFilledStartupBuffer) {
   wait_for_stop.Run();
 }
 
-namespace {
-
-enum class FinishType {
-  kWaitForTimeout,
-  kStopExplicitly,
-};
-
-std::ostream& operator<<(std::ostream& o, FinishType type) {
-  switch (type) {
-    case FinishType::kStopExplicitly:
-      o << "Stop";
-      return o;
-    case FinishType::kWaitForTimeout:
-      o << "Wait";
-      return o;
-  }
-}
-
-enum class OutputType {
-  kProto,
-  kJSON,
-};
-
-std::ostream& operator<<(std::ostream& o, OutputType type) {
-  switch (type) {
-    case OutputType::kJSON:
-      o << "json";
-      return o;
-    case OutputType::kProto:
-      o << "proto";
-      return o;
-  }
-}
-
-enum class OutputLocation {
-  // Write trace to a given file.
-  kGivenFile,
-  // Write trace into a given directory (basename will be set to trace1 before
-  // starting).
-  kDirectoryWithDefaultBasename,
-  // Write trace into a given directory (basename will be set to trace1 before
-  // starting, and updated to trace2 before calling Stop()).
-  kDirectoryWithBasenameUpdatedBeforeStop,
-};
-
-std::ostream& operator<<(std::ostream& o, OutputLocation type) {
-  switch (type) {
-    case OutputLocation::kGivenFile:
-      o << "file";
-      return o;
-    case OutputLocation::kDirectoryWithDefaultBasename:
-      o << "dir/trace1";
-      return o;
-    case OutputLocation::kDirectoryWithBasenameUpdatedBeforeStop:
-      o << "dir/trace2";
-      return o;
-  }
-}
-
-}  // namespace
-
-class StartupTracingTest
-    : public ContentBrowserTest,
-      public testing::WithParamInterface<
-          std::tuple<FinishType, OutputType, OutputLocation>> {
- public:
-  StartupTracingTest() = default;
-
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    command_line->AppendSwitch(switches::kTraceStartup);
-    if (GetFinishType() == FinishType::kWaitForTimeout) {
-      command_line->AppendSwitchASCII(switches::kTraceStartupDuration, "3");
-    } else {
-      command_line->AppendSwitchASCII(switches::kTraceStartupDuration, "0");
-    }
-    command_line->AppendSwitchASCII(switches::kTraceStartupFormat,
-                                    GetOutputTypeAsString());
-
-    if (GetOutputLocation() == OutputLocation::kGivenFile) {
-      base::CreateTemporaryFile(&temp_file_path_);
-    } else {
-      base::CreateNewTempDirectory(base::FilePath::StringType(),
-                                   &temp_file_path_);
-      temp_file_path_ = temp_file_path_.AsEndingWithSeparator();
-    }
-
-    command_line->AppendSwitchASCII(switches::kEnableTracingOutput,
-                                    temp_file_path_.AsUTF8Unsafe());
-
-    if (GetOutputLocation() != OutputLocation::kGivenFile) {
-      // --enable-tracing-format switch should be initialised before
-      // calling SetDefaultBasenameForTest, which forces the creation of
-      // TraceStartupConfig, which queries the command line flags and
-      // stores the snapshot.
-      StartupTracingController::GetInstance().SetDefaultBasenameForTest(
-          "trace1",
-          StartupTracingController::ExtensionType::kAppendAppropriate);
-    }
-  }
-
-  FinishType GetFinishType() { return std::get<0>(GetParam()); }
-
-  OutputType GetOutputType() { return std::get<1>(GetParam()); }
-
-  std::string GetOutputTypeAsString() {
-    switch (GetOutputType()) {
-      case OutputType::kJSON:
-        return "json";
-      case OutputType::kProto:
-        return "proto";
-    }
-  }
-
-  OutputLocation GetOutputLocation() { return std::get<2>(GetParam()); }
-
-  base::FilePath GetExpectedPath() {
-    std::string filename;
-
-    switch (GetOutputLocation()) {
-      case OutputLocation::kGivenFile:
-        return temp_file_path_;
-      case OutputLocation::kDirectoryWithDefaultBasename:
-        filename = "trace1";
-        break;
-      case OutputLocation::kDirectoryWithBasenameUpdatedBeforeStop:
-        filename = "trace2";
-        break;
-    }
-
-    // Renames are not supported together with timeouts.
-    if (GetFinishType() == FinishType::kWaitForTimeout)
-      filename = "trace1";
-
-    return temp_file_path_.AppendASCII(filename + "." +
-                                       GetOutputTypeAsString());
-  }
-
-  void CheckOutput(base::FilePath path) {
-    std::string trace;
-    base::ScopedAllowBlockingForTesting allow_blocking;
-    ASSERT_TRUE(base::ReadFileToString(path, &trace))
-        << "Failed to read file " << path;
-
-    if (GetOutputType() == OutputType::kJSON) {
-      EXPECT_TRUE(base::JSONReader::Read(trace));
-    }
-
-    // Both proto and json should have the trace event name recorded somewhere
-    // as a substring.
-    EXPECT_TRUE(trace.find("StartupTracingController::Start") !=
-                std::string::npos);
-  }
-
-  void Wait() {
-    if (GetFinishType() == FinishType::kWaitForTimeout) {
-      WaitForCondition(base::BindRepeating([]() {
-                         return StartupTracingController::GetInstance()
-                             .is_finished_for_testing();
-                       }),
-                       "finish file write");
-    } else {
-      StartupTracingController::GetInstance().WaitUntilStopped();
-    }
-  }
-
- protected:
-  base::FilePath temp_file_path_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(StartupTracingTest);
-};
-
-INSTANTIATE_TEST_SUITE_P(
-    All,
-    StartupTracingTest,
-    testing::Combine(
-        testing::Values(FinishType::kStopExplicitly,
-                        FinishType::kWaitForTimeout),
-        testing::Values(OutputType::kJSON, OutputType::kProto),
-        testing::Values(
-            OutputLocation::kGivenFile,
-            OutputLocation::kDirectoryWithDefaultBasename,
-            OutputLocation::kDirectoryWithBasenameUpdatedBeforeStop)));
-
-// Failing on Android/Win ASAN, Linux TSAN. crbug.com/1041392
-#if (defined(OS_ANDROID) && defined(ADDRESS_SANITIZER)) || \
-    (defined(OS_WIN) && defined(ADDRESS_SANITIZER)) ||     \
-    ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(THREAD_SANITIZER))
-#define MAYBE_TestEnableTracing DISABLED_TestStartupTracing
-#else
-#define MAYBE_TestEnableTracing TestStartupTracing
-#endif
-IN_PROC_BROWSER_TEST_P(StartupTracingTest, TestEnableTracing) {
-  EXPECT_TRUE(NavigateToURL(shell(), GetTestUrl("", "title1.html")));
-
-  if (GetOutputLocation() ==
-      OutputLocation::kDirectoryWithBasenameUpdatedBeforeStop) {
-    StartupTracingController::GetInstance().SetDefaultBasenameForTest(
-        "trace2", StartupTracingController::ExtensionType::kAppendAppropriate);
-  }
-
-  Wait();
-
-  CheckOutput(GetExpectedPath());
-}
-
 }  // namespace content

content/browser/tracing/startup_tracing_controller.h

@@ -6,14 +6,13 @@
 #define CONTENT_BROWSER_TRACING_STARTUP_TRACING_CONTROLLER_H_
 
 #include "base/threading/sequence_bound.h"
-#include "content/common/content_export.h"
 
 namespace content {
 
 // Class responsible for starting and stopping startup tracing as configured by
 // StartupTracingConfig. All interactions with it are limited to UI thread, but
 // the actual logic lives on a background ThreadPool sequence.
-class CONTENT_EXPORT StartupTracingController {
+class StartupTracingController {
  public:
   StartupTracingController();
   ~StartupTracingController();
@@ -23,55 +22,16 @@ class CONTENT_EXPORT StartupTracingController {
   void StartIfNeeded();
   void WaitUntilStopped();
 
-  // By default, a trace is written into a temporary file which then is renamed,
-  // however this can lead to data loss when the browser process crashes.
-  // Embedders can disable this (especially if a name provided to
-  // SetDefaultBasename makes it clear that the trace is incomplete and final
-  // name will be provided via SetDefaultBasename call before calling Stop).
-  enum class TempFilePolicy {
-    kUseTemporaryFile,
-    kWriteDirectly,
-  };
-  void SetUsingTemporaryFile(TempFilePolicy temp_file_policy);
-
-  // Set default basename for the trace output file to allow //content embedders
-  // to customise it using some metadata (like test names).
-  //
-  // If --enable-trace-output is a directory (default value, empty, designated
-  // "current directory"), then the startup trace will be written in a file with
-  // the given basename in this directory. Depending on the |extension_type|,
-  // an appropriate extension (.json or .proto) will be added.
-  //
-  // Note that embedders can call it even after tracing has started and Perfetto
-  // started streaming the trace into it — in that case,
-  // StartupTracingController will rename the file after finishing. However,
-  // this is guaranteed to work only when tracing lasts until Stop() (not with
-  // duration-based tracing).
-  enum class ExtensionType {
-    kAppendAppropriate,
-    kNone,
-  };
-  void SetDefaultBasename(std::string basename, ExtensionType extension_type);
-  // As the test harness calls SetDefaultBasename, expose ForTest() version for
-  // the tests checking the StartupTracingController logic itself.
-  void SetDefaultBasenameForTest(std::string basename,
-                                 ExtensionType extension_type);
-
-  bool is_finished_for_testing() const { return state_ == State::kStopped; }
-
  private:
   void Stop(base::OnceClosure on_finished_callback);
 
   void OnStoppedOnUIThread();
 
-  base::FilePath GetOutputPath();
-
   enum class State {
-    kNotEnabled,
     kRunning,
-    kStopped,
+    kNotRunning,
   };
-  State state_ = State::kNotEnabled;
+  State state_ = State::kNotRunning;
 
   // All actual interactions with the tracing service and the process of writing
   // files happens on a background thread.
@@ -80,11 +40,6 @@ class CONTENT_EXPORT StartupTracingController {
 
   base::OnceClosure on_tracing_finished_;
   base::FilePath output_file_;
-
-  std::string default_basename_;
-  bool basename_for_test_set_ = false;
-
-  TempFilePolicy temp_file_policy_ = TempFilePolicy::kUseTemporaryFile;
 };
 
 }  // namespace content

content/public/common/content_switches.cc

@@ -402,6 +402,14 @@ const char kEnableStrictPowerfulFeatureRestrictions[] =
 // Enabled threaded compositing for web tests.
 const char kEnableThreadedCompositing[]     = "enable-threaded-compositing";
 
+// Enable tracing during the execution of browser tests.
+const char kEnableTracing[]                 = "enable-tracing";
+
+// The filename to write the output of the test tracing to. If it is empty
+// or it ends in a directory separator then an auto-generated filename will be
+// appended.
+const char kEnableTracingOutput[]           = "enable-tracing-output";
+
 // Enable screen capturing support for MediaStream API.
 const char kEnableUserMediaScreenCapturing[] =
     "enable-usermedia-screen-capturing";

content/public/common/content_switches.h

@@ -123,6 +123,8 @@ CONTENT_EXPORT extern const char kEnableSpatialNavigation[];
 CONTENT_EXPORT extern const char kEnableStrictMixedContentChecking[];
 CONTENT_EXPORT extern const char kEnableStrictPowerfulFeatureRestrictions[];
 CONTENT_EXPORT extern const char kEnableThreadedCompositing[];
+CONTENT_EXPORT extern const char kEnableTracing[];
+CONTENT_EXPORT extern const char kEnableTracingOutput[];
 CONTENT_EXPORT extern const char kEnableUserMediaScreenCapturing[];
 CONTENT_EXPORT extern const char kEnableUseZoomForDSF[];
 CONTENT_EXPORT extern const char kEnableViewport[];

content/public/test/browser_test_base.cc

@@ -46,7 +46,6 @@
 #include "content/browser/startup_helper.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/browser/tracing/memory_instrumentation_util.h"
-#include "content/browser/tracing/startup_tracing_controller.h"
 #include "content/browser/tracing/tracing_controller_impl.h"
 #include "content/public/app/content_main.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -167,47 +166,10 @@ void RunTaskOnRendererThread(base::OnceClosure task,
   GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(quit_task));
 }
 
-enum class TraceBasenameType {
-  kWithoutTestStatus,
-  kWithTestStatus,
-};
-
-std::string GetDefaultTraceBasename(TraceBasenameType type) {
-  std::string test_suite_name = ::testing::UnitTest::GetInstance()
-                                    ->current_test_info()
-                                    ->test_suite_name();
-  std::string test_name =
-      ::testing::UnitTest::GetInstance()->current_test_info()->name();
-  // Parameterised tests might have slashes in their full name — replace them
-  // before using it as a file name to avoid trying to write to an incorrect
-  // location.
-  base::ReplaceChars(test_suite_name, "/", "_", &test_suite_name);
-  base::ReplaceChars(test_name, "/", "_", &test_name);
-  // Add random number to the trace file to distinguish traces from different
-  // test runs. We don't use timestamp here to avoid collisions with parallel
-  // runs of the same test. Browser test runner runs one test per browser
-  // process instantiation, so saving the seed here is appopriate.
-  // GetDefaultTraceBasename() is going to be called twice:
-  // - for the first time, before the test starts to get the name of the file to
-  // stream the results (to avoid losing them if test crashes).
-  // - the second time, if test execution finishes normally, to calculate the
-  // resulting name of the file, including test result.
-  static std::string random_seed =
-      base::NumberToString(base::RandInt(1e7, 1e8 - 1));
-  std::string status;
-  if (type == TraceBasenameType::kWithTestStatus) {
-    status = ::testing::UnitTest::GetInstance()
-                     ->current_test_info()
-                     ->result()
-                     ->Passed()
-                 ? "OK"
-                 : "FAIL";
-  } else {
-    // In order to be able to stream the test to the file,
-    status = "NOT_FINISHED";
-  }
-  return "trace_test_" + test_suite_name + "_" + test_name + "_" + random_seed +
-         "_" + status;
+void TraceStopTracingComplete(base::OnceClosure quit,
+                              const base::FilePath& file_path) {
+  LOG(ERROR) << "Tracing written to: " << file_path.value();
+  std::move(quit).Run();
 }
 
 // See SetInitialWebContents comment for more information.
@@ -521,16 +483,6 @@ void BrowserTestBase::SetUp() {
       std::make_unique<CreatedMainPartsClosure>(base::BindOnce(
           &BrowserTestBase::CreatedBrowserMainParts, base::Unretained(this)));
 
-  // If tracing is enabled, customise the output filename based on the name of
-  // the test.
-  StartupTracingController::GetInstance().SetDefaultBasename(
-      GetDefaultTraceBasename(TraceBasenameType::kWithoutTestStatus),
-      StartupTracingController::ExtensionType::kAppendAppropriate);
-  // Write to the provided file directly to recover at least some data when the
-  // test crashes or times out.
-  StartupTracingController::GetInstance().SetUsingTemporaryFile(
-      StartupTracingController::TempFilePolicy::kWriteDirectly);
-
 #if defined(OS_ANDROID)
   // For all other platforms, we call ContentMain for browser tests which goes
   // through the normal browser initialization paths. For Android, we must set
@@ -718,6 +670,36 @@ void BrowserTestBase::WaitUntilJavaIsReady(base::OnceClosure quit_closure) {
 }
 #endif
 
+namespace {
+
+std::string GetDefaultTraceFilename() {
+  std::string test_suite_name = ::testing::UnitTest::GetInstance()
+                                    ->current_test_info()
+                                    ->test_suite_name();
+  std::string test_name =
+      ::testing::UnitTest::GetInstance()->current_test_info()->name();
+  // Parameterised tests might have slashes in their full name — replace them
+  // before using it as a file name to avoid trying to write to an incorrect
+  // location.
+  base::ReplaceChars(test_suite_name, "/", "_", &test_suite_name);
+  base::ReplaceChars(test_name, "/", "_", &test_name);
+  // Add random number to the trace file to distinguish traces from different
+  // test runs.
+  // We don't use timestamp here to avoid collisions with parallel runs of the
+  // same test.
+  std::string random_seed = base::NumberToString(base::RandInt(1e7, 1e8 - 1));
+  std::string status = ::testing::UnitTest::GetInstance()
+                               ->current_test_info()
+                               ->result()
+                               ->Passed()
+                           ? "OK"
+                           : "FAIL";
+  return "trace_test_" + test_suite_name + "_" + test_name + "_" + random_seed +
+         "_" + status + ".json";
+}
+
+}  // namespace
+
 void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
 #if !defined(OS_ANDROID)
   // All FeatureList overrides should have been registered prior to browser test
@@ -746,6 +728,17 @@ void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
     signal(SIGTERM, DumpStackTraceSignalHandler);
 #endif  // defined(OS_POSIX)
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTracing)) {
+    base::trace_event::TraceConfig trace_config(
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kEnableTracing),
+        base::trace_event::RECORD_CONTINUOUSLY);
+    TracingController::GetInstance()->StartTracing(
+        trace_config,
+        TracingController::StartTracingDoneCallback());
+  }
+
   {
     // This can be called from a posted task. Allow nested tasks here, because
     // otherwise the test body will have to do it in order to use RunLoop for
@@ -789,23 +782,31 @@ void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
     TearDownOnMainThread();
   }
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTracing)) {
+    base::FilePath trace_file =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValuePath(
+            switches::kEnableTracingOutput);
+    // If |trace_file| ends in a directory separator or is empty use a generated
+    // name in that directory (empty means current directory).
+    if (trace_file.empty() || trace_file.EndsWithSeparator())
+      trace_file = trace_file.AppendASCII(GetDefaultTraceFilename());
+
+    // Wait for tracing to collect results from the renderers.
+    base::RunLoop run_loop;
+    TracingController::GetInstance()->StopTracing(
+        TracingControllerImpl::CreateFileEndpoint(
+            trace_file, base::BindOnce(&TraceStopTracingComplete,
+                                       run_loop.QuitClosure(), trace_file)));
+    run_loop.Run();
+  }
+
   PostRunTestOnMainThread();
 
   // Sometimes tests initialize a storage partition and the initialization
   // schedules some tasks which need to be executed before finishing tests.
   // Run these tasks.
   content::RunAllPendingInMessageLoop();
-
-  // Update the trace output filename to include the test result.
-  StartupTracingController::GetInstance().SetDefaultBasename(
-      GetDefaultTraceBasename(TraceBasenameType::kWithTestStatus),
-      StartupTracingController::ExtensionType::kAppendAppropriate);
-
-#if defined(OS_ANDROID)
-  // On Android, browser main runner is not shut down, so stop trace recording
-  // here.
-  StartupTracingController::GetInstance().WaitUntilStopped();
-#endif
 }
 
 void BrowserTestBase::SetAllowNetworkAccessToHostResolutions() {