Introduce browser side FrameTreeNodeBlameContext

content/browser/frame_host/frame_tree_node_blame_context.h

Index: content/browser/frame_host/frame_tree_node_blame_context.h
diff --git a/content/browser/frame_host/frame_tree_node_blame_context.h b/content/browser/frame_host/frame_tree_node_blame_context.h
new file mode 100644
index 0000000000000000000000000000000000000000..b268c80ae4f7b7a1f42aa7914cad911bb77e3fa6
--- /dev/null
+++ b/content/browser/frame_host/frame_tree_node_blame_context.h
@@ -0,0 +1,49 @@
+// 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 CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H
+#define CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H
+
+#include "base/macros.h"
+#include "base/trace_event/blame_context.h"
+
+namespace base {
+namespace trace_event {
+class TracedValue;
+}
+}
+
+namespace content {
+
+class FrameTreeNode;
+
+class FrameTreeNodeBlameContext : public base::trace_event::BlameContext {

[Charlie Reis, 2016/04/20 16:58:58]

This class needs comments explaining the context for this.  (It could be
misinterpreted as part of the core frame logic rather than tracing/scheduling.)

[Xiaocheng, 2016/04/21 08:01:32]

Done.

+ public:
+  FrameTreeNodeBlameContext(FrameTreeNode* node);
+  ~FrameTreeNodeBlameContext() override;
+
+  // There are two patterns for getting argument values of a snapshot, both of
+  // which have potential threading issues:
+  // 1. When taking a snapshot, copy the arguments from the FrameTreeNode;
+  // 2. When a FrameTreeNode navigates, copy the arguments to BlameContext.
+  // We adopt the second approach which looks less troublesome with threading.
+  void UpdateArguments(FrameTreeNode* node);
+  void ClearArguments();
+
+ private:
+  // FIXME: A data race can occur if tracing starts when the main thread is in
+  // the middle of UpdateArguments() or ClearArguments(). AsValueInto() may
+  // generate undefined result in this case.
+  void AsValueInto(base::trace_event::TracedValue* value) override;
+
+  int process_id_;
+  int routing_id_;
+  std::string url_;

[Charlie Reis, 2016/04/20 16:58:58]

Why is a URL being stored in a string?  I would recommend against that, but if
it's necessary please explain why in a comment.

[benjhayden, 2016/04/20 17:29:45]

That's my fault. You're right, there should be a comment.
This field will only ever be serialized to the JSON trace log.
(Actually, the trace log format could change, but it probably won't ever use
anything besides a string for URLs. Probably.)
I wanted to be explicit about how the URL would be converted to a string,
instead of burying it under layers like TracedValue and BlameContext.
I'd be open to storing it as a GURL here and stringifying it in AsValueInto() if
you think that would be better.

[Xiaocheng, 2016/04/21 08:01:32]

I agree that a URL shouldn't be stored as a string. Fixed.

+
+  DISALLOW_COPY_AND_ASSIGN(FrameTreeNodeBlameContext);
+};
+
+}  // content
+
+#endif  // CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H