tracing: split trace event filter classes out of TraceLog

base/trace_event/trace_log.cc

Index: base/trace_event/trace_log.cc
diff --git a/base/trace_event/trace_log.cc b/base/trace_event/trace_log.cc
index 4b21437c66114667b725e3dcfb1b70d7ea18f84b..c2b688f9a354cbdf6e8a2f5924e97be4dfa10066 100644
--- a/base/trace_event/trace_log.cc
+++ b/base/trace_event/trace_log.cc
@@ -32,13 +32,16 @@
 #include "base/threading/worker_pool.h"
 #include "base/time/time.h"
 #include "base/trace_event/category_registry.h"
+#include "base/trace_event/event_name_filter.h"
 #include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
+#include "base/trace_event/heap_profiler_event_filter.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_buffer.h"
 #include "base/trace_event/trace_event.h"
+#include "base/trace_event/trace_event_filter_test_utils.h"
 #include "base/trace_event/trace_event_synthetic_delay.h"
 #include "build/build_config.h"
 
@@ -84,82 +87,11 @@ const size_t kEchoToConsoleTraceEventBufferChunks = 256;
 const size_t kTraceEventBufferSizeInBytes = 100 * 1024;
 const int kThreadFlushTimeoutMs = 3000;
 
-const char kEventNameWhitelist[] = "event_name_whitelist";
-
 #define MAX_TRACE_EVENT_FILTERS 32
 
 // List of TraceEventFilter objects from the most recent tracing session.
-base::LazyInstance<std::vector<std::unique_ptr<TraceLog::TraceEventFilter>>>::
-    Leaky g_category_group_filters = LAZY_INSTANCE_INITIALIZER;
-
-class EventNameFilter : public TraceLog::TraceEventFilter {
- public:
-  EventNameFilter(const base::DictionaryValue* filter_args) {
-    const base::ListValue* whitelist = nullptr;
-    if (filter_args->GetList(kEventNameWhitelist, &whitelist)) {
-      for (size_t i = 0; i < whitelist->GetSize(); ++i) {
-        std::string event_name;
-        if (!whitelist->GetString(i, &event_name))
-          continue;
-
-        whitelist_.insert(event_name);
-      }
-    }
-  }
-
-  bool FilterTraceEvent(const TraceEvent& trace_event) const override {
-    return ContainsKey(whitelist_, trace_event.name());
-  }
-
- private:
-  std::unordered_set<std::string> whitelist_;
-};
-
-// This filter is used to record trace events as pseudo stack for the heap
-// profiler. It does not filter-out any events from the trace, ie. the behavior
-// of trace events being added to TraceLog remains same: the events are added
-// iff enabled for recording and not filtered-out by any other filter.
-class HeapProfilerFilter : public TraceLog::TraceEventFilter {
- public:
-  HeapProfilerFilter() {}
-
-  bool FilterTraceEvent(const TraceEvent& trace_event) const override {
-    if (AllocationContextTracker::capture_mode() !=
-        AllocationContextTracker::CaptureMode::PSEUDO_STACK) {
-      return true;
-    }
-
-    // TODO(primiano): Add support for events with copied name crbug.com/581079.
-    if (trace_event.flags() & TRACE_EVENT_FLAG_COPY)
-      return true;
-
-    const char* category_name =
-        TraceLog::GetCategoryGroupName(trace_event.category_group_enabled());
-    if (trace_event.phase() == TRACE_EVENT_PHASE_BEGIN ||
-        trace_event.phase() == TRACE_EVENT_PHASE_COMPLETE) {
-      AllocationContextTracker::GetInstanceForCurrentThread()
-          ->PushPseudoStackFrame({category_name, trace_event.name()});
-    } else if (trace_event.phase() == TRACE_EVENT_PHASE_END) {
-      // The pop for |TRACE_EVENT_PHASE_COMPLETE| events is in |EndEvent|.
-      AllocationContextTracker::GetInstanceForCurrentThread()
-          ->PopPseudoStackFrame({category_name, trace_event.name()});
-    }
-    // Do not filter-out any events and always return true. TraceLog adds the
-    // event only if it is enabled for recording.
-    return true;
-  }
-
-  void EndEvent(const char* name, const char* category_group) override {
-    if (AllocationContextTracker::capture_mode() ==
-        AllocationContextTracker::CaptureMode::PSEUDO_STACK) {
-      AllocationContextTracker::GetInstanceForCurrentThread()
-          ->PopPseudoStackFrame({category_group, name});
-    }
-  }
-};
-
-TraceLog::TraceEventFilterConstructorForTesting
-    g_trace_event_filter_constructor_for_testing = nullptr;
+base::LazyInstance<std::vector<std::unique_ptr<TraceEventFilter>>>::Leaky
+    g_category_group_filters = LAZY_INSTANCE_INITIALIZER;
 
 // The name of the current thread. This is used to decide if the current
 // thread name has changed. We combine all the seen thread names into the
@@ -232,17 +164,13 @@ void MakeHandle(uint32_t chunk_seq,
 }
 
 template <typename Function>
-void ForEachCategoryGroupFilter(const unsigned char* category_group_enabled,
-                                Function filter_fn) {
-  const TraceCategory* category =
-      CategoryRegistry::GetCategoryByStatePtr(category_group_enabled);
-  uint32_t filter_bitmap = category->enabled_filters();
-  int index = 0;
-  while (filter_bitmap) {
+void ForEachCategoryFilter(const unsigned char* category_group_enabled,
+                           Function filter_fn) {
+  auto* cat = CategoryRegistry::GetCategoryByStatePtr(category_group_enabled);
+  uint32_t filter_bitmap = cat->enabled_filters();
+  for (int index = 0; filter_bitmap != 0; filter_bitmap >>= 1, index++) {

[kraynov, 2016/12/06 18:24:23]

It became more readable, but could be improved, something like:
int index = 0;
for (uint32_t mask = 1; mask <= filter_bitmap; mask <<= 1, index++) {
  auto filter = g_category_group_filters.Get()[index];
  if ((filter_bitmap & mask) && filter)
    filter_fn(filter.get());
}

[Primiano Tucci (use gerrit), 2016/12/07 11:17:47]

not worth it, I'm going to change ths in the next CL. See
https://codereview.chromium.org/2557743002/

[kraynov, 2016/12/07 11:30:50]

Acknowledged.

     if (filter_bitmap & 1 && g_category_group_filters.Get()[index])
       filter_fn(g_category_group_filters.Get()[index].get());
-    filter_bitmap = filter_bitmap >> 1;
-    index++;
   }
 }
 
@@ -516,6 +444,7 @@ const char* TraceLog::GetCategoryGroupName(
 }
 
 void TraceLog::UpdateCategoryState(TraceCategory* category) {
+  lock_.AssertAcquired();
   DCHECK(category->is_valid());
   unsigned char state_flags = 0;
   if (enabled_modes_ & RECORDING_MODE &&
@@ -556,6 +485,7 @@ void TraceLog::UpdateCategoryState(TraceCategory* category) {
 }
 
 void TraceLog::UpdateCategoryRegistry() {
+  lock_.AssertAcquired();
   CreateFiltersForTraceConfig();
   for (TraceCategory& category : CategoryRegistry::GetAllCategories()) {
     UpdateCategoryState(&category);
@@ -571,7 +501,7 @@ void TraceLog::CreateFiltersForTraceConfig() {
   if (g_category_group_filters.Get().size())
     return;
 
-  for (auto& event_filter : enabled_event_filters_) {
+  for (auto& filter_config : enabled_event_filters_) {
     if (g_category_group_filters.Get().size() >= MAX_TRACE_EVENT_FILTERS) {
       NOTREACHED()
           << "Too many trace event filters installed in the current session";
@@ -579,17 +509,24 @@ void TraceLog::CreateFiltersForTraceConfig() {
     }
 
     std::unique_ptr<TraceEventFilter> new_filter;
-    if (event_filter.predicate_name() ==
-        TraceEventFilter::kEventWhitelistPredicate) {
-      new_filter = MakeUnique<EventNameFilter>(event_filter.filter_args());
-    } else if (event_filter.predicate_name() ==
-               TraceEventFilter::kHeapProfilerPredicate) {
-      new_filter = MakeUnique<HeapProfilerFilter>();
-    } else if (event_filter.predicate_name() == "testing_predicate") {
-      CHECK(g_trace_event_filter_constructor_for_testing);
-      new_filter = g_trace_event_filter_constructor_for_testing();
+    if (filter_config.predicate_name() == EventNameFilter::kName) {
+      auto whitelist = MakeUnique<std::unordered_set<std::string>>();
+      const ListValue* list = nullptr;
+      if (filter_config.filter_args()->GetList("event_name_whitelist", &list)) {
+        for (size_t i = 0; i < list->GetSize(); ++i) {
+          std::string event_name;
+          if (list->GetString(i, &event_name))
+            whitelist->insert(event_name);
+        }
+      }
+      new_filter = MakeUnique<EventNameFilter>(std::move(whitelist));
+    } else if (filter_config.predicate_name() ==
+               HeapProfilerEventFilter::kName) {
+      new_filter = MakeUnique<HeapProfilerEventFilter>();
+    } else if (filter_config.predicate_name() == TestEventFilter::kName) {
+      new_filter = MakeUnique<TestEventFilter>();
     } else {
-      NOTREACHED();
+      CHECK(false) << "Unknown trace filter " << filter_config.predicate_name();
     }
     g_category_group_filters.Get().push_back(std::move(new_filter));
   }
@@ -791,9 +728,8 @@ void TraceLog::SetDisabledWhileLocked(uint8_t modes_to_disable) {
   if (modes_to_disable & FILTERING_MODE)
     enabled_event_filters_.clear();
 
-  if (modes_to_disable & RECORDING_MODE) {
+  if (modes_to_disable & RECORDING_MODE)
     trace_config_.Clear();
-  }
 
   UpdateCategoryRegistry();
 
@@ -1338,7 +1274,7 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
                                 arg_values, convertable_values, flags);
 
     disabled_by_filters = true;
-    ForEachCategoryGroupFilter(
+    ForEachCategoryFilter(
         category_group_enabled, [&new_trace_event, &disabled_by_filters](
                                     TraceEventFilter* trace_event_filter) {
           if (trace_event_filter->FilterTraceEvent(*new_trace_event))
@@ -1468,10 +1404,10 @@ void TraceLog::EndFilteredEvent(const unsigned char* category_group_enabled,
                                 const char* name,
                                 TraceEventHandle handle) {
   const char* category_name = GetCategoryGroupName(category_group_enabled);
-  ForEachCategoryGroupFilter(
+  ForEachCategoryFilter(
       category_group_enabled,
       [name, category_name](TraceEventFilter* trace_event_filter) {
-        trace_event_filter->EndEvent(name, category_name);
+        trace_event_filter->EndEvent(category_name, name);
       });
 }
 
@@ -1612,11 +1548,6 @@ void TraceLog::DeleteForTesting() {
   CategoryRegistry::ResetForTesting();
 }
 
-void TraceLog::SetTraceEventFilterConstructorForTesting(
-    TraceEventFilterConstructorForTesting predicate) {
-  g_trace_event_filter_constructor_for_testing = predicate;
-}
-
 TraceEvent* TraceLog::GetEventByHandle(TraceEventHandle handle) {
   return GetEventByHandleInternal(handle, NULL);
 }