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

base/debug/trace_event_impl.h

Index: base/debug/trace_event_impl.h
diff --git a/base/debug/trace_event_impl.h b/base/debug/trace_event_impl.h
index 7b191b8a403f2bbf7baf25bde779ba691478db26..a5719e1bc472b8e10bea9415eaa24fd7749e60e9 100644
--- a/base/debug/trace_event_impl.h
+++ b/base/debug/trace_event_impl.h
@@ -364,28 +364,46 @@ class BASE_EXPORT CategoryFilter {
 
 class TraceSamplingThread;
 
-class BASE_EXPORT TraceLog {
- public:
-  enum Mode {
-    DISABLED = 0,
-    RECORDING_MODE,
-    MONITORING_MODE,
-  };
+struct BASE_EXPORT TraceOptions {
 
   // Options determines how the trace buffer stores data.
-  enum Options {
+  enum RecordMode {
     // Record until the trace buffer is full.
-    RECORD_UNTIL_FULL = 1 << 0,
+    RECORD_UNTIL_FULL,
 
     // Record until the user ends the trace. The trace buffer is a fixed size
     // and we use it as a ring buffer during recording.
-    RECORD_CONTINUOUSLY = 1 << 1,
-
-    // Enable the sampling profiler in the recording mode.
-    ENABLE_SAMPLING = 1 << 2,
+    RECORD_CONTINUOUSLY,
 
     // Echo to console. Events are discarded.
-    ECHO_TO_CONSOLE = 1 << 3,
+    ECHO_TO_CONSOLE,
+  };
+
+  static const char* kRecordUntilFull;
+  static const char* kRecordContinuously;
+  static const char* kTraceToConsole;
+  static const char* kEnableSampling;
+
+ TraceOptions()
+      : record_mode(RECORD_UNTIL_FULL), enable_sampling(false) {}
+
+ TraceOptions(RecordMode record_mode, bool enable_sampling)
+     : record_mode(record_mode), enable_sampling(enable_sampling) {}

[Xianzhu, 2014/07/29 21:59:05]

Indentation of the above lines seems incorrect.

[Xianzhu, 2014/07/29 22:00:27]

Sorry, this is an old comment. Please ignore it.

+
+  explicit TraceOptions(StringPiece options);
+
+  std::string ToString() const;
+
+  RecordMode record_mode;
+  bool enable_sampling;
+};
+
+class BASE_EXPORT TraceLog {
+ public:
+  enum Mode {
+    DISABLED = 0,
+    RECORDING_MODE,
+    MONITORING_MODE,
   };
 
   // The pointer returned from GetCategoryGroupEnabledInternal() points to a
@@ -410,16 +428,15 @@ class BASE_EXPORT TraceLog {
   // Retrieves a copy (for thread-safety) of the current CategoryFilter.
   CategoryFilter GetCurrentCategoryFilter();
 
-  Options trace_options() const {
-    return static_cast<Options>(subtle::NoBarrier_Load(&trace_options_));
-  }
+  // Retrieves a copy (for thread-safety) of the current TraceOptions.
+  TraceOptions GetCurrentTraceOptions() const;
 
   // Enables normal tracing (recording trace events in the trace buffer).
   // See CategoryFilter comments for details on how to control what categories
   // will be traced. If tracing has already been enabled, |category_filter| will
   // be merged into the current category filter.
   void SetEnabled(const CategoryFilter& category_filter,
-                  Mode mode, Options options);
+                  Mode mode, TraceOptions options);
 
   // Disables normal tracing for all categories.
   void SetDisabled();
@@ -599,6 +616,8 @@ class BASE_EXPORT TraceLog {
   void SetCurrentThreadBlocksMessageLoop();
 
  private:
+  typedef unsigned int InternalTraceOptions;
+
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
                            TraceBufferRingBufferGetReturnChunk);
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
@@ -607,6 +626,9 @@ class BASE_EXPORT TraceLog {
                            TraceBufferRingBufferFullIteration);
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
                            TraceBufferVectorReportFull);
+  FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
+                           ConvertTraceOptionsToInternalOptions);
+
 
   // This allows constructor and destructor to be private and usable only
   // by the Singleton class.
@@ -624,6 +646,9 @@ class BASE_EXPORT TraceLog {
   // category filter.
   void UpdateSyntheticDelaysFromCategoryFilter();
 
+  InternalTraceOptions GetInternalOptionsFromTraceOptions(
+      const TraceOptions& options);
+
   class ThreadLocalEventBuffer;
   class OptionalAutoLock;
 
@@ -632,6 +657,11 @@ class BASE_EXPORT TraceLog {
   const unsigned char* GetCategoryGroupEnabledInternal(const char* name);
   void AddMetadataEventsWhileLocked();
 
+  InternalTraceOptions trace_options() const {
+    return static_cast<InternalTraceOptions>(
+        subtle::NoBarrier_Load(&trace_options_));
+  }
+
   TraceBuffer* trace_buffer() const { return logged_events_.get(); }
   TraceBuffer* CreateTraceBuffer();
   TraceBuffer* CreateTraceBufferVectorOfSize(size_t max_chunks);
@@ -671,6 +701,14 @@ class BASE_EXPORT TraceLog {
     return timestamp - time_offset_;
   }
 
+  // Internal representation of trace options since we store the currently used
+  // trace option as an AtomicWord.
+  static const InternalTraceOptions NONE;

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

Not a big fan of this duplication. We now have to remember to define each of
these things in two different spots. Once here and once in TraceOptions above.

+  static const InternalTraceOptions RECORD_UNTIL_FULL;
+  static const InternalTraceOptions RECORD_CONTINUOUSLY;
+  static const InternalTraceOptions ENABLE_SAMPLING;
+  static const InternalTraceOptions ECHO_TO_CONSOLE;
+
   // This lock protects TraceLog member accesses (except for members protected
   // by thread_info_lock_) from arbitrary threads.
   mutable Lock lock_;