Remove TraceOptions string based constructor. Add setter method

SetFromString that allows setting TraceOptions from a string instead.

SetFromString returns true upon success.

BUG=400382

Review URL: https://codereview.chromium.org/443523003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288193 0039d316-1c4b-4281-b951-d872f2087c98

base/debug/trace_event_impl.cc

@@ -984,13 +984,13 @@ TraceBucketData::~TraceBucketData() {
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-TraceOptions::TraceOptions(const std::string& options_string)
-    : record_mode(RECORD_UNTIL_FULL),
-      enable_sampling(false),
-      enable_systrace(false) {
+bool TraceOptions::SetFromString(const std::string& options_string) {
+  record_mode = RECORD_UNTIL_FULL;
+  enable_sampling = false;
+  enable_systrace = false;
+
   std::vector<std::string> split;
   std::vector<std::string>::iterator iter;
-
   base::SplitString(options_string, ',', &split);
   for (iter = split.begin(); iter != split.end(); ++iter) {
     if (*iter == kRecordUntilFull) {
@@ -1006,9 +1006,10 @@ TraceOptions::TraceOptions(const std::string& options_string)
     } else if (*iter == kEnableSystrace) {
       enable_systrace = true;
     } else {
-      NOTREACHED();
+      return false;
     }
   }
+  return true;
 }
 
 std::string TraceOptions::ToString() const {

base/debug/trace_event_impl.h

@@ -409,11 +409,21 @@ struct BASE_EXPORT TraceOptions {
   // options_string, the last one takes precedence. If none of the trace
   // recording mode is specified, recording mode is RECORD_UNTIL_FULL.
   //
-  // Example: TraceOptions("record-until-full")
-  // Example: TraceOptions("record-continuously, enable-sampling")
-  // Example: TraceOptions("record-until-full, trace-to-console") would have
-  //          ECHO_TO_CONSOLE as the recording mode.
-  explicit TraceOptions(const std::string& options_string);
+  // The trace option will first be reset to the default option
+  // (record_mode set to RECORD_UNTIL_FULL, enable_sampling and enable_systrace
+  // set to false) before options parsed from |options_string| are applied on
+  // it.
+  // If |options_string| is invalid, the final state of trace_options is
+  // undefined.
+  //
+  // Example: trace_options.SetFromString("record-until-full")
+  // Example: trace_options.SetFromString(
+  //              "record-continuously, enable-sampling")
+  // Example: trace_options.SetFromString("record-until-full, trace-to-console")
+  // will set ECHO_TO_CONSOLE as the recording mode.
+  //
+  // Returns true on success.
+  bool SetFromString(const std::string& options_string);
 
   std::string ToString() const;
 

base/debug/trace_event_unittest.cc

@@ -2998,56 +2998,56 @@ TEST_F(TraceEventTestFixture, SyntheticDelayConfigurationToString) {
   EXPECT_EQ(config, filter.ToString());
 }
 
-TEST(TraceOptionsTest, DISABLED_TraceOptionsFromString) {
-  TraceOptions options = TraceOptions("record-until-full");
+TEST(TraceOptionsTest, TraceOptionsFromString) {
+  TraceOptions options;
+  EXPECT_TRUE(options.SetFromString("record-until-full"));
   EXPECT_EQ(RECORD_UNTIL_FULL, options.record_mode);
   EXPECT_FALSE(options.enable_sampling);
   EXPECT_FALSE(options.enable_systrace);
 
-  options = TraceOptions(RECORD_CONTINUOUSLY);
+  EXPECT_TRUE(options.SetFromString("record-continuously"));
   EXPECT_EQ(RECORD_CONTINUOUSLY, options.record_mode);
   EXPECT_FALSE(options.enable_sampling);
   EXPECT_FALSE(options.enable_systrace);
 
-  options = TraceOptions("trace-to-console");
+  EXPECT_TRUE(options.SetFromString("trace-to-console"));
   EXPECT_EQ(ECHO_TO_CONSOLE, options.record_mode);
   EXPECT_FALSE(options.enable_sampling);
   EXPECT_FALSE(options.enable_systrace);
 
-  options = TraceOptions("record-as-much-as-possible");
+  EXPECT_TRUE(options.SetFromString("record-as-much-as-possible"));
   EXPECT_EQ(RECORD_AS_MUCH_AS_POSSIBLE, options.record_mode);
   EXPECT_FALSE(options.enable_sampling);
   EXPECT_FALSE(options.enable_systrace);
 
-  options = TraceOptions("record-until-full, enable-sampling");
+  EXPECT_TRUE(options.SetFromString("record-until-full, enable-sampling"));
   EXPECT_EQ(RECORD_UNTIL_FULL, options.record_mode);
   EXPECT_TRUE(options.enable_sampling);
   EXPECT_FALSE(options.enable_systrace);
 
-  options = TraceOptions("enable-systrace,record-continuously");
+  EXPECT_TRUE(options.SetFromString("enable-systrace,record-continuously"));
   EXPECT_EQ(RECORD_CONTINUOUSLY, options.record_mode);
   EXPECT_FALSE(options.enable_sampling);
   EXPECT_TRUE(options.enable_systrace);
 
-  options = TraceOptions("enable-systrace, trace-to-console,enable-sampling");
+  EXPECT_TRUE(options.SetFromString(
+      "enable-systrace, trace-to-console,enable-sampling"));
   EXPECT_EQ(ECHO_TO_CONSOLE, options.record_mode);
   EXPECT_TRUE(options.enable_sampling);
   EXPECT_TRUE(options.enable_systrace);
 
-  options =
-      TraceOptions("record-continuously,record-until-full,trace-to-console");
+  EXPECT_TRUE(options.SetFromString(
+      "record-continuously,record-until-full,trace-to-console"));
   EXPECT_EQ(ECHO_TO_CONSOLE, options.record_mode);
   EXPECT_FALSE(options.enable_systrace);
   EXPECT_FALSE(options.enable_sampling);
 
-  options = TraceOptions("");
+  EXPECT_TRUE(options.SetFromString(""));
   EXPECT_EQ(RECORD_UNTIL_FULL, options.record_mode);
   EXPECT_FALSE(options.enable_systrace);
   EXPECT_FALSE(options.enable_sampling);
 
-#if GTEST_HAS_EXCEPTIONS
-  EXPECT_THROW(TraceOptions("foo-bar-baz"), int);
-#endif
+  EXPECT_FALSE(options.SetFromString("foo-bar-baz"));
 }
 
 TEST(TraceOptionsTest, TraceOptionsToString) {
@@ -3066,7 +3066,8 @@ TEST(TraceOptionsTest, TraceOptionsToString) {
         TraceOptions original_option = TraceOptions(modes[i]);
         original_option.enable_sampling = enable_sampling_options[j];
         original_option.enable_systrace = enable_systrace_options[k];
-        TraceOptions new_options = TraceOptions(original_option.ToString());
+        TraceOptions new_options;
+        EXPECT_TRUE(new_options.SetFromString(original_option.ToString()));
         EXPECT_EQ(original_option.record_mode, new_options.record_mode);
         EXPECT_EQ(original_option.enable_sampling, new_options.enable_sampling);
         EXPECT_EQ(original_option.enable_systrace, new_options.enable_systrace);

components/tracing/child_trace_message_filter.cc

@@ -59,11 +59,12 @@ void ChildTraceMessageFilter::OnBeginTracing(
       browser_time;
   TraceLog::GetInstance()->SetTimeOffset(time_offset);
 #endif
-
+  base::debug::TraceOptions trace_options;
+  trace_options.SetFromString(options);
   TraceLog::GetInstance()->SetEnabled(
       base::debug::CategoryFilter(category_filter_str),
       base::debug::TraceLog::RECORDING_MODE,
-      base::debug::TraceOptions(options));
+      trace_options);
 }
 
 void ChildTraceMessageFilter::OnEndTracing() {
@@ -81,10 +82,12 @@ void ChildTraceMessageFilter::OnEnableMonitoring(
     const std::string& category_filter_str,
     base::TimeTicks browser_time,
     const std::string& options) {
+  base::debug::TraceOptions trace_options;
+  trace_options.SetFromString(options);
   TraceLog::GetInstance()->SetEnabled(
       base::debug::CategoryFilter(category_filter_str),
       base::debug::TraceLog::MONITORING_MODE,
-      base::debug::TraceOptions(options));
+      trace_options);
 }
 
 void ChildTraceMessageFilter::OnDisableMonitoring() {

content/browser/android/tracing_controller_android.cc

@@ -35,15 +35,16 @@ bool TracingControllerAndroid::StartTracing(JNIEnv* env,
                                             jstring jtraceoptions) {
   std::string categories =
       base::android::ConvertJavaStringToUTF8(env, jcategories);
-  std::string trace_options =
-      base::android::ConvertJavaStringToUTF8(env, jtraceoptions);
+  base::debug::TraceOptions trace_options;
+  trace_options.SetFromString(
+      base::android::ConvertJavaStringToUTF8(env, jtraceoptions));
 
   // This log is required by adb_profile_chrome.py.
   LOG(WARNING) << "Logging performance trace to file";
 
   return TracingController::GetInstance()->EnableRecording(
       base::debug::CategoryFilter(categories),
-      base::debug::TraceOptions(trace_options),
+      trace_options,
       TracingController::EnableRecordingDoneCallback());
 }