Remove FrameTreeNodeBlameContext, as it is not used. Bug: 1270671 Change-Id: I24efd09ef4006d071fdc492d7bd9eda393e9e918 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3555935 Reviewed-by: Alexander Timin <altimin@chromium.org> Commit-Queue: Harkiran Bolaria <hbolaria@google.com> Cr-Commit-Position: refs/heads/main@{#986490}
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index 266282b..c2ffe0a9 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn
@@ -1472,8 +1472,6 @@ "renderer_host/frame_tree.h", "renderer_host/frame_tree_node.cc", "renderer_host/frame_tree_node.h", - "renderer_host/frame_tree_node_blame_context.cc", - "renderer_host/frame_tree_node_blame_context.h", "renderer_host/global_routing_id.cc", "renderer_host/http_error_navigation_throttle.cc", "renderer_host/http_error_navigation_throttle.h",
diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc index 469db69..f973371 100644 --- a/content/browser/renderer_host/frame_tree_node.cc +++ b/content/browser/renderer_host/frame_tree_node.cc
@@ -149,15 +149,11 @@ is_created_by_script_(is_created_by_script), devtools_frame_token_(devtools_frame_token), frame_owner_properties_(frame_owner_properties), - blame_context_(frame_tree_node_id_, FrameTreeNode::From(parent)), render_manager_(this, frame_tree->manager_delegate()) { 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); - - // Note: this should always be done last in the constructor. - blame_context_.Initialize(); } void FrameTreeNode::DestroyInnerFrameTreeIfExists() { @@ -401,7 +397,6 @@ void FrameTreeNode::SetCurrentURL(const GURL& url) { current_frame_host()->SetLastCommittedUrl(url); - blame_context_.TakeSnapshot(); } void FrameTreeNode::SetCollapsed(bool collapsed) {
diff --git a/content/browser/renderer_host/frame_tree_node.h b/content/browser/renderer_host/frame_tree_node.h index 591cadcd..115895a 100644 --- a/content/browser/renderer_host/frame_tree_node.h +++ b/content/browser/renderer_host/frame_tree_node.h
@@ -16,7 +16,6 @@ #include "base/memory/ref_counted.h" #include "base/observer_list.h" #include "content/browser/renderer_host/frame_tree.h" -#include "content/browser/renderer_host/frame_tree_node_blame_context.h" #include "content/browser/renderer_host/navigator.h" #include "content/browser/renderer_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_frame_host_manager.h" @@ -360,9 +359,6 @@ // FrameTreeNode. void BeforeUnloadCanceled(); - // Returns the BlameContext associated with this node. - FrameTreeNodeBlameContext& blame_context() { return blame_context_; } - // Updates the user activation state in the browser frame tree and in the // frame trees in all renderer processes except the renderer for this node // (which initiated the update). Returns |false| if the update tries to @@ -699,11 +695,6 @@ // for details on how this state is maintained. blink::UserActivationState user_activation_state_; - // A helper for tracing the snapshots of this FrameTreeNode and attributing - // browser process activities to this node (when possible). It is unrelated - // to the core logic of FrameTreeNode. - FrameTreeNodeBlameContext blame_context_; - // Fenced Frames: // Nonce used in the net::IsolationInfo and blink::StorageKey for a fenced // frame and any iframes nested within it. Not set if this frame is not in a
diff --git a/content/browser/renderer_host/frame_tree_node_blame_context.cc b/content/browser/renderer_host/frame_tree_node_blame_context.cc deleted file mode 100644 index 245fee4..0000000 --- a/content/browser/renderer_host/frame_tree_node_blame_context.cc +++ /dev/null
@@ -1,69 +0,0 @@ -// 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 "content/browser/renderer_host/frame_tree_node_blame_context.h" - -#include "base/process/process_handle.h" -#include "base/strings/stringprintf.h" -#include "base/trace_event/traced_value.h" -#include "content/browser/renderer_host/frame_tree_node.h" -#include "url/gurl.h" - -namespace content { - -namespace { - -const char kFrameTreeNodeBlameContextCategory[] = "navigation"; -const char kFrameTreeNodeBlameContextName[] = "FrameTreeNodeBlameContext"; -const char kFrameTreeNodeBlameContextType[] = "FrameTreeNode"; -const char kFrameTreeNodeBlameContextScope[] = "FrameTreeNode"; -const char kRenderFrameBlameContextScope[] = "RenderFrame"; - -} // namespace - -FrameTreeNodeBlameContext::FrameTreeNodeBlameContext(int node_id, - FrameTreeNode* parent) - : base::trace_event::BlameContext( - kFrameTreeNodeBlameContextCategory, - kFrameTreeNodeBlameContextName, - kFrameTreeNodeBlameContextType, - kFrameTreeNodeBlameContextScope, - node_id, - parent ? &parent->blame_context() : nullptr) {} - -FrameTreeNodeBlameContext::~FrameTreeNodeBlameContext() {} - -void FrameTreeNodeBlameContext::AsValueInto( - base::trace_event::TracedValue* value) { - BlameContext::AsValueInto(value); - - // id() is equal to the owner FrameTreeNode's id, as set in the constructor. - FrameTreeNode* owner = FrameTreeNode::GloballyFindByID(id()); - DCHECK(owner); - - RenderFrameHostImpl* current_frame_host = owner->current_frame_host(); - if (!current_frame_host) - return; - - int process_id = base::kNullProcessId; - if (current_frame_host->GetProcess()->GetProcess().IsValid()) - process_id = current_frame_host->GetProcess()->GetProcess().Pid(); - - if (process_id >= 0) { - int routing_id = current_frame_host->GetRoutingID(); - DCHECK_NE(routing_id, MSG_ROUTING_NONE); - - value->BeginDictionary("renderFrame"); - value->SetInteger("pid_ref", process_id); - value->SetString("id_ref", base::StringPrintf("0x%x", routing_id)); - value->SetString("scope", kRenderFrameBlameContextScope); - value->EndDictionary(); - } - - GURL url = current_frame_host->GetLastCommittedURL(); - if (url.is_valid()) - value->SetString("url", url.spec()); -} - -} // namespace content
diff --git a/content/browser/renderer_host/frame_tree_node_blame_context.h b/content/browser/renderer_host/frame_tree_node_blame_context.h deleted file mode 100644 index 877d6859..0000000 --- a/content/browser/renderer_host/frame_tree_node_blame_context.h +++ /dev/null
@@ -1,41 +0,0 @@ -// 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_RENDERER_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H_ -#define CONTENT_BROWSER_RENDERER_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H_ - -#include "base/trace_event/blame_context.h" -#include "url/gurl.h" - -namespace base { -namespace trace_event { -class TracedValue; -} // namespace trace_event -} // namespace base - -namespace content { - -class FrameTreeNode; - -// FrameTreeNodeBlameContext is a helper class for tracing snapshots of each -// FrameTreeNode and attributing browser activities to frames (when possible), -// in the framework of FrameBlamer (crbug.com/546021). This class is unrelated -// to the core logic of FrameTreeNode. -class FrameTreeNodeBlameContext : public base::trace_event::BlameContext { - public: - FrameTreeNodeBlameContext(int node_id, FrameTreeNode* parent); - - FrameTreeNodeBlameContext(const FrameTreeNodeBlameContext&) = delete; - FrameTreeNodeBlameContext& operator=(const FrameTreeNodeBlameContext&) = - delete; - - ~FrameTreeNodeBlameContext() override; - - private: - void AsValueInto(base::trace_event::TracedValue* value) override; -}; - -} // namespace content - -#endif // CONTENT_BROWSER_RENDERER_HOST_FRAME_TREE_NODE_BLAME_CONTEXT_H_
diff --git a/content/browser/renderer_host/frame_tree_node_blame_context_unittest.cc b/content/browser/renderer_host/frame_tree_node_blame_context_unittest.cc deleted file mode 100644 index 8301870b..0000000 --- a/content/browser/renderer_host/frame_tree_node_blame_context_unittest.cc +++ /dev/null
@@ -1,233 +0,0 @@ -// 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 "content/browser/renderer_host/frame_tree_node_blame_context.h" - -#include <algorithm> -#include <memory> -#include <set> -#include <string> - -#include "base/containers/contains.h" -#include "base/strings/stringprintf.h" -#include "base/test/trace_event_analyzer.h" -#include "base/trace_event/traced_value.h" -#include "content/browser/renderer_host/frame_tree.h" -#include "content/browser/renderer_host/frame_tree_node.h" -#include "content/test/test_render_view_host.h" -#include "content/test/test_web_contents.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "third_party/blink/public/common/frame/frame_owner_element_type.h" -#include "third_party/blink/public/common/frame/frame_policy.h" -#include "third_party/blink/public/common/tokens/tokens.h" -#include "third_party/blink/public/mojom/frame/frame_owner_properties.mojom.h" - -namespace content { - -namespace { - -bool EventPointerCompare(const trace_analyzer::TraceEvent* lhs, - const trace_analyzer::TraceEvent* rhs) { - CHECK(lhs); - CHECK(rhs); - return *lhs < *rhs; -} - -void ExpectFrameTreeNodeObject(const trace_analyzer::TraceEvent* event) { - EXPECT_EQ("navigation", event->category); - EXPECT_EQ("FrameTreeNode", event->name); -} - -void ExpectFrameTreeNodeSnapshot(const trace_analyzer::TraceEvent* event) { - ExpectFrameTreeNodeObject(event); - EXPECT_TRUE(event->HasArg("snapshot")); - EXPECT_TRUE(event->arg_values.at("snapshot").is_dict()); -} - -std::string GetParentNodeID(const trace_analyzer::TraceEvent* event) { - const base::Value& arg_snapshot = event->arg_values.at("snapshot"); - EXPECT_TRUE(arg_snapshot.is_dict()); - const base::Value* parent = arg_snapshot.FindKey("parent"); - if (!parent) - return std::string(); - EXPECT_TRUE(parent->is_dict()); - const std::string* parent_id = parent->FindStringKey("id_ref"); - EXPECT_TRUE(parent_id); - return *parent_id; -} - -std::string GetSnapshotURL(const trace_analyzer::TraceEvent* event) { - const base::Value& arg_snapshot = event->arg_values.at("snapshot"); - EXPECT_TRUE(arg_snapshot.is_dict()); - const base::Value* url = arg_snapshot.FindKey("url"); - if (!url) - return std::string(); - EXPECT_TRUE(url->is_string()); - return url->GetString(); -} - -} // namespace - -class FrameTreeNodeBlameContextTest : public RenderViewHostImplTestHarness { - public: - FrameTree& tree() { return contents()->GetPrimaryFrameTree(); } - FrameTreeNode* root() { return tree().root(); } - int process_id() { - return root()->current_frame_host()->GetProcess()->GetID(); - } - - // Creates a frame tree specified by |shape|, which is a string of paired - // parentheses. Each pair of parentheses represents a FrameTreeNode, and the - // nesting of parentheses represents the parent-child relation between nodes. - // Nodes represented by outer-most parentheses are children of the root node. - // NOTE: Each node can have at most 9 child nodes, and the tree height (i.e., - // max # of edges in any root-to-leaf path) must be at most 9. - // See the test cases for sample usage. - void CreateFrameTree(const char* shape) { - main_test_rfh()->InitializeRenderFrameIfNeeded(); - CreateSubframes(root(), 1, shape); - } - - void RemoveAllNonRootFrames() { - while (root()->child_count()) - tree().RemoveFrame(root()->child_at(0)); - } - - private: - int CreateSubframes(FrameTreeNode* node, int self_id, const char* shape) { - int consumption = 0; - for (int child_num = 1; shape[consumption++] == '('; ++child_num) { - int child_id = self_id * 10 + child_num; - tree().AddFrame( - node->current_frame_host(), process_id(), child_id, - TestRenderFrameHost::CreateStubFrameRemote(), - TestRenderFrameHost::CreateStubBrowserInterfaceBrokerReceiver(), - TestRenderFrameHost::CreateStubPolicyContainerBindParams(), - blink::mojom::TreeScopeType::kDocument, std::string(), - base::StringPrintf("uniqueName%d", child_id), false, - blink::LocalFrameToken(), base::UnguessableToken::Create(), - blink::FramePolicy(), blink::mojom::FrameOwnerProperties(), false, - blink::FrameOwnerElementType::kIframe, - /*is_dummy_frame_for_inner_tree=*/false); - FrameTreeNode* child = node->child_at(child_num - 1); - consumption += CreateSubframes(child, child_id, shape + consumption); - } - return consumption; - } -}; - -// Creates a frame tree, tests if (i) the creation of each new frame is -// correctly traced, and (ii) the topology given by the snapshots is correct. -TEST_F(FrameTreeNodeBlameContextTest, FrameCreation) { - /* Shape of the frame tree to be created: - * () - * / \ - * () () - * / \ | - * () () () - * | - * () - */ - const char* tree_shape = "(()())((()))"; - - trace_analyzer::Start("*"); - CreateFrameTree(tree_shape); - auto analyzer = trace_analyzer::Stop(); - - trace_analyzer::TraceEventVector events; - trace_analyzer::Query q = - trace_analyzer::Query::EventPhaseIs(TRACE_EVENT_PHASE_CREATE_OBJECT) || - trace_analyzer::Query::EventPhaseIs(TRACE_EVENT_PHASE_SNAPSHOT_OBJECT); - analyzer->FindEvents(q, &events); - - // Two events for each new node: creation and snapshot. - EXPECT_EQ(12u, events.size()); - - std::set<FrameTreeNode*> creation_traced; - std::set<FrameTreeNode*> snapshot_traced; - for (auto* event : events) { - ExpectFrameTreeNodeObject(event); - FrameTreeNode* node = - tree().FindByID(strtol(event->id.c_str(), nullptr, 16)); - EXPECT_NE(nullptr, node); - if (event->HasArg("snapshot")) { - ExpectFrameTreeNodeSnapshot(event); - EXPECT_FALSE(base::Contains(snapshot_traced, node)); - snapshot_traced.insert(node); - std::string parent_id = GetParentNodeID(event); - EXPECT_FALSE(parent_id.empty()); - EXPECT_EQ(node->parent()->frame_tree_node(), - tree().FindByID(strtol(parent_id.c_str(), nullptr, 16))); - } else { - EXPECT_EQ(TRACE_EVENT_PHASE_CREATE_OBJECT, event->phase); - EXPECT_FALSE(base::Contains(creation_traced, node)); - creation_traced.insert(node); - } - } -} - -// Deletes frames from a frame tree, tests if the destruction of each frame is -// correctly traced. -TEST_F(FrameTreeNodeBlameContextTest, FrameDeletion) { - /* Shape of the frame tree to be created: - * () - * / \ - * () () - * / \ | - * () () () - * | - * () - */ - const char* tree_shape = "(()())((()))"; - - CreateFrameTree(tree_shape); - std::set<int> node_ids; - for (FrameTreeNode* node : tree().Nodes()) - node_ids.insert(node->frame_tree_node_id()); - - trace_analyzer::Start("*"); - RemoveAllNonRootFrames(); - auto analyzer = trace_analyzer::Stop(); - - trace_analyzer::TraceEventVector events; - trace_analyzer::Query q = - trace_analyzer::Query::EventPhaseIs(TRACE_EVENT_PHASE_DELETE_OBJECT); - analyzer->FindEvents(q, &events); - - // The removal of all non-root nodes should be traced. - EXPECT_EQ(6u, events.size()); - for (auto* event : events) { - ExpectFrameTreeNodeObject(event); - int id = strtol(event->id.c_str(), nullptr, 16); - EXPECT_TRUE(base::Contains(node_ids, id)); - node_ids.erase(id); - } -} - -// Changes URL of the root node. Tests if URL change is correctly traced. -TEST_F(FrameTreeNodeBlameContextTest, URLChange) { - main_test_rfh()->InitializeRenderFrameIfNeeded(); - GURL url1("http://a.com/"); - GURL url2("https://b.net/"); - - trace_analyzer::Start("*"); - root()->SetCurrentURL(url1); - root()->SetCurrentURL(url2); - root()->SetCurrentURL(GURL()); - auto analyzer = trace_analyzer::Stop(); - - trace_analyzer::TraceEventVector events; - trace_analyzer::Query q = - trace_analyzer::Query::EventPhaseIs(TRACE_EVENT_PHASE_SNAPSHOT_OBJECT); - analyzer->FindEvents(q, &events); - std::sort(events.begin(), events.end(), EventPointerCompare); - - // Three snapshots are traced, one for each URL change. - EXPECT_EQ(3u, events.size()); - EXPECT_EQ(url1.spec(), GetSnapshotURL(events[0])); - EXPECT_EQ(url2.spec(), GetSnapshotURL(events[1])); - EXPECT_EQ("", GetSnapshotURL(events[2])); -} - -} // namespace content
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index 0e5008b..ce6b142 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn
@@ -2183,7 +2183,6 @@ "../browser/renderer_host/embedded_frame_sink_provider_impl_unittest.cc", "../browser/renderer_host/fenced_frame_tree_node_unittest.cc", "../browser/renderer_host/frame_token_message_queue_unittest.cc", - "../browser/renderer_host/frame_tree_node_blame_context_unittest.cc", "../browser/renderer_host/frame_tree_unittest.cc", "../browser/renderer_host/input/fling_controller_unittest.cc", "../browser/renderer_host/input/fling_scheduler_unittest.cc",