Implement frame attribution (FrameBlamer)

base/debug/blame_context.h

Index: base/debug/blame_context.h
diff --git a/base/debug/blame_context.h b/base/debug/blame_context.h
new file mode 100644
index 0000000000000000000000000000000000000000..2e0a4475ae8d0564e17ddfcf874a4200b0837376
--- /dev/null
+++ b/base/debug/blame_context.h
@@ -0,0 +1,112 @@
+// 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_DEBUG_BLAME_CONTEXT_H_
+#define BASE_DEBUG_BLAME_CONTEXT_H_
+
+#include <inttypes.h>
+
+#include "base/macros.h"
+#include "base/strings/stringprintf.h"
+#include "base/trace_event/trace_event.h"
+#include "base/trace_event/trace_event_argument.h"
+
+namespace base {
+namespace debug {
+
+class BlameContextBase {
+ public:
+  virtual void Enter() = 0;
+  virtual void Leave() = 0;
+
+ protected:
+  BlameContextBase() {}
+  virtual ~BlameContextBase() {}
+
+  DISALLOW_COPY_AND_ASSIGN(BlameContextBase);
+};
+
+template <const char* category, const char* name, const char* scope>

[benjhayden, 2016/02/29 19:30:37]

It seems like this just requires subclasses and users like
RenderFrameBlameContext to repeat themselves a few times.
Why not just have users pass these into the constructor like
TraceScopedTrackableObject does?
Also, if BlameContext is not a template, then you won't need a separate base
class, iiuc.

[Sami, 2016/02/29 20:20:31]

I agree that this is a little weird but I don't see a better way of doing it
with our current infrastructure. The basic problem is that all trace events
(including enter/leave context) will initialize a static structure at the call
site that points to the trace category info data structure. The pointer is
atomically set up the first time the we encounter the trace event and cached for
all eternity.

The result of this is that a single TRACE_EVENT macro instantiation can only
ever be associated with single category. In other words, you can't pass in the
category string dynamically since it may have already been cached in the static
pointer.

The workaround for is to ensure every time we pass in a unique category string
to TRACE_EVENT, we generate a separate bit of code for that instance using a
template. TraceScopedTrackableObject also has this problem since it's only
templatized by the object type, not the category.

I guess we could simplify this a little by requiring you to only pass the
category string as a template (or hard coding it so that all blame contexts use
the same category), but I'm not sure if that's better or worse than this
version.

(Full credit to fady@ for figuring this out earlier.)

+class BlameContext : public BlameContextBase,
+                     trace_event::TraceLog::EnabledStateObserver {
+ public:
+  explicit BlameContext(int64_t id)
+      : id_(id),
+        parent_id_(0),
+        parent_scope_(nullptr),
+        trace_object_(category, name, TRACE_ID_WITH_SCOPE(scope, id)) {
+    trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this);
+  }
+
+  template <const char* parent_category,
+            const char* parent_name,
+            const char* parent_scope>
+  BlameContext(int64_t id,
+               const BlameContext<parent_category, parent_name, parent_scope>&
+                   parent_context)
+      : id_(id),
+        parent_id_(parent_context.id()),
+        parent_scope_(parent_scope),
+        trace_object_(category, name, TRACE_ID_WITH_SCOPE(scope, id)) {
+    trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this);
+  }
+
+  // BlameContextBase implementation:
+  void Enter() override {
+    TRACE_EVENT_ENTER_CONTEXT(category, name, TRACE_ID_WITH_SCOPE(scope, id_));
+  }
+
+  void Leave() override {
+    TRACE_EVENT_LEAVE_CONTEXT(category, name, TRACE_ID_WITH_SCOPE(scope, id_));
+  }
+
+  void TakeSnapshot() {
+    bool is_tracing = false;

[benjhayden, 2016/02/29 19:30:37]

Looks like most of this method could be moved into TraceScopedTrackableObject.
Devtools might not need some of these features, but it seems like it
conceptually belongs there. I can imagine non-BlameContext
TraceScopedTrackableObjects that would want to replicate it.

[Sami, 2016/02/29 20:20:31]

Right, we could move it there. I think I'd still want to have the AsValueInto()
virtual here somehow so that subclasses can add more metadata to the snapshot.
Agreed about TraceScopedTrackableObjects, but I'm not sure what the DevTools
connection is here?

+    TRACE_EVENT_CATEGORY_GROUP_ENABLED(category, &is_tracing);
+    if (!is_tracing)
+      return;
+    scoped_refptr<trace_event::TracedValue> snapshot(
+        new trace_event::TracedValue);
+    AsValueInto(snapshot.get());
+    scoped_refptr<trace_event::ConvertableToTraceFormat> snapshot_data(
+        snapshot);
+    trace_object_.Snapshot(snapshot_data);
+  }
+
+  int64_t id() const { return id_; }
+
+  // trace_event::TraceLog::EnabledStateObserver implementation:
+  void OnTraceLogEnabled() override { TakeSnapshot(); }
+
+  void OnTraceLogDisabled() override {}
+
+ protected:
+  virtual ~BlameContext() {
+    trace_event::TraceLog::GetInstance()->RemoveEnabledStateObserver(this);
+  }
+
+  virtual void AsValueInto(trace_event::TracedValue* state) {
+    if (!parent_id_)
+      return;
+    state->BeginDictionary("parent");
+    state->SetString("id_ref", StringPrintf("0x%" PRIx64, parent_id_));
+    state->SetString("scope", parent_scope_);
+    state->EndDictionary();
+  }
+
+ private:
+  const int64_t id_;
+  const int64_t parent_id_;
+  const char* parent_scope_;
+
+  trace_event::TraceScopedTrackableObject<trace_event::TraceID::WithScope>
+      trace_object_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlameContext);
+};
+
+}  // namespace debug
+}  // namespace base
+
+#endif  // BASE_DEBUG_BLAME_CONTEXT_H_