Refactor trace_event_impl's SetEnabled to use TraceOptions. Propagate this through the whole stack.

base/debug/trace_event_impl.cc

Index: base/debug/trace_event_impl.cc
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index 93ec6fe29974f891c499c7c6a03645bc645198df..04a405312c51c66d4f216f1e009819fd6c8c16f2 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -970,6 +970,49 @@ TraceBucketData::~TraceBucketData() {
 
 ////////////////////////////////////////////////////////////////////////////////
 //
+// TraceOptions
+//
+////////////////////////////////////////////////////////////////////////////////
+
+TraceOptions::TraceOptions(StringPiece options)
+    : record_mode(RECORD_UNTIL_FULL), enable_sampling(false) {
+  std::vector<std::string> split;
+  std::vector<std::string>::iterator iter;
+
+  base::SplitString(options.as_string(), ',', &split);
+  for (iter = split.begin(); iter != split.end(); ++iter) {
+    if (*iter == kRecordUntilFull) {
+      record_mode = RECORD_UNTIL_FULL;
+    } else if (*iter == kRecordContinuously) {
+      record_mode = RECORD_CONTINUOUSLY;
+    } else if (*iter == kTraceToConsole) {
+      record_mode = ECHO_TO_CONSOLE;
+    } else if (*iter == kEnableSampling) {
+      enable_sampling = true;
+    }
+  }
+}
+
+std::string TraceOptions::ToString() const {
+  std::string ret;
+  switch (record_mode) {
+    case RECORD_UNTIL_FULL:
+      ret = kRecordUntilFull;
+      break;
+    case RECORD_CONTINUOUSLY:
+      ret = kRecordContinuously;
+      break;
+    case ECHO_TO_CONSOLE:
+      ret = kTraceToConsole;
+      break;
+    default:
+      NOTREACHED();
+  }
+  return enable_sampling ? ret + "," + kEnableSampling : ret;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//
 // TraceLog
 //
 ////////////////////////////////////////////////////////////////////////////////
@@ -1175,7 +1218,9 @@ TraceLog::TraceLog()
 
     LOG(ERROR) << "Start " << switches::kTraceToConsole
                << " with CategoryFilter '" << filter << "'.";
-    SetEnabled(CategoryFilter(filter), RECORDING_MODE, ECHO_TO_CONSOLE);
+    SetEnabled(CategoryFilter(filter),
+               RECORDING_MODE,
+               TraceOptions(TraceOptions::ECHO_TO_CONSOLE, false));
   }
 #endif
 
@@ -1325,7 +1370,7 @@ void TraceLog::GetKnownCategoryGroups(
 
 void TraceLog::SetEnabled(const CategoryFilter& category_filter,
                           Mode mode,
-                          Options options) {
+                          TraceOptions options) {
   std::vector<EnabledStateObserver*> observer_list;
   {
     AutoLock lock(lock_);
@@ -1333,10 +1378,13 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
     // Can't enable tracing when Flush() is in progress.
     DCHECK(!flush_message_loop_proxy_.get());
 
-    Options old_options = trace_options();
+    InternalTraceOptions new_options =
+        GetInternalOptionsFromTraceOptions(options);
+
+   InternalTraceOptions old_options = trace_options();
 
     if (IsEnabled()) {
-      if (options != old_options) {
+      if (new_options != old_options) {
         DLOG(ERROR) << "Attempting to re-enable tracing with a different "
                     << "set of options.";
       }
@@ -1358,8 +1406,8 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
 
     mode_ = mode;
 
-    if (options != old_options) {
-      subtle::NoBarrier_Store(&trace_options_, options);
+    if (new_options != old_options) {
+      subtle::NoBarrier_Store(&trace_options_, new_options);
       UseNextTraceBuffer();
     }
 
@@ -1369,7 +1417,7 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
     UpdateCategoryGroupEnabledFlags();
     UpdateSyntheticDelaysFromCategoryFilter();
 
-    if (options & ENABLE_SAMPLING) {
+    if (new_options & ENABLE_SAMPLING) {
       sampling_thread_.reset(new TraceSamplingThread);
       sampling_thread_->RegisterSampleBucket(
           &g_trace_state[0],
@@ -1402,11 +1450,41 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
   }
 }
 
+TraceLog::InternalTraceOptions TraceLog::GetInternalOptionsFromTraceOptions(

[dsinclair, 2014/07/28 18:52:36]

The spit between trace options and internal trace options seems like it's going
to cause bugs in the future.

When can we use one, and when the other? What happens if we grab the
non-internal one at some point, does things get out of sync?

[nednguyen, 2014/07/28 20:27:22]

The InternalTraceOptions is only used by the trace_event_impl's implementation
as they are private. So all callers of TraceLog must use the TraceOptions. 

There maybe a better way to convey this, maybe a private namespace? I am not
sure, please give some advices.

+    const TraceOptions& options) {
+  InternalTraceOptions ret = options.enable_sampling ? ENABLE_SAMPLING : NONE;
+  switch (options.record_mode) {
+    case TraceOptions::RECORD_UNTIL_FULL:
+      return ret | RECORD_UNTIL_FULL;
+    case TraceOptions::RECORD_CONTINUOUSLY:
+      return ret | RECORD_CONTINUOUSLY;
+    case TraceOptions::ECHO_TO_CONSOLE:
+      return ret | ECHO_TO_CONSOLE;
+  }
+  NOTREACHED();
+  return NONE;
+}
+
 CategoryFilter TraceLog::GetCurrentCategoryFilter() {
   AutoLock lock(lock_);
   return category_filter_;
 }
 
+TraceOptions TraceLog::GetCurrentTraceOptions() const {
+  TraceOptions ret;
+  InternalTraceOptions option = trace_options();
+  ret.enable_sampling = (option & ENABLE_SAMPLING) != 0;
+  if (option & RECORD_UNTIL_FULL)
+    ret.record_mode = TraceOptions::RECORD_UNTIL_FULL;
+  else if (option & RECORD_CONTINUOUSLY)
+    ret.record_mode = TraceOptions::RECORD_CONTINUOUSLY;
+  else if (option & ECHO_TO_CONSOLE)
+    ret.record_mode = TraceOptions::ECHO_TO_CONSOLE;
+  else
+    NOTREACHED();
+  return ret;
+}
+
 void TraceLog::SetDisabled() {
   AutoLock lock(lock_);
   SetDisabledWhileLocked();
@@ -1496,7 +1574,7 @@ bool TraceLog::BufferIsFull() const {
 }
 
 TraceBuffer* TraceLog::CreateTraceBuffer() {
-  Options options = trace_options();
+  InternalTraceOptions options = trace_options();
   if (options & RECORD_CONTINUOUSLY)
     return new TraceBufferRingBuffer(kTraceEventRingBufferChunks);
   else if ((options & ENABLE_SAMPLING) && mode_ == MONITORING_MODE)