tracing: simplify lifetime of TraceEventFilter(s)

base/trace_event/event_filter_registry.cc

Index: base/trace_event/event_filter_registry.cc
diff --git a/base/trace_event/event_filter_registry.cc b/base/trace_event/event_filter_registry.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ba45d93621aefa04ca2037c4ee0623edd93f70d1
--- /dev/null
+++ b/base/trace_event/event_filter_registry.cc
@@ -0,0 +1,105 @@
+// 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.
+
+#include "base/trace_event/event_filter_registry.h"
+
+#include <string.h>
+
+#include <type_traits>
+
+#include "base/atomicops.h"
+#include "base/logging.h"
+#include "base/trace_event/trace_event_filter.h"
+
+namespace base {
+namespace trace_event {
+
+// static
+constexpr uint32_t EventFilterRegistry::kMaxFilters;
+
+// static
+EventFilterRegistry::FilterEntry
+    EventFilterRegistry::filters_[EventFilterRegistry::kMaxFilters] = {};
+
+// static
+uint32_t EventFilterRegistry::filters_index_ = 0;
+
+// static
+void EventFilterRegistry::RegisterFilter(
+    std::unique_ptr<TraceEventFilter> filter) {
+  static_assert(std::is_pod<EventFilterRegistry::FilterEntry>::value,
+                "FilterEntry must be pod");
+  DCHECK_LT(filters_index_, kMaxFilters);
+  for (uint32_t i = 0; i < kMaxFilters; i++) {

[ssid, 2016/12/12 04:50:18]

Why do you need a loop here?
Can you just do
CHECK(!filters_[filters_index_].active);
And execute for i=0 case?

The only case where filters_[filters_index_].active is true is when you add 33
filters in the same session or there's a race in registering and unregistering

[Primiano Tucci (use gerrit), 2016/12/12 14:38:54]

Right I wrote this when I wanted to support unregistration of individual  (so to
support having holes in the sequence) but then realized that right now we don't
need it, and forgot to simplify this code.
So the principle of keeping it simple unless you need complexity wins, doing
your way.

+    FilterEntry& entry = filters_[(filters_index_ + i) % kMaxFilters];
+    if (!entry.filter || !entry.active) {
+      if (entry.filter)
+        delete entry.filter;
+      entry.filter = filter.release();
+      entry.active = true;
+      filters_index_ = (filters_index_ + i + 1) % kMaxFilters;
+      return;
+    }
+    CHECK(false) << "Attempting to register too many trace filters";
+  }
+}
+
+// static
+void EventFilterRegistry::UnregisterAllFilters() {
+  for (uint32_t i = 0; i < kMaxFilters; i++)
+    filters_[i].active = false;
+}
+
+// static
+void EventFilterRegistry::ResetForTesting() {
+  for (uint32_t i = 0; i < kMaxFilters; i++) {
+    if (filters_[i].filter)
+      delete filters_[i].filter;
+    filters_[i].filter = nullptr;
+    filters_[i].active = false;
+  }
+  filters_index_ = 0;
+}
+
+// static
+EventFilterRegistry::Iterator EventFilterRegistry::GetActiveFilters() {
+  // Start with offset == -1 and MoveNext() so that the first iteration starts
+  // at the first non-free slot >= |filters_index_|, which is the oldest but
+  // still active filter in the FIFO.
+  const uint32_t offset = static_cast<uint32_t>(-1);
+  auto it = EventFilterRegistry::Iterator(filters_index_, offset);
+  it.MoveNext();
+  return it;
+}
+
+EventFilterRegistry::Iterator::Iterator(uint32_t start, uint32_t offset)
+    : start_(start), offset_(offset) {}
+
+bool EventFilterRegistry::Iterator::IsValid() const {
+  return offset_ < kMaxFilters;
+}
+
+void EventFilterRegistry::Iterator::MoveNext() {
+  for (offset_++; offset_ < kMaxFilters; offset_++) {
+    if (filters_[(start_ + offset_) % kMaxFilters].active)
+      return;
+  }
+  // End reached, kMaxFilters is our equivalent of STL's end().
+  offset_ = kMaxFilters;
+}
+
+uint32_t EventFilterRegistry::Iterator::GetIndex() const {
+  DCHECK_LT(offset_, kMaxFilters);
+  return (start_ + offset_) % kMaxFilters;
+}
+
+const TraceEventFilter* EventFilterRegistry::Iterator::operator->() const {
+  const uint32_t index = GetIndex();
+  const TraceEventFilter* filter = filters_[index].filter;
+  DCHECK(filter);
+  return filter;
+}
+
+}  // namespace trace_event
+}  // namespace base