[tracing] Add filtering mode in TraceLog

base/trace_event/trace_log.h

Index: base/trace_event/trace_log.h
diff --git a/base/trace_event/trace_log.h b/base/trace_event/trace_log.h
index 3eb79b819f7883acd4147689e8fce68077bc4441..c8b4e87f52a7d760c34549c7d1577f240046c258 100644
--- a/base/trace_event/trace_log.h
+++ b/base/trace_event/trace_log.h
@@ -45,10 +45,8 @@ struct BASE_EXPORT TraceLogStatus {
 
 class BASE_EXPORT TraceLog : public MemoryDumpProvider {
  public:
-  enum Mode {
-    DISABLED = 0,
-    RECORDING_MODE
-  };
+  // Argument passed to TrcaeLog::SetEnabled.
+  enum Mode { RECORDING_MODE = 1 << 0, FILTERING_MODE = 1 << 1 };
 
   // The pointer returned from GetCategoryGroupEnabledInternal() points to a
   // value with zero or more of the following bits. Used in this class only.
@@ -78,16 +76,30 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   // if the current thread supports that (has a message loop).
   void InitializeThreadLocalEventBufferIfSupported();
 
-  // Enables normal tracing (recording trace events in the trace buffer).
   // See TraceConfig 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 TraceConfig& trace_config, Mode mode);
-
-  // Disables normal tracing for all categories.
+  // will be traced. SetDisabled must be called for each mode that is enabled.
+  // RECORDING_MODE: Enables normal tracing (recording trace events in the trace
+  //                 buffer).
+  // FILTERING_MODE: Trace events are enabled just for filtering but not for
+  //                 recording. TraceLog::IsEnabled still returns false when
+  //                 running in this mode. Only event filters config of
+  //                 |trace_config| argument is used.
+  // If tracing has already been enabled, category filter (enabled and disabled
+  // categories) will be merged into the current category filter, and trace
+  // event filters will be used only if filtering was not enabled already.
+  void SetEnabled(const TraceConfig& trace_config, Mode new_mode);
+
+  // Disables tracing for all categories for the specified |mode| only. Default
+  // |mode| is taken as RECORDING_MODE.
   void SetDisabled();
+  void SetDisabled(Mode mode);
+
+  // Returns true if TraceLog is enabled on recording mode.
+  // Note: Returns false even if FILTERING_MODE is enabled.
+  bool IsEnabled() { return mode_ & RECORDING_MODE; }

[oystein (OOO til 10th of July), 2016/09/22 19:21:06]

I think we should rename this to IsEnabledForRecording() now.

[ssid, 2016/09/22 19:37:43]

Do you mind if I do this as a separate clean up CL? I would have to change some
20 files where this is being used. multiple owners will get spammed by
discussion in this CL.

[oystein (OOO til 10th of July), 2016/09/22 20:16:42]

Yep that's fine.

 
-  bool IsEnabled() { return mode_ != DISABLED; }
+  // Returns true if filters are enabled, irrespective of the recording mode.
+  bool IsFilteringEnabled();

[oystein (OOO til 10th of July), 2016/09/22 19:21:06]

What's the reason for not just checking for filtering mode here, like
IsEnabled(ForRecording?) above? Doing an IsEnabled(Mode).

[ssid, 2016/09/22 19:37:43]

So, I really need to check if filters are enabled rather than if filtering mode
is enabled. This is because if filters were enabled by any mode previously then
we cannot add more filters since it will cause races on the filters vector.
I added an additional function to get mode.

[oystein (OOO til 10th of July), 2016/09/22 20:16:42]

Okay, that makes sense.I think I'd avoid "Enabled" as part of the name in that
case though, to make it clear it's not an equivalent function to IsEnabled??().
"HaveActiveFilters" maybe, up to you!

 
   // The number of times we have begun recording traces. If tracing is off,
   // returns -1. If tracing is on, then it returns the number of times we have
@@ -384,6 +396,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture, TraceBufferVectorReportFull);
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
                            ConvertTraceConfigToInternalOptions);
+  FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture, TraceFilteringMode);
   FRIEND_TEST_ALL_PREFIXES(TraceEventTestFixture,
                            TraceRecordAsMuchAsPossibleMode);
 
@@ -436,7 +449,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   TraceEvent* AddEventToThreadSharedChunkWhileLocked(TraceEventHandle* handle,
                                                      bool check_buffer_is_full);
   void CheckIfBufferIsFullWhileLocked();
-  void SetDisabledWhileLocked();
+  void SetDisabledWhileLocked(Mode mode);
 
   TraceEvent* GetEventByHandleInternal(TraceEventHandle handle,
                                        OptionalAutoLock* lock);
@@ -485,7 +498,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   // This lock protects accesses to thread_names_, thread_event_start_times_
   // and thread_colors_.
   Lock thread_info_lock_;
-  Mode mode_;
+  int mode_;  // See TraceLog::Mode.

[oystein (OOO til 10th of July), 2016/09/22 19:21:06]

nit: Since this is now a bitmask it should probably use a specific sized type
like uint8_t instead. 

[ssid, 2016/09/22 19:37:44]

Done.

   int num_traces_recorded_;
   std::unique_ptr<TraceBuffer> logged_events_;
   std::vector<std::unique_ptr<TraceEvent>> metadata_events_;
@@ -527,6 +540,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
 
   TraceConfig trace_config_;
   TraceConfig event_callback_trace_config_;
+  TraceConfig::EventFilters event_filters_enabled_;
 
   ThreadLocalPointer<ThreadLocalEventBuffer> thread_local_event_buffer_;
   ThreadLocalBoolean thread_blocks_message_loop_;