tracing: split trace event filter classes out of TraceLog

base/trace_event/trace_event_filter_test_utils.h

Index: base/trace_event/trace_event_filter_test_utils.h
diff --git a/base/trace_event/trace_event_filter_test_utils.h b/base/trace_event/trace_event_filter_test_utils.h
new file mode 100644
index 0000000000000000000000000000000000000000..a28cd3702d869c19e990a7afca0cb4de60fa1476
--- /dev/null
+++ b/base/trace_event/trace_event_filter_test_utils.h
@@ -0,0 +1,53 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_TRACE_EVENT_TRACE_EVENT_FILTER_TEST_UTILS_H_
+#define BASE_TRACE_EVENT_TRACE_EVENT_FILTER_TEST_UTILS_H_
+
+#include "base/base_export.h"
+#include "base/macros.h"
+#include "base/trace_event/trace_event_filter.h"
+
+namespace base {
+namespace trace_event {
+
+class BASE_EXPORT TestEventFilter : public TraceEventFilter {

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

TestEventFilter says "some TraceEventFilter with trivial implementation for
testing purposes". But in fact it's interceptor which counts number of
invocations. Let's call it e.g. EventCounterFilter.

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

?? Where?
This is a testing-only class and should have Test in the name.
However see the new patchset. I split out a HitsCOunter class

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

Okay. In patch set 2 it looks fine, thanks. But it was in the main codebase in
patch set 1, so I've complained about naming.

+ public:
+  static const char kName[];
+
+  TestEventFilter();
+  ~TestEventFilter() override;
+
+  // TraceEventFilter implementation.
+  bool FilterTraceEvent(const TraceEvent& trace_event) const override;
+  void EndEvent(const char* category_name, const char* name) const override;
+
+  // Testing methods.
+  static void set_filter_return_value(bool value) {
+    filter_return_value_ = value;
+  }
+
+  static size_t filter_trace_event_hit_count() {
+    return filter_trace_event_hit_count_;
+  }
+
+  static size_t end_event_hit_count() { return end_event_hit_count_; }
+
+  static void clear_counts() {
+    filter_trace_event_hit_count_ = 0;
+    end_event_hit_count_ = 0;
+  }
+
+ private:
+  static size_t filter_trace_event_hit_count_;
+  static size_t end_event_hit_count_;

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

Yes, I understand that this class is written for testing purposes, but I don't
like when non-static functions change static fields. Is there any strong reason
to make it static?

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

The problem is that we cannot manage the lifetime of this class in the test
fixture. This is created and owned by the tracelog.
Having said this, I did split a bit the design, separating a HitsCounter struct.
SO now the test fixture owns the struct

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

Acknowledged.

+  static bool filter_return_value_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestEventFilter);
+};
+
+}  // namespace trace_event
+}  // namespace base
+
+#endif  // BASE_TRACE_EVENT_TRACE_EVENT_FILTER_TEST_UTILS_H_