Introduce browser side FrameTreeNodeBlameContext

content/browser/frame_host/frame_tree_node.cc

Index: content/browser/frame_host/frame_tree_node.cc
diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc
index 547e872f2210bb42d1f420d3506b42d0f9b50a31..4ece6a130310bc807b0df1fcad519b9c7332b2f9 100644
--- a/content/browser/frame_host/frame_tree_node.cc
+++ b/content/browser/frame_host/frame_tree_node.cc
@@ -15,7 +15,6 @@
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
-#include "content/browser/frame_host/traced_frame_tree_node.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/common/frame_messages.h"
 #include "content/common/site_isolation_policy.h"
@@ -101,17 +100,12 @@ FrameTreeNode::FrameTreeNode(
           false /* is a potentially trustworthy unique origin */),
       pending_sandbox_flags_(blink::WebSandboxFlags::None),
       frame_owner_properties_(frame_owner_properties),
-      loading_progress_(kLoadingProgressNotStarted) {
+      loading_progress_(kLoadingProgressNotStarted),
+      blame_context_(nullptr) {
   std::pair<FrameTreeNodeIdMap::iterator, bool> result =
       g_frame_tree_node_id_map.Get().insert(
           std::make_pair(frame_tree_node_id_, this));
   CHECK(result.second);
-
-  TRACE_EVENT_OBJECT_CREATED_WITH_ID(
-      "navigation", "FrameTreeNode",
-      TRACE_ID_WITH_SCOPE("FrameTreeNode", frame_tree_node_id_));
-  // Don't TraceSnapshot() until the RenderFrameHostManager is initialized and
-  // calls SetCurrentURL().
 }
 
 FrameTreeNode::~FrameTreeNode() {
@@ -124,9 +118,9 @@ FrameTreeNode::~FrameTreeNode() {
 
   g_frame_tree_node_id_map.Get().erase(frame_tree_node_id_);
 
-  TRACE_EVENT_OBJECT_DELETED_WITH_ID(
-      "navigation", "FrameTreeNode",
-      TRACE_ID_WITH_SCOPE("FrameTreeNode", frame_tree_node_id_));
+  // TODO(xiaochengh): Is this check necessary?
+  if (blame_context_)
+    blame_context_->ClearArguments();
 }
 
 void FrameTreeNode::AddObserver(Observer* observer) {
@@ -148,6 +142,11 @@ FrameTreeNode* FrameTreeNode::AddChild(std::unique_ptr<FrameTreeNode> child,
   CHECK_EQ(process_id, render_manager_.current_host()->GetProcess()->GetID());
   child->set_parent(this);
 
+  // TODO(xiaochengh): Is it the right timing to initialize BlameContext?
+  // It has to be after setting parent, because BlameContext::parent_id_ cannot
+  // be changed once initialized.
+  child->InitializeBlameContext();

[Xiaocheng, 2016/04/20 02:01:05]

One more question: BlameContext::parent_id_ is declared as a const, but the
topology of the frame tree can be changed -- we can, say, create a new iframe
and then put all the other existing iframes into it. Does it introduce any
issue, e.g., reassigning the parent of an existing BlameContext?

[Sami, 2016/04/20 12:15:08]

If the topology changes, the easiest thing to do is to create another blame
context with the same id as before. If this is a problem, we can modify
base::BlameContext to support changing the parent.

+
   // Initialize the RenderFrameHost for the new node.  We always create child
   // frames in the same SiteInstance as the current frame, and they can swap to
   // a different one if they navigate away.
@@ -183,7 +182,12 @@ void FrameTreeNode::RemoveChild(FrameTreeNode* child) {
 
 void FrameTreeNode::ResetForNewProcess() {
   current_frame_host()->set_last_committed_url(GURL());
-  TraceSnapshot();
+
+  // TODO(xiaochengh): Is this check necessary?
+  if (blame_context_) {
+    blame_context_->UpdateArguments(this);
+    blame_context_->TakeSnapshot();
+  }
 
   // Remove child nodes from the tree, then delete them. This destruction
   // operation will notify observers.
@@ -209,7 +213,12 @@ void FrameTreeNode::SetCurrentURL(const GURL& url) {
   if (!has_committed_real_load_ && url != GURL(url::kAboutBlankURL))
     has_committed_real_load_ = true;
   current_frame_host()->set_last_committed_url(url);
-  TraceSnapshot();
+
+  // TODO(xiaochengh): Is this check necessary?
+  if (blame_context_) {
+    blame_context_->UpdateArguments(this);
+    blame_context_->TakeSnapshot();
+  }
 }
 
 void FrameTreeNode::SetCurrentOrigin(
@@ -478,13 +487,10 @@ void FrameTreeNode::BeforeUnloadCanceled() {
   }
 }
 
-void FrameTreeNode::TraceSnapshot() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
-      "navigation", "FrameTreeNode",
-      TRACE_ID_WITH_SCOPE("FrameTreeNode", frame_tree_node_id_),
-      std::unique_ptr<base::trace_event::ConvertableToTraceFormat>(
-          new TracedFrameTreeNode(*this)));
+void FrameTreeNode::InitializeBlameContext() {
+  DCHECK(!blame_context_);
+  blame_context_ = new FrameTreeNodeBlameContext(this);

[benjhayden, 2016/04/20 01:37:34]

Where is this object deleted?

[Xiaocheng, 2016/04/20 02:01:05]

See the reply for the last comment.

+  blame_context_->Initialize();
 }
 
 }  // namespace content