Add tracing AutoOpenCloseEvent.

base/trace_event/persistent_async_event.cc

+// 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/persistent_async_event.h"
+
+#include "base/macros.h"
+#include "base/time/time.h"
+#include "base/trace_event/trace_event.h"
+
+namespace base {
+

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

nit: remove this extra line

[alexandermont, 2016/09/15 21:05:12]

Done

+namespace trace_event {
+
+PersistentAsyncEvent::PersistentAsyncEvent(PersistentAsyncEvent::Type type,
+  const char* category, const char* event_name):
+  active_(false),
+  category_(category),
+  event_name_(event_name),
+  weak_factory_(this) {
+  DCHECK(thread_checker_.CalledOnValidThread());

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

remove this line, as it is tautological.
The threadchecker learns its "valid" thread on ctor.
But this is the ctor so the thread where you are in can't possibly be anything
other than the "valid" one :)

[alexandermont, 2016/09/15 21:05:13]

Done

+  base::trace_event::TraceLog::GetInstance()->AddAsyncEnabledStateObserver(
+      weak_factory_.GetWeakPtr());
+}
+
+PersistentAsyncEvent::~PersistentAsyncEvent() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  base::trace_event::TraceLog::GetInstance()->RemoveAsyncEnabledStateObserver(
+      this);
+}
+
+void PersistentAsyncEvent::Begin() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

oh I didn't think about enforce begin/end to be also forced to be called on the
right thread. Ok if works for your use case (in which case it's perfect as it
is)
If in future we need begin/end to be called on different thread we can make the
start_time a base::subtle::atomicint and use a release-store here (and an
acquire load in OnTraceLogEnabled)

[alexandermont, 2016/09/15 21:05:13]

okay

+  // We need to use the COPY version of the macro because the category and

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

nit: add a line between comments and the previous line (i.e. here between 33 and
34)

+  // event names aren't string literals; they're passed in through
+  // the constructor.
+  TRACE_EVENT_COPY_ASYNC_BEGIN0(category_, event_name_,

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

I dupe my previou (unresolved) question:
Why _COPY_ and not just RACE_EVENT_COPY_BEGIN0 ?
The args here seem to be constant strings.

[alexandermont, 2016/09/15 21:05:13]

Fixed

+      static_cast<void*>(this));
+  active_ = true;
+  start_time_ = base::TimeTicks::Now();
+}
+
+void PersistentAsyncEvent::End() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  TRACE_EVENT_COPY_ASYNC_END0(category_, event_name_, static_cast<void*>(this));
+  active_ = false;

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

just clear start_Time_?

[alexandermont, 2016/09/15 21:05:13]

Done

+}
+
+void PersistentAsyncEvent::OnTraceLogEnabled() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (active_)
+    TRACE_EVENT_COPY_ASYNC_BEGIN_WITH_TIMESTAMP0(

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

ditto about s/_COPY_//

+        category_, event_name_, static_cast<void*>(this),
+        start_time_.ToInternalValue());

[Primiano Tucci (use gerrit), 2016/09/15 10:08:29]

I think here you want InMicroseconds() not ToInternalValue.
As the name suggests you are not supposed to rely on the internal representation
of timeticks.

[alexandermont, 2016/09/15 21:05:12]

This is a TimeTicks value, not a TimeDelta value. Only TimeDelta has the
InMicroseconds() function; TimeTicks does not. Am I supposed to convert the
TimeTicks to a TimeDelta somehow?

[Primiano Tucci (use gerrit), 2016/09/15 22:06:25]

ah right, my bad. do whatever we do for the other events (no idea where we take
the time from) and be consistent with that.

+}
+
+void PersistentAsyncEvent::OnTraceLogDisabled() {}
+
+}  // namespace trace_event
+}  // namespace base