diff options
Diffstat (limited to 'chromium/components/performance_manager/graph')
20 files changed, 816 insertions, 129 deletions
diff --git a/chromium/components/performance_manager/graph/frame_node_impl.cc b/chromium/components/performance_manager/graph/frame_node_impl.cc index bd2d47c950e..d09bcce5831 100644 --- a/chromium/components/performance_manager/graph/frame_node_impl.cc +++ b/chromium/components/performance_manager/graph/frame_node_impl.cc @@ -26,7 +26,7 @@ FrameNodeImpl::FrameNodeImpl(ProcessNodeImpl* process_node, FrameNodeImpl* parent_frame_node, int frame_tree_node_id, int render_frame_id, - const base::UnguessableToken& dev_tools_token, + const FrameToken& frame_token, int32_t browsing_instance_id, int32_t site_instance_id) : parent_frame_node_(parent_frame_node), @@ -34,7 +34,7 @@ FrameNodeImpl::FrameNodeImpl(ProcessNodeImpl* process_node, process_node_(process_node), frame_tree_node_id_(frame_tree_node_id), render_frame_id_(render_frame_id), - dev_tools_token_(dev_tools_token), + frame_token_(frame_token), browsing_instance_id_(browsing_instance_id), site_instance_id_(site_instance_id), render_frame_host_proxy_(content::GlobalFrameRoutingId( @@ -49,6 +49,7 @@ FrameNodeImpl::FrameNodeImpl(ProcessNodeImpl* process_node, FrameNodeImpl::~FrameNodeImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(child_worker_nodes_.empty()); + DCHECK(opened_page_nodes_.empty()); } void FrameNodeImpl::Bind( @@ -135,8 +136,8 @@ int FrameNodeImpl::render_frame_id() const { return render_frame_id_; } -const base::UnguessableToken& FrameNodeImpl::dev_tools_token() const { - return dev_tools_token_; +const FrameToken& FrameNodeImpl::frame_token() const { + return frame_token_; } int32_t FrameNodeImpl::browsing_instance_id() const { @@ -156,6 +157,11 @@ const base::flat_set<FrameNodeImpl*>& FrameNodeImpl::child_frame_nodes() const { return child_frame_nodes_; } +const base::flat_set<PageNodeImpl*>& FrameNodeImpl::opened_page_nodes() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return opened_page_nodes_; +} + mojom::LifecycleState FrameNodeImpl::lifecycle_state() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return lifecycle_state_.value(); @@ -208,6 +214,7 @@ const base::flat_set<WorkerNodeImpl*>& FrameNodeImpl::child_worker_nodes() } const PriorityAndReason& FrameNodeImpl::priority_and_reason() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return priority_and_reason_.value(); } @@ -216,6 +223,11 @@ bool FrameNodeImpl::had_form_interaction() const { return document_.had_form_interaction.value(); } +bool FrameNodeImpl::is_audible() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return is_audible_.value(); +} + void FrameNodeImpl::SetIsCurrent(bool is_current) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); is_current_.SetAndMaybeNotify(this, is_current); @@ -255,6 +267,12 @@ void FrameNodeImpl::SetIsHoldingIndexedDBLock(bool is_holding_indexeddb_lock) { is_holding_indexeddb_lock_.SetAndMaybeNotify(this, is_holding_indexeddb_lock); } +void FrameNodeImpl::SetIsAudible(bool is_audible) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK_NE(is_audible, is_audible_.value()); + is_audible_.SetAndMaybeNotify(this, is_audible); +} + void FrameNodeImpl::OnNavigationCommitted(const GURL& url, bool same_document) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -309,6 +327,28 @@ void FrameNodeImpl::SetPriorityAndReason( priority_and_reason_.SetAndMaybeNotify(this, priority_and_reason); } +void FrameNodeImpl::AddOpenedPage(util::PassKey<PageNodeImpl>, + PageNodeImpl* page_node) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK(page_node); + DCHECK_NE(page_node_, page_node); + DCHECK(graph()->NodeInGraph(page_node)); + DCHECK_EQ(this, page_node->opener_frame_node()); + bool inserted = opened_page_nodes_.insert(page_node).second; + DCHECK(inserted); +} + +void FrameNodeImpl::RemoveOpenedPage(util::PassKey<PageNodeImpl>, + PageNodeImpl* page_node) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK(page_node); + DCHECK_NE(page_node_, page_node); + DCHECK(graph()->NodeInGraph(page_node)); + DCHECK_EQ(this, page_node->opener_frame_node()); + size_t removed = opened_page_nodes_.erase(page_node); + DCHECK_EQ(1u, removed); +} + const FrameNode* FrameNodeImpl::GetParentFrameNode() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return parent_frame_node(); @@ -329,9 +369,9 @@ int FrameNodeImpl::GetFrameTreeNodeId() const { return frame_tree_node_id(); } -const base::UnguessableToken& FrameNodeImpl::GetDevToolsToken() const { +const FrameToken& FrameNodeImpl::GetFrameToken() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - return dev_tools_token(); + return frame_token(); } int32_t FrameNodeImpl::GetBrowsingInstanceId() const { @@ -365,6 +405,26 @@ const base::flat_set<const FrameNode*> FrameNodeImpl::GetChildFrameNodes() return children; } +bool FrameNodeImpl::VisitOpenedPageNodes(const PageNodeVisitor& visitor) const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + for (auto* page_impl : opened_page_nodes()) { + const PageNode* page = page_impl; + if (!visitor.Run(page)) + return false; + } + return true; +} + +const base::flat_set<const PageNode*> FrameNodeImpl::GetOpenedPageNodes() + const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + base::flat_set<const PageNode*> opened; + for (auto* page : opened_page_nodes()) + opened.insert(static_cast<const PageNode*>(page)); + DCHECK_EQ(opened.size(), opened_page_nodes().size()); + return opened; +} + FrameNodeImpl::LifecycleState FrameNodeImpl::GetLifecycleState() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return lifecycle_state(); @@ -431,6 +491,11 @@ bool FrameNodeImpl::HadFormInteraction() const { return had_form_interaction(); } +bool FrameNodeImpl::IsAudible() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return is_audible(); +} + void FrameNodeImpl::AddChildFrame(FrameNodeImpl* child_frame_node) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(child_frame_node); @@ -474,6 +539,9 @@ void FrameNodeImpl::OnBeforeLeavingGraph() { DCHECK(child_frame_nodes_.empty()); + // Sever opener relationships. + SeverOpenedPagesAndMaybeReparent(); + // Leave the page. DCHECK(graph()->NodeInGraph(page_node_)); page_node_->RemoveFrame(this); @@ -493,6 +561,42 @@ void FrameNodeImpl::OnBeforeLeavingGraph() { render_frame_id_, this); } +void FrameNodeImpl::SeverOpenedPagesAndMaybeReparent() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + + // Copy |opened_page_nodes_| as we'll be modifying it in this loop: when we + // call PageNodeImpl::(Set|Clear)OpenerFrameNodeAndOpenedType() this will call + // back into this frame node and call RemoveOpenedPage(). + base::flat_set<PageNodeImpl*> opened_nodes = opened_page_nodes_; + for (auto* opened_node : opened_nodes) { + auto opened_type = opened_node->opened_type(); + + // Reparent opened pages to this frame's parent to maintain the relationship + // between the frame trees for bookkeeping. For the relationship to be + // finally severed one of the frame trees must completely disappear, or it + // must be explicitly severed (this can happen with portals). + if (parent_frame_node_) { + opened_node->SetOpenerFrameNodeAndOpenedType(parent_frame_node_, + opened_type); + } else { + // There's no new parent, so simply clear the opener. + opened_node->ClearOpenerFrameNodeAndOpenedType(); + } + } + + // Expect each page node to have called RemoveOpenedPage(), and for this to + // now be empty. + DCHECK(opened_page_nodes_.empty()); +} + +FrameNodeImpl* FrameNodeImpl::GetFrameTreeRoot() const { + FrameNodeImpl* root = const_cast<FrameNodeImpl*>(this); + while (root->parent_frame_node()) + root = parent_frame_node(); + DCHECK_NE(nullptr, root); + return root; +} + bool FrameNodeImpl::HasFrameNodeInAncestors(FrameNodeImpl* frame_node) const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (parent_frame_node_ == frame_node || @@ -513,6 +617,11 @@ bool FrameNodeImpl::HasFrameNodeInDescendants(FrameNodeImpl* frame_node) const { return false; } +bool FrameNodeImpl::HasFrameNodeInTree(FrameNodeImpl* frame_node) const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return GetFrameTreeRoot() == frame_node->GetFrameTreeRoot(); +} + FrameNodeImpl::DocumentProperties::DocumentProperties() = default; FrameNodeImpl::DocumentProperties::~DocumentProperties() = default; diff --git a/chromium/components/performance_manager/graph/frame_node_impl.h b/chromium/components/performance_manager/graph/frame_node_impl.h index a568de6b5c1..bcc31d55609 100644 --- a/chromium/components/performance_manager/graph/frame_node_impl.h +++ b/chromium/components/performance_manager/graph/frame_node_impl.h @@ -11,6 +11,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/unguessable_token.h" +#include "base/util/type_safety/pass_key.h" #include "components/performance_manager/graph/node_base.h" #include "components/performance_manager/public/graph/frame_node.h" #include "components/performance_manager/public/render_frame_host_proxy.h" @@ -64,7 +65,7 @@ class FrameNodeImpl FrameNodeImpl* parent_frame_node, int frame_tree_node_id, int render_frame_id, - const base::UnguessableToken& dev_tools_token, + const FrameToken& frame_token, int32_t browsing_instance_id, int32_t site_instance_id); ~FrameNodeImpl() override; @@ -92,13 +93,14 @@ class FrameNodeImpl ProcessNodeImpl* process_node() const; int frame_tree_node_id() const; int render_frame_id() const; - const base::UnguessableToken& dev_tools_token() const; + const FrameToken& frame_token() const; int32_t browsing_instance_id() const; int32_t site_instance_id() const; const RenderFrameHostProxy& render_frame_host_proxy() const; // Getters for non-const properties. These are not thread safe. const base::flat_set<FrameNodeImpl*>& child_frame_nodes() const; + const base::flat_set<PageNodeImpl*>& opened_page_nodes() const; LifecycleState lifecycle_state() const; InterventionPolicy origin_trial_freeze_policy() const; bool has_nonempty_beforeunload() const; @@ -111,11 +113,13 @@ class FrameNodeImpl const base::flat_set<WorkerNodeImpl*>& child_worker_nodes() const; const PriorityAndReason& priority_and_reason() const; bool had_form_interaction() const; + bool is_audible() const; // Setters are not thread safe. void SetIsCurrent(bool is_current); void SetIsHoldingWebLock(bool is_holding_weblock); void SetIsHoldingIndexedDBLock(bool is_holding_indexeddb_lock); + void SetIsAudible(bool is_audible); // Invoked when a navigation is committed in the frame. void OnNavigationCommitted(const GURL& url, bool same_document); @@ -131,10 +135,22 @@ class FrameNodeImpl return weak_factory_.GetWeakPtr(); } + void SeverOpenedPagesAndMaybeReparentForTesting() { + SeverOpenedPagesAndMaybeReparent(); + } + + protected: + friend class PageNodeImpl; + + // Invoked by opened pages when this frame is set/cleared as their opener. + // See PageNodeImpl::(Set|Clear)OpenerFrameNodeAndOpenedType. + void AddOpenedPage(util::PassKey<PageNodeImpl> key, PageNodeImpl* page_node); + void RemoveOpenedPage(util::PassKey<PageNodeImpl> key, + PageNodeImpl* page_node); + private: friend class FrameNodeImplDescriber; friend class FramePriorityAccess; - friend class PageNodeImpl; friend class ProcessNodeImpl; // Rest of FrameNode implementation. These are private so that users of the @@ -143,11 +159,13 @@ class FrameNodeImpl const PageNode* GetPageNode() const override; const ProcessNode* GetProcessNode() const override; int GetFrameTreeNodeId() const override; - const base::UnguessableToken& GetDevToolsToken() const override; + const FrameToken& GetFrameToken() const override; int32_t GetBrowsingInstanceId() const override; int32_t GetSiteInstanceId() const override; bool VisitChildFrameNodes(const FrameNodeVisitor& visitor) const override; const base::flat_set<const FrameNode*> GetChildFrameNodes() const override; + bool VisitOpenedPageNodes(const PageNodeVisitor& visitor) const override; + const base::flat_set<const PageNode*> GetOpenedPageNodes() const override; LifecycleState GetLifecycleState() const override; InterventionPolicy GetOriginTrialFreezePolicy() const override; bool HasNonemptyBeforeUnload() const override; @@ -160,6 +178,7 @@ class FrameNodeImpl const base::flat_set<const WorkerNode*> GetChildWorkerNodes() const override; const PriorityAndReason& GetPriorityAndReason() const override; bool HadFormInteraction() const override; + bool IsAudible() const override; // Properties associated with a Document, which are reset when a // different-document navigation is committed in the frame. @@ -205,8 +224,21 @@ class FrameNodeImpl void OnJoiningGraph() override; void OnBeforeLeavingGraph() override; + // Helper function to sever all opened page relationships. This is called + // before destroying the frame node in "OnBeforeLeavingGraph". Note that this + // will reparent opened pages to this frame's parent so that tracking is + // maintained. + void SeverOpenedPagesAndMaybeReparent(); + + // This is not quite the same as GetMainFrame, because there can be multiple + // main frames while the main frame is navigating. This explicitly walks up + // the tree to find the main frame that corresponds to this frame tree node, + // even if it is not current. + FrameNodeImpl* GetFrameTreeRoot() const; + bool HasFrameNodeInAncestors(FrameNodeImpl* frame_node) const; bool HasFrameNodeInDescendants(FrameNodeImpl* frame_node) const; + bool HasFrameNodeInTree(FrameNodeImpl* frame_node) const; mojo::Receiver<mojom::DocumentCoordinationUnit> receiver_{this}; @@ -218,11 +250,11 @@ class FrameNodeImpl const int frame_tree_node_id_; // The routing id of the frame. const int render_frame_id_; - // A unique identifier shared with all representations of this node across - // content and blink. The token is only defined by the browser process and - // is never sent back from the renderer in control calls. It should never be - // used to look up the FrameTreeNode instance. - const base::UnguessableToken dev_tools_token_; + + // This is the unique token for this frame instance as per e.g. + // RenderFrameHost::GetFrameToken(). + const FrameToken frame_token_; + // The unique ID of the BrowsingInstance this frame belongs to. Frames in the // same BrowsingInstance are allowed to script each other at least // asynchronously (if cross-site), and sometimes synchronously (if same-site, @@ -238,6 +270,9 @@ class FrameNodeImpl base::flat_set<FrameNodeImpl*> child_frame_nodes_; + // The set of pages that have been opened by this frame. + base::flat_set<PageNodeImpl*> opened_page_nodes_; + // Does *not* change when a navigation is committed. ObservedProperty::NotifiesOnlyOnChanges< LifecycleState, @@ -283,6 +318,13 @@ class FrameNodeImpl priority_and_reason_{PriorityAndReason(base::TaskPriority::LOWEST, kDefaultPriorityReason)}; + // Indicates if the frame is audible. This is tracked independently of a + // document, and if a document swap occurs the audio stream monitor machinery + // will keep this up to date. + ObservedProperty:: + NotifiesOnlyOnChanges<bool, &FrameNodeObserver::OnIsAudibleChanged> + is_audible_{false}; + // Inline storage for FramePriorityDecorator data. frame_priority::AcceptedVote accepted_vote_; diff --git a/chromium/components/performance_manager/graph/frame_node_impl_describer.cc b/chromium/components/performance_manager/graph/frame_node_impl_describer.cc index 7eadd8a358a..8e1e61c2245 100644 --- a/chromium/components/performance_manager/graph/frame_node_impl_describer.cc +++ b/chromium/components/performance_manager/graph/frame_node_impl_describer.cc @@ -83,7 +83,7 @@ base::Value FrameNodeImplDescriber::DescribeFrameNodeData( // Frame node properties. ret.SetIntKey("frame_tree_node_id", impl->frame_tree_node_id_); ret.SetIntKey("render_frame_id", impl->render_frame_id_); - ret.SetStringKey("dev_tools_token", impl->dev_tools_token_.ToString()); + ret.SetStringKey("frame_token", impl->frame_token_.value().ToString()); ret.SetIntKey("browsing_instance_id", impl->browsing_instance_id_); ret.SetIntKey("site_instance_id", impl->site_instance_id_); ret.SetStringKey("lifecycle_state", @@ -95,6 +95,7 @@ base::Value FrameNodeImplDescriber::DescribeFrameNodeData( ret.SetBoolKey("is_current", impl->is_current_.value()); ret.SetKey("priority", PriorityAndReasonToValue(impl->priority_and_reason_.value())); + ret.SetBoolKey("is_audible", impl->is_audible_.value()); return ret; } diff --git a/chromium/components/performance_manager/graph/frame_node_impl_unittest.cc b/chromium/components/performance_manager/graph/frame_node_impl_unittest.cc index cb971e443eb..467696d12dc 100644 --- a/chromium/components/performance_manager/graph/frame_node_impl_unittest.cc +++ b/chromium/components/performance_manager/graph/frame_node_impl_unittest.cc @@ -4,6 +4,7 @@ #include "components/performance_manager/graph/frame_node_impl.h" +#include "base/test/gtest_util.h" #include "components/performance_manager/graph/page_node_impl.h" #include "components/performance_manager/graph/process_node_impl.h" #include "components/performance_manager/test_support/graph_test_harness.h" @@ -21,6 +22,10 @@ const FrameNode* ToPublic(FrameNodeImpl* frame_node) { return frame_node; } +const PageNode* ToPublic(PageNodeImpl* page_node) { + return page_node; +} + } // namespace TEST_F(FrameNodeImplTest, SafeDowncast) { @@ -142,6 +147,7 @@ class LenientMockObserver : public FrameNodeImpl::Observer { MOCK_METHOD2(OnPriorityAndReasonChanged, void(const FrameNode*, const PriorityAndReason& previous_value)); MOCK_METHOD1(OnHadFormInteractionChanged, void(const FrameNode*)); + MOCK_METHOD1(OnIsAudibleChanged, void(const FrameNode*)); MOCK_METHOD1(OnNonPersistentNotificationCreated, void(const FrameNode*)); MOCK_METHOD2(OnFirstContentfulPaint, void(const FrameNode*, base::TimeDelta)); @@ -343,6 +349,22 @@ TEST_F(FrameNodeImplTest, FormInteractions) { graph()->RemoveFrameNodeObserver(&obs); } +TEST_F(FrameNodeImplTest, IsAudible) { + auto process = CreateNode<ProcessNodeImpl>(); + auto page = CreateNode<PageNodeImpl>(); + auto frame_node = CreateFrameNodeAutoId(process.get(), page.get()); + EXPECT_FALSE(frame_node->is_audible()); + + MockObserver obs; + graph()->AddFrameNodeObserver(&obs); + + EXPECT_CALL(obs, OnIsAudibleChanged(frame_node.get())); + frame_node->SetIsAudible(true); + EXPECT_TRUE(frame_node->is_audible()); + + graph()->RemoveFrameNodeObserver(&obs); +} + TEST_F(FrameNodeImplTest, FirstContentfulPaint) { auto process = CreateNode<ProcessNodeImpl>(); auto page = CreateNode<PageNodeImpl>(); @@ -375,8 +397,7 @@ TEST_F(FrameNodeImplTest, PublicInterface) { public_frame_node->GetProcessNode()); EXPECT_EQ(frame_node->frame_tree_node_id(), public_frame_node->GetFrameTreeNodeId()); - EXPECT_EQ(frame_node->dev_tools_token(), - public_frame_node->GetDevToolsToken()); + EXPECT_EQ(frame_node->frame_token(), public_frame_node->GetFrameToken()); EXPECT_EQ(frame_node->browsing_instance_id(), public_frame_node->GetBrowsingInstanceId()); EXPECT_EQ(frame_node->site_instance_id(), @@ -438,4 +459,197 @@ TEST_F(FrameNodeImplTest, VisitChildFrameNodes) { EXPECT_EQ(1u, visited.size()); } +namespace { + +class LenientMockPageObserver : public PageNode::ObserverDefaultImpl { + public: + LenientMockPageObserver() = default; + ~LenientMockPageObserver() override = default; + + MOCK_METHOD1(OnBeforePageNodeRemoved, void(const PageNode* page_node)); + + // Note that opener functionality is actually tested in the + // performance_manager_browsertest. + MOCK_METHOD3(OnOpenerFrameNodeChanged, + void(const PageNode*, const FrameNode*, OpenedType)); +}; + +using MockPageObserver = ::testing::StrictMock<LenientMockPageObserver>; + +} // namespace + +TEST_F(FrameNodeImplTest, OpenerRelationships) { + using OpenedType = PageNode::OpenedType; + + auto process = CreateNode<ProcessNodeImpl>(); + auto pageA = CreateNode<PageNodeImpl>(); + auto frameA1 = CreateFrameNodeAutoId(process.get(), pageA.get()); + auto frameA2 = + CreateFrameNodeAutoId(process.get(), pageA.get(), frameA1.get()); + auto pageB = CreateNode<PageNodeImpl>(); + auto frameB1 = CreateFrameNodeAutoId(process.get(), pageB.get()); + auto pageC = CreateNode<PageNodeImpl>(); + auto frameC1 = CreateFrameNodeAutoId(process.get(), pageC.get()); + + // Use these to test the public APIs as well. + const FrameNode* pframeA1 = static_cast<const FrameNode*>(frameA1.get()); + const PageNode* ppageB = static_cast<const PageNode*>(pageB.get()); + + MockPageObserver obs; + graph()->AddPageNodeObserver(&obs); + + // You can always call the pre-delete opener clearing helper, even if you + // have no such relationships. + frameB1->SeverOpenedPagesAndMaybeReparentForTesting(); + + // You can't clear an opener if you don't already have one. + EXPECT_DCHECK_DEATH(pageB->ClearOpenerFrameNodeAndOpenedType()); + + // You can't be an opener for your own frame tree. + EXPECT_DCHECK_DEATH(pageA->SetOpenerFrameNodeAndOpenedType( + frameA1.get(), OpenedType::kPopup)); + + // You can't set a null opener or an invalid opened type. + EXPECT_DCHECK_DEATH( + pageB->SetOpenerFrameNodeAndOpenedType(nullptr, OpenedType::kInvalid)); + EXPECT_DCHECK_DEATH(pageB->SetOpenerFrameNodeAndOpenedType( + frameA1.get(), OpenedType::kInvalid)); + + EXPECT_EQ(nullptr, pageB->opener_frame_node()); + EXPECT_EQ(nullptr, ppageB->GetOpenerFrameNode()); + EXPECT_EQ(OpenedType::kInvalid, pageB->opened_type()); + EXPECT_EQ(OpenedType::kInvalid, ppageB->GetOpenedType()); + EXPECT_TRUE(frameA1->opened_page_nodes().empty()); + EXPECT_TRUE(pframeA1->GetOpenedPageNodes().empty()); + + // Set an opener relationship. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), nullptr, + OpenedType::kInvalid)); + pageB->SetOpenerFrameNodeAndOpenedType(frameA1.get(), OpenedType::kGuestView); + EXPECT_EQ(frameA1.get(), pageB->opener_frame_node()); + EXPECT_EQ(frameA1.get(), ppageB->GetOpenerFrameNode()); + EXPECT_EQ(OpenedType::kGuestView, pageB->opened_type()); + EXPECT_EQ(OpenedType::kGuestView, ppageB->GetOpenedType()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().size()); + EXPECT_EQ(1u, pframeA1->GetOpenedPageNodes().size()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().count(pageB.get())); + EXPECT_EQ(1u, pframeA1->GetOpenedPageNodes().count(pageB.get())); + testing::Mock::VerifyAndClear(&obs); + + // Set another opener relationship. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageC.get(), nullptr, + OpenedType::kInvalid)); + pageC->SetOpenerFrameNodeAndOpenedType(frameA1.get(), OpenedType::kPopup); + EXPECT_EQ(frameA1.get(), pageC->opener_frame_node()); + EXPECT_EQ(OpenedType::kPopup, pageC->opened_type()); + EXPECT_EQ(2u, frameA1->opened_page_nodes().size()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().count(pageB.get())); + testing::Mock::VerifyAndClear(&obs); + + // Do a traversal. + std::set<const PageNode*> visited; + EXPECT_TRUE( + ToPublic(frameA1.get()) + ->VisitOpenedPageNodes(base::BindRepeating( + [](std::set<const PageNode*>* visited, const PageNode* page) { + EXPECT_TRUE(visited->insert(page).second); + return true; + }, + base::Unretained(&visited)))); + EXPECT_THAT(visited, testing::UnorderedElementsAre(ToPublic(pageB.get()), + ToPublic(pageC.get()))); + + // Do an aborted visit. + visited.clear(); + EXPECT_FALSE( + ToPublic(frameA1.get()) + ->VisitOpenedPageNodes(base::BindRepeating( + [](std::set<const PageNode*>* visited, const PageNode* page) { + EXPECT_TRUE(visited->insert(page).second); + return false; + }, + base::Unretained(&visited)))); + EXPECT_EQ(1u, visited.size()); + + // Manually clear the first relationship (initiated from the page). + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), frameA1.get(), + OpenedType::kGuestView)); + pageB->ClearOpenerFrameNodeAndOpenedType(); + EXPECT_EQ(nullptr, pageB->opener_frame_node()); + EXPECT_EQ(OpenedType::kInvalid, pageB->opened_type()); + EXPECT_EQ(frameA1.get(), pageC->opener_frame_node()); + EXPECT_EQ(OpenedType::kPopup, pageC->opened_type()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().size()); + EXPECT_EQ(0u, frameA1->opened_page_nodes().count(pageB.get())); + testing::Mock::VerifyAndClear(&obs); + + // Clear the second relationship (initiated from the frame). + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageC.get(), frameA1.get(), + OpenedType::kPopup)); + frameA1->SeverOpenedPagesAndMaybeReparentForTesting(); + EXPECT_EQ(nullptr, pageC->opener_frame_node()); + EXPECT_EQ(OpenedType::kInvalid, pageC->opened_type()); + EXPECT_TRUE(frameA1->opened_page_nodes().empty()); + testing::Mock::VerifyAndClear(&obs); + + // Set a popup opener relationship on node A2. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), nullptr, + OpenedType::kInvalid)); + pageB->SetOpenerFrameNodeAndOpenedType(frameA2.get(), OpenedType::kPopup); + EXPECT_EQ(frameA2.get(), pageB->opener_frame_node()); + EXPECT_EQ(OpenedType::kPopup, pageB->opened_type()); + EXPECT_TRUE(frameA1->opened_page_nodes().empty()); + EXPECT_EQ(1u, frameA2->opened_page_nodes().size()); + EXPECT_EQ(1u, frameA2->opened_page_nodes().count(pageB.get())); + testing::Mock::VerifyAndClear(&obs); + + // Clear it with the helper, and expect it to be reparented to node A1. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), frameA2.get(), + OpenedType::kPopup)); + frameA2->SeverOpenedPagesAndMaybeReparentForTesting(); + EXPECT_EQ(frameA1.get(), pageB->opener_frame_node()); + EXPECT_EQ(OpenedType::kPopup, pageB->opened_type()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().size()); + EXPECT_EQ(1u, frameA1->opened_page_nodes().count(pageB.get())); + EXPECT_TRUE(frameA2->opened_page_nodes().empty()); + testing::Mock::VerifyAndClear(&obs); + + // Clear it again with the helper. This time reparenting can't happen, as it + // was already parented to the root. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), frameA1.get(), + OpenedType::kPopup)); + frameA1->SeverOpenedPagesAndMaybeReparentForTesting(); + EXPECT_EQ(nullptr, pageB->opener_frame_node()); + EXPECT_EQ(OpenedType::kInvalid, pageB->opened_type()); + EXPECT_TRUE(frameA1->opened_page_nodes().empty()); + EXPECT_TRUE(frameA2->opened_page_nodes().empty()); + testing::Mock::VerifyAndClear(&obs); + + // verify that the opener relationship is torn down before any node removal + // notification arrives. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), nullptr, + OpenedType::kInvalid)); + pageB->SetOpenerFrameNodeAndOpenedType(frameA2.get(), OpenedType::kPopup); + EXPECT_EQ(frameA2.get(), pageB->opener_frame_node()); + EXPECT_EQ(OpenedType::kPopup, pageB->opened_type()); + EXPECT_TRUE(frameA1->opened_page_nodes().empty()); + EXPECT_EQ(1u, frameA2->opened_page_nodes().size()); + EXPECT_EQ(1u, frameA2->opened_page_nodes().count(pageB.get())); + testing::Mock::VerifyAndClear(&obs); + + { + ::testing::InSequence seq; + + // These must occur in sequence. + EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), frameA2.get(), + OpenedType::kPopup)); + EXPECT_CALL(obs, OnBeforePageNodeRemoved(pageB.get())); + } + frameB1.reset(); + pageB.reset(); + testing::Mock::VerifyAndClear(&obs); + + graph()->RemovePageNodeObserver(&obs); +} + } // namespace performance_manager diff --git a/chromium/components/performance_manager/graph/graph_impl.cc b/chromium/components/performance_manager/graph/graph_impl.cc index 30c1fabef64..81425e79a14 100644 --- a/chromium/components/performance_manager/graph/graph_impl.cc +++ b/chromium/components/performance_manager/graph/graph_impl.cc @@ -153,7 +153,8 @@ GraphImpl::GraphImpl() { GraphImpl::~GraphImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - // All graph registered objects should have been unregistered. + // All graph registered and owned objects should have been cleaned up. + DCHECK(graph_owned_.empty()); DCHECK(registered_objects_.empty()); // At this point, all typed observers should be empty. @@ -180,8 +181,7 @@ void GraphImpl::TearDown() { // Clean up graph owned objects. This causes their TakeFromGraph callbacks to // be invoked, and ideally they clean up any observers they may have, etc. - while (!graph_owned_.empty()) - auto object = TakeFromGraph(graph_owned_.begin()->first); + graph_owned_.ReleaseObjects(this); // At this point, all typed observers should be empty. DCHECK(graph_observers_.empty()); @@ -258,39 +258,22 @@ void GraphImpl::RemoveWorkerNodeObserver(WorkerNodeObserver* observer) { void GraphImpl::PassToGraph(std::unique_ptr<GraphOwned> graph_owned) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - auto* raw = graph_owned.get(); - DCHECK(!base::Contains(graph_owned_, raw)); - graph_owned_.insert(std::make_pair(raw, std::move(graph_owned))); - raw->OnPassedToGraph(this); + graph_owned_.PassObject(std::move(graph_owned), this); } std::unique_ptr<GraphOwned> GraphImpl::TakeFromGraph(GraphOwned* graph_owned) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - std::unique_ptr<GraphOwned> object; - auto it = graph_owned_.find(graph_owned); - if (it != graph_owned_.end()) { - DCHECK_EQ(graph_owned, it->first); - DCHECK_EQ(graph_owned, it->second.get()); - object = std::move(it->second); - graph_owned_.erase(it); - object->OnTakenFromGraph(this); - } - return object; + return graph_owned_.TakeObject(graph_owned, this); } void GraphImpl::RegisterObject(GraphRegistered* object) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK_EQ(nullptr, GetRegisteredObject(object->GetTypeId())); - registered_objects_.insert(object); - // If there are ever so many registered objects we should consider changing - // data structures. - DCHECK_GT(100u, registered_objects_.size()); + registered_objects_.RegisterObject(object); } void GraphImpl::UnregisterObject(GraphRegistered* object) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK_EQ(object, GetRegisteredObject(object->GetTypeId())); - registered_objects_.erase(object); + registered_objects_.UnregisterObject(object); } const SystemNode* GraphImpl::FindOrCreateSystemNode() { @@ -344,11 +327,7 @@ const void* GraphImpl::GetImpl() const { GraphRegistered* GraphImpl::GetRegisteredObject(uintptr_t type_id) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - auto it = registered_objects_.find(type_id); - if (it == registered_objects_.end()) - return nullptr; - DCHECK_EQ((*it)->GetTypeId(), type_id); - return *it; + return registered_objects_.GetRegisteredObject(type_id); } // static @@ -396,6 +375,10 @@ void GraphImpl::OnNodeAdded(NodeBase* node) { void GraphImpl::OnBeforeNodeRemoved(NodeBase* node) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // Clear any node-specific state and issue the relevant notifications before + // sending the last-gasp removal notification for this node. + node->OnBeforeLeavingGraph(); + // This handles the strongly typed observer notifications. switch (node->type()) { case NodeTypeEnum::kFrame: { @@ -524,8 +507,9 @@ void GraphImpl::AddNewNode(NodeBase* new_node) { auto it = nodes_.insert(new_node); DCHECK(it.second); // Inserted successfully - // Allow the node to initialize itself now that it's been added. + // Add the node to the graph and allow it to initialize itself. new_node->JoinGraph(this); + new_node->OnJoiningGraph(); // Then notify observers. OnNodeAdded(new_node); diff --git a/chromium/components/performance_manager/graph/graph_impl.h b/chromium/components/performance_manager/graph/graph_impl.h index fc4232ab05d..158459ee706 100644 --- a/chromium/components/performance_manager/graph/graph_impl.h +++ b/chromium/components/performance_manager/graph/graph_impl.h @@ -19,9 +19,11 @@ #include "base/macros.h" #include "base/process/process_handle.h" #include "base/sequence_checker.h" +#include "components/performance_manager/owned_objects.h" #include "components/performance_manager/public/graph/graph.h" #include "components/performance_manager/public/graph/graph_registered.h" #include "components/performance_manager/public/graph/node_attached_data.h" +#include "components/performance_manager/registered_objects.h" #include "services/metrics/public/cpp/ukm_recorder.h" namespace performance_manager { @@ -187,7 +189,11 @@ class GraphImpl : public Graph { // Graph-owned objects. For now we only expect O(10) clients, hence the // flat_map. - base::flat_map<GraphOwned*, std::unique_ptr<GraphOwned>> graph_owned_; + OwnedObjects<GraphOwned, + /* CallbackArgType = */ Graph*, + &GraphOwned::OnPassedToGraph, + &GraphOwned::OnTakenFromGraph> + graph_owned_; // Allocated on first use. mutable std::unique_ptr<NodeDataDescriberRegistry> describer_registry_; @@ -199,28 +205,8 @@ class GraphImpl : public Graph { std::map<NodeAttachedDataKey, std::unique_ptr<NodeAttachedData>>; NodeAttachedDataMap node_attached_data_map_; - // Comparator for GraphRegistered objects, which sorts by TypeId. This is a - // transparent comparator (see base::flat_tree) which allows comparing - // GraphRegistered objects and TypeIds with each other. - struct GraphRegisteredComparator { - using is_transparent = void; - bool operator()(const GraphRegistered* gr1, - const GraphRegistered* gr2) const { - return gr1->GetTypeId() < gr2->GetTypeId(); - } - bool operator()(const GraphRegistered* gr1, uintptr_t type_id) const { - return gr1->GetTypeId() < type_id; - } - bool operator()(uintptr_t type_id, const GraphRegistered* gr2) const { - return type_id < gr2->GetTypeId(); - } - }; - - // Storage for GraphRegistered objects. They are stored by pointer to object, - // but sorted by their uintptr_t TypeIds. These must all be unregistered - // before this object is destroyed. - base::flat_set<GraphRegistered*, GraphRegisteredComparator> - registered_objects_; + // Storage for GraphRegistered objects. + RegisteredObjects<GraphRegistered> registered_objects_; // The most recently assigned serialization ID. int64_t current_node_serialization_id_ = 0u; diff --git a/chromium/components/performance_manager/graph/graph_impl_unittest.cc b/chromium/components/performance_manager/graph/graph_impl_unittest.cc index bee42687424..05d5de1f915 100644 --- a/chromium/components/performance_manager/graph/graph_impl_unittest.cc +++ b/chromium/components/performance_manager/graph/graph_impl_unittest.cc @@ -322,4 +322,24 @@ TEST_F(GraphImplTest, NodeDataDescribers) { EXPECT_EQ(0u, descr.DictSize()); } +TEST_F(GraphImplTest, OpenersClearedOnTeardown) { + auto process = CreateNode<ProcessNodeImpl>(); + auto pageA = CreateNode<PageNodeImpl>(); + auto frameA1 = CreateFrameNodeAutoId(process.get(), pageA.get()); + auto frameA2 = + CreateFrameNodeAutoId(process.get(), pageA.get(), frameA1.get()); + auto pageB = CreateNode<PageNodeImpl>(); + auto frameB1 = CreateFrameNodeAutoId(process.get(), pageB.get()); + auto pageC = CreateNode<PageNodeImpl>(); + auto frameC1 = CreateFrameNodeAutoId(process.get(), pageC.get()); + + // Set up some opener relationships. These should be gracefully torn down as + // the graph cleans up nodes, otherwise the frame and page node destructors + // will explode. + pageB->SetOpenerFrameNodeAndOpenedType(frameA1.get(), + PageNode::OpenedType::kGuestView); + pageC->SetOpenerFrameNodeAndOpenedType(frameA2.get(), + PageNode::OpenedType::kPopup); +} + } // namespace performance_manager diff --git a/chromium/components/performance_manager/graph/graph_registered_unittest.cc b/chromium/components/performance_manager/graph/graph_registered_unittest.cc index de6737571ad..e7b6a2450e1 100644 --- a/chromium/components/performance_manager/graph/graph_registered_unittest.cc +++ b/chromium/components/performance_manager/graph/graph_registered_unittest.cc @@ -5,7 +5,10 @@ #include "components/performance_manager/public/graph/graph_registered.h" #include "base/test/gtest_util.h" +#include "components/performance_manager/performance_manager_registry_impl.h" +#include "components/performance_manager/public/performance_manager.h" #include "components/performance_manager/test_support/graph_test_harness.h" +#include "components/performance_manager/test_support/performance_manager_test_harness.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -91,4 +94,56 @@ TEST_F(GraphRegisteredTest, GraphRegistrationWorks) { graph()->UnregisterObject(&foo); } +namespace { + +// This class is non-sensically both a GraphRegistered object and a +// PerformanceManagerRegistered object. This is done to ensure that the +// implementations use the appropriately typed "TypeId" functions, as there are +// now two of them available! +class Baz : public GraphRegisteredImpl<Baz>, + public PerformanceManagerRegisteredImpl<Baz> { + public: + Baz() = default; + ~Baz() override = default; +}; + +using GraphAndPerformanceManagerRegisteredTest = PerformanceManagerTestHarness; + +} // namespace + +TEST_F(GraphAndPerformanceManagerRegisteredTest, GraphAndPMRegistered) { + // Each TypeId should be backed by a distinct "TypeId" implementation and + // value. + const uintptr_t kGraphId = GraphRegisteredImpl<Baz>::TypeId(); + const uintptr_t kPMId = PerformanceManagerRegisteredImpl<Baz>::TypeId(); + ASSERT_NE(kGraphId, kPMId); + + // Create a stand-alone graph that is bound to this sequence so we can test + // both the PM and a graph on the same sequence. + std::unique_ptr<GraphImpl> graph(new GraphImpl()); + + PerformanceManagerRegistryImpl* registry = + PerformanceManagerRegistryImpl::GetInstance(); + + Baz baz; + graph->RegisterObject(&baz); + registry->RegisterObject(&baz); + + EXPECT_EQ(&baz, graph->GetRegisteredObject(kGraphId)); + EXPECT_EQ(&baz, registry->GetRegisteredObject(kPMId)); + EXPECT_EQ(nullptr, graph->GetRegisteredObject(kPMId)); + EXPECT_EQ(nullptr, registry->GetRegisteredObject(kGraphId)); + + // This directly tests that the templated helper function uses the correct + // instance of TypeId. + EXPECT_EQ(&baz, graph->GetRegisteredObjectAs<Baz>()); + EXPECT_EQ(&baz, PerformanceManager::GetRegisteredObjectAs<Baz>()); + + graph->UnregisterObject(&baz); + registry->UnregisterObject(&baz); + + graph->TearDown(); + graph.reset(); +} + } // namespace performance_manager
\ No newline at end of file diff --git a/chromium/components/performance_manager/graph/node_attached_data.h b/chromium/components/performance_manager/graph/node_attached_data.h index 6256f169fe9..3f432bb70a7 100644 --- a/chromium/components/performance_manager/graph/node_attached_data.h +++ b/chromium/components/performance_manager/graph/node_attached_data.h @@ -7,7 +7,7 @@ #include <memory> -#include "base/logging.h" +#include "base/check_op.h" #include "base/macros.h" #include "components/performance_manager/graph/node_base.h" #include "components/performance_manager/public/graph/node_attached_data.h" diff --git a/chromium/components/performance_manager/graph/node_attached_data_impl.h b/chromium/components/performance_manager/graph/node_attached_data_impl.h index 147244f6f3e..6c2185980ee 100644 --- a/chromium/components/performance_manager/graph/node_attached_data_impl.h +++ b/chromium/components/performance_manager/graph/node_attached_data_impl.h @@ -9,7 +9,7 @@ #include <type_traits> #include <utility> -#include "base/logging.h" +#include "base/check_op.h" #include "base/memory/ptr_util.h" #include "components/performance_manager/graph/node_attached_data.h" diff --git a/chromium/components/performance_manager/graph/node_base.cc b/chromium/components/performance_manager/graph/node_base.cc index e458a8eb016..36d28adb73e 100644 --- a/chromium/components/performance_manager/graph/node_base.cc +++ b/chromium/components/performance_manager/graph/node_base.cc @@ -40,8 +40,6 @@ void NodeBase::JoinGraph(GraphImpl* graph) { DCHECK(graph->NodeInGraph(this)); graph_ = graph; - - OnJoiningGraph(); } void NodeBase::LeaveGraph() { @@ -49,8 +47,6 @@ void NodeBase::LeaveGraph() { DCHECK(graph_); DCHECK(graph_->NodeInGraph(this)); - OnBeforeLeavingGraph(); - graph_ = nullptr; } diff --git a/chromium/components/performance_manager/graph/node_base.h b/chromium/components/performance_manager/graph/node_base.h index a37e31687b7..0a05e7b070c 100644 --- a/chromium/components/performance_manager/graph/node_base.h +++ b/chromium/components/performance_manager/graph/node_base.h @@ -73,13 +73,10 @@ class NodeBase { return graph->GetObservers<Observer>(); } - // Joins the |graph|. Assigns |graph_| and invokes OnJoiningGraph() to allow - // subclasses to initialize. + // Joins the |graph|. void JoinGraph(GraphImpl* graph); - // Leaves the graph that this node is a part of. Invokes - // OnBeforeLeavingGraph() to allow subclasses to uninitialize then clears - // |graph_|. + // Leaves the graph that this node is a part of. void LeaveGraph(); // Called as this node is joining |graph_|, a good opportunity to initialize diff --git a/chromium/components/performance_manager/graph/page_node.cc b/chromium/components/performance_manager/graph/page_node.cc index 772581fa119..113fcf4c684 100644 --- a/chromium/components/performance_manager/graph/page_node.cc +++ b/chromium/components/performance_manager/graph/page_node.cc @@ -8,6 +8,21 @@ namespace performance_manager { +// static +const char* PageNode::ToString(PageNode::OpenedType opened_type) { + switch (opened_type) { + case PageNode::OpenedType::kInvalid: + return "kInvalid"; + case PageNode::OpenedType::kPopup: + return "kPopup"; + case PageNode::OpenedType::kGuestView: + return "kGuestView"; + case PageNode::OpenedType::kPortal: + return "kPortal"; + } + NOTREACHED(); +} + PageNode::PageNode() = default; PageNode::~PageNode() = default; @@ -18,3 +33,10 @@ PageNode::ObserverDefaultImpl::ObserverDefaultImpl() = default; PageNode::ObserverDefaultImpl::~ObserverDefaultImpl() = default; } // namespace performance_manager + +std::ostream& operator<<( + std::ostream& os, + performance_manager::PageNode::OpenedType opened_type) { + os << performance_manager::PageNode::ToString(opened_type); + return os; +}
\ No newline at end of file diff --git a/chromium/components/performance_manager/graph/page_node_impl.cc b/chromium/components/performance_manager/graph/page_node_impl.cc index b64294cd990..14ec570a3fa 100644 --- a/chromium/components/performance_manager/graph/page_node_impl.cc +++ b/chromium/components/performance_manager/graph/page_node_impl.cc @@ -34,6 +34,8 @@ PageNodeImpl::PageNodeImpl(const WebContentsProxy& contents_proxy, PageNodeImpl::~PageNodeImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK_EQ(nullptr, opener_frame_node_); + DCHECK_EQ(OpenedType::kInvalid, opened_type_); } const WebContentsProxy& PageNodeImpl::contents_proxy() const { @@ -150,6 +152,18 @@ FrameNodeImpl* PageNodeImpl::GetMainFrameNodeImpl() const { return *main_frame_nodes_.begin(); } +FrameNodeImpl* PageNodeImpl::opener_frame_node() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK(opener_frame_node_ || opened_type_ == OpenedType::kInvalid); + return opener_frame_node_; +} + +PageNodeImpl::OpenedType PageNodeImpl::opened_type() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK(opener_frame_node_ || opened_type_ == OpenedType::kInvalid); + return opened_type_; +} + bool PageNodeImpl::is_visible() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return is_visible_.value(); @@ -231,6 +245,43 @@ bool PageNodeImpl::had_form_interaction() const { return had_form_interaction_.value(); } +void PageNodeImpl::SetOpenerFrameNodeAndOpenedType(FrameNodeImpl* opener, + OpenedType opened_type) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK(opener); + DCHECK(graph()->NodeInGraph(opener)); + DCHECK_NE(this, opener->page_node()); + DCHECK_NE(OpenedType::kInvalid, opened_type); + + auto* previous_opener = opener_frame_node_; + auto previous_type = opened_type_; + + if (previous_opener) + previous_opener->RemoveOpenedPage(PassKey(), this); + opener_frame_node_ = opener; + opened_type_ = opened_type; + opener->AddOpenedPage(PassKey(), this); + + for (auto* observer : GetObservers()) + observer->OnOpenerFrameNodeChanged(this, previous_opener, previous_type); +} + +void PageNodeImpl::ClearOpenerFrameNodeAndOpenedType() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DCHECK_NE(nullptr, opener_frame_node_); + DCHECK_NE(OpenedType::kInvalid, opened_type_); + + auto* previous_opener = opener_frame_node_; + auto previous_type = opened_type_; + + opener_frame_node_->RemoveOpenedPage(PassKey(), this); + opener_frame_node_ = nullptr; + opened_type_ = OpenedType::kInvalid; + + for (auto* observer : GetObservers()) + observer->OnOpenerFrameNodeChanged(this, previous_opener, previous_type); +} + void PageNodeImpl::set_usage_estimate_time( base::TimeTicks usage_estimate_time) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -262,6 +313,10 @@ void PageNodeImpl::OnJoiningGraph() { void PageNodeImpl::OnBeforeLeavingGraph() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // Sever opener relationships. + if (opener_frame_node_) + ClearOpenerFrameNodeAndOpenedType(); + DCHECK_EQ(0u, frame_node_count_); } @@ -270,6 +325,16 @@ const std::string& PageNodeImpl::GetBrowserContextID() const { return browser_context_id(); } +const FrameNode* PageNodeImpl::GetOpenerFrameNode() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return opener_frame_node(); +} + +PageNodeImpl::OpenedType PageNodeImpl::GetOpenedType() const { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return opened_type(); +} + bool PageNodeImpl::IsVisible() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return is_visible(); diff --git a/chromium/components/performance_manager/graph/page_node_impl.h b/chromium/components/performance_manager/graph/page_node_impl.h index 3c391831177..91197591060 100644 --- a/chromium/components/performance_manager/graph/page_node_impl.h +++ b/chromium/components/performance_manager/graph/page_node_impl.h @@ -13,6 +13,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/time/time.h" +#include "base/util/type_safety/pass_key.h" #include "components/performance_manager/graph/node_attached_data.h" #include "components/performance_manager/graph/node_base.h" #include "components/performance_manager/public/graph/page_node.h" @@ -27,6 +28,8 @@ class PageNodeImpl : public PublicNodeImpl<PageNodeImpl, PageNode>, public TypedNodeBase<PageNodeImpl, PageNode, PageNodeObserver> { public: + using PassKey = util::PassKey<PageNodeImpl>; + static constexpr NodeTypeEnum Type() { return NodeTypeEnum::kPage; } PageNodeImpl(const WebContentsProxy& contents_proxy, @@ -70,6 +73,8 @@ class PageNodeImpl // Accessors. const std::string& browser_context_id() const; + FrameNodeImpl* opener_frame_node() const; + OpenedType opened_type() const; bool is_visible() const; bool is_audible() const; bool is_loading() const; @@ -86,6 +91,11 @@ class PageNodeImpl const std::string& contents_mime_type() const; bool had_form_interaction() const; + // Invoked to set/clear the opener of this page. + void SetOpenerFrameNodeAndOpenedType(FrameNodeImpl* opener, + OpenedType opened_type); + void ClearOpenerFrameNodeAndOpenedType(); + void set_usage_estimate_time(base::TimeTicks usage_estimate_time); void set_private_footprint_kb_estimate( uint64_t private_footprint_kb_estimate); @@ -113,9 +123,12 @@ class PageNodeImpl friend class PageAggregatorAccess; friend class PageLoadTrackerAccess; friend class PageNodeImplDescriber; + friend class SiteDataAccess; // PageNode implementation. const std::string& GetBrowserContextID() const override; + const FrameNode* GetOpenerFrameNode() const override; + OpenedType GetOpenedType() const override; bool IsVisible() const override; base::TimeDelta GetTimeSinceLastVisibilityChange() const override; bool IsAudible() const override; @@ -196,6 +209,12 @@ class PageNodeImpl // The unique ID of the browser context that this page belongs to. const std::string browser_context_id_; + // The opener of this page, if there is one. + FrameNodeImpl* opener_frame_node_ = nullptr; + + // The way in which this page was opened, if it was opened. + OpenedType opened_type_ = OpenedType::kInvalid; + // Whether or not the page is visible. Driven by browser instrumentation. // Initialized on construction. ObservedProperty::NotifiesOnlyOnChanges<bool, @@ -249,6 +268,9 @@ class PageNodeImpl // Storage for PageLoadTracker user data. std::unique_ptr<NodeAttachedData> page_load_tracker_data_; + // Storage for SiteDataNodeData user data. + std::unique_ptr<NodeAttachedData> site_data_; + // Inline storage for FrozenFrameAggregator user data. InternalNodeAttachedDataStorage<sizeof(uintptr_t) + 8> frozen_frame_data_; diff --git a/chromium/components/performance_manager/graph/page_node_impl_describer.cc b/chromium/components/performance_manager/graph/page_node_impl_describer.cc index 8b04bc72769..6032b968847 100644 --- a/chromium/components/performance_manager/graph/page_node_impl_describer.cc +++ b/chromium/components/performance_manager/graph/page_node_impl_describer.cc @@ -76,6 +76,10 @@ base::Value PageNodeImplDescriber::DescribePageNodeData( page_node_impl->is_holding_indexeddb_lock_.value()); result.SetBoolKey("had_form_interaction", page_node_impl->had_form_interaction_.value()); + if (page_node_impl->opened_type_ != PageNode::OpenedType::kInvalid) { + result.SetStringKey("opened_type", + PageNode::ToString(page_node_impl->opened_type_)); + } return result; } diff --git a/chromium/components/performance_manager/graph/page_node_impl_unittest.cc b/chromium/components/performance_manager/graph/page_node_impl_unittest.cc index 6283d5f6c7a..08cdcfb9755 100644 --- a/chromium/components/performance_manager/graph/page_node_impl_unittest.cc +++ b/chromium/components/performance_manager/graph/page_node_impl_unittest.cc @@ -201,6 +201,10 @@ class LenientMockObserver : public PageNodeImpl::Observer { MOCK_METHOD1(OnPageNodeAdded, void(const PageNode*)); MOCK_METHOD1(OnBeforePageNodeRemoved, void(const PageNode*)); + // Note that opener functionality is actually tested in the FrameNodeImpl + // and GraphImpl unittests. + MOCK_METHOD3(OnOpenerFrameNodeChanged, + void(const PageNode*, const FrameNode*, OpenedType)); MOCK_METHOD1(OnIsVisibleChanged, void(const PageNode*)); MOCK_METHOD1(OnIsAudibleChanged, void(const PageNode*)); MOCK_METHOD1(OnIsLoadingChanged, void(const PageNode*)); diff --git a/chromium/components/performance_manager/graph/policies/process_priority_policy.cc b/chromium/components/performance_manager/graph/policies/process_priority_policy.cc index 91c79a717fb..cfc313d4b4b 100644 --- a/chromium/components/performance_manager/graph/policies/process_priority_policy.cc +++ b/chromium/components/performance_manager/graph/policies/process_priority_policy.cc @@ -6,7 +6,6 @@ #include "base/bind.h" #include "base/memory/ptr_util.h" -#include "base/task/post_task.h" #include "components/performance_manager/public/render_process_host_proxy.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" @@ -69,8 +68,8 @@ void DispatchSetProcessPriority(const ProcessNode* process_node, // driven 100% from the PM, we could post directly to the launcher thread // via the base::Process directly. const auto& proxy = process_node->GetRenderProcessHostProxy(); - base::PostTask( - FROM_HERE, {content::BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&SetProcessPriorityOnUIThread, proxy, foreground)); } diff --git a/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy.cc b/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy.cc index 8b27ecc2e6f..22bedb9e0bb 100644 --- a/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy.cc +++ b/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy.cc @@ -6,7 +6,6 @@ #include "base/bind.h" #include "base/no_destructor.h" -#include "base/task/post_task.h" #include "base/task/task_traits.h" #include "components/performance_manager/graph/page_node_impl.h" #include "components/performance_manager/public/features.h" @@ -60,6 +59,7 @@ TabLoadingFrameNavigationPolicy::TabLoadingFrameNavigationPolicy() auto params = features::TabLoadingFrameNavigationThrottlesParams::GetParams(); timeout_min_ = params.minimum_throttle_timeout; timeout_max_ = params.maximum_throttle_timeout; + fcp_multiple_ = params.fcp_multiple; } TabLoadingFrameNavigationPolicy::~TabLoadingFrameNavigationPolicy() { @@ -123,6 +123,24 @@ bool TabLoadingFrameNavigationPolicy::ShouldThrottleNavigation( return true; } +base::TimeTicks TabLoadingFrameNavigationPolicy::GetPageTimeoutForTesting( + const PageNode* page_node) const { + size_t i = 0; + for (; i < timeouts_.size(); ++i) { + if (timeouts_[i].page_node == page_node) + return timeouts_[i].timeout; + } + return base::TimeTicks(); +} + +base::TimeDelta TabLoadingFrameNavigationPolicy::CalculateTimeoutFromFCP( + base::TimeDelta fcp) const { + // No need to cap the timeout with |timeout_max_|, as the timeout starts with + // that by default, and timeout updates can only make the timeout decrease. + // See MaybeUpdatePageTimeout for details. + return std::max(fcp * (fcp_multiple_ - 1.0), timeout_min_); +} + void TabLoadingFrameNavigationPolicy::OnBeforePageNodeRemoved( const PageNode* page_node) { // There's no public graph accessor. We could cache this in OnPassedToGraph, @@ -142,22 +160,19 @@ void TabLoadingFrameNavigationPolicy::OnFirstContentfulPaint( if (!frame_node->IsMainFrame() || !frame_node->IsCurrent()) return; - // Wait for another FCP time period, but enforce a lower bound. - double fcp_ms = time_since_navigation_start.InMillisecondsF(); - double delta_ms = std::max(timeout_min_.InMillisecondsF(), fcp_ms); + // Update the timer if needed. MaybeUpdatePageTimeout(frame_node->GetPageNode(), - base::TimeDelta::FromMillisecondsD(delta_ms)); + CalculateTimeoutFromFCP(time_since_navigation_start)); } void TabLoadingFrameNavigationPolicy::OnPassedToGraph(Graph* graph) { DCHECK(NothingRegistered(graph)); - base::PostTask(FROM_HERE, - {content::BrowserThread::UI, base::TaskPriority::USER_VISIBLE}, - base::BindOnce( - [](MechanismDelegate* mechanism) { - mechanism->SetThrottlingEnabled(true); - }, - base::Unretained(mechanism_))); + content::GetUIThreadTaskRunner({base::TaskPriority::USER_VISIBLE}) + ->PostTask(FROM_HERE, base::BindOnce( + [](MechanismDelegate* mechanism) { + mechanism->SetThrottlingEnabled(true); + }, + base::Unretained(mechanism_))); graph->AddFrameNodeObserver(this); graph->AddPageNodeObserver(this); graph->RegisterObject(this); @@ -165,13 +180,12 @@ void TabLoadingFrameNavigationPolicy::OnPassedToGraph(Graph* graph) { void TabLoadingFrameNavigationPolicy::OnTakenFromGraph(Graph* graph) { DCHECK(IsRegistered(graph)); - base::PostTask(FROM_HERE, - {content::BrowserThread::UI, base::TaskPriority::USER_VISIBLE}, - base::BindOnce( - [](MechanismDelegate* mechanism) { - mechanism->SetThrottlingEnabled(false); - }, - base::Unretained(mechanism_))); + content::GetUIThreadTaskRunner({base::TaskPriority::USER_VISIBLE}) + ->PostTask(FROM_HERE, base::BindOnce( + [](MechanismDelegate* mechanism) { + mechanism->SetThrottlingEnabled(false); + }, + base::Unretained(mechanism_))); graph->UnregisterObject(this); graph->RemovePageNodeObserver(this); graph->RemoveFrameNodeObserver(this); @@ -232,7 +246,7 @@ void TabLoadingFrameNavigationPolicy::CreatePageTimeout( void TabLoadingFrameNavigationPolicy::MaybeUpdatePageTimeout( const PageNode* page_node, base::TimeDelta timeout) { - // Find or create an entry for the given |page_node|. + // Find the entry for the given |page_node|. size_t i = 0; for (; i < timeouts_.size(); ++i) { if (timeouts_[i].page_node == page_node) @@ -324,16 +338,18 @@ void TabLoadingFrameNavigationPolicy::StopThrottlingExpiredPages() { // the contents. Note that |mechanism_| is expected to effectively live // forever (it is only a testing seam, in production it is a static // singleton), so passing base::Unretained is safe. - base::PostTask( - FROM_HERE, - {content::BrowserThread::UI, base::TaskPriority::USER_VISIBLE}, - base::BindOnce( - [](MechanismDelegate* mechanism, const WebContentsProxy& proxy) { - auto* contents = proxy.Get(); - if (contents) - mechanism->StopThrottling(contents, proxy.LastNavigationId()); - }, - base::Unretained(mechanism_), page_node->GetContentsProxy())); + content::GetUIThreadTaskRunner({base::TaskPriority::USER_VISIBLE}) + ->PostTask(FROM_HERE, base::BindOnce( + [](MechanismDelegate* mechanism, + const WebContentsProxy& proxy) { + auto* contents = proxy.Get(); + if (contents) + mechanism->StopThrottling( + contents, + proxy.LastNewDocNavigationId()); + }, + base::Unretained(mechanism_), + page_node->GetContentsProxy())); } } diff --git a/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy_unittest.cc b/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy_unittest.cc index 6332d5a19a7..262cf8edf03 100644 --- a/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy_unittest.cc +++ b/chromium/components/performance_manager/graph/policies/tab_loading_frame_navigation_policy_unittest.cc @@ -5,16 +5,17 @@ #include "components/performance_manager/public/graph/policies/tab_loading_frame_navigation_policy.h" #include "base/bind.h" -#include "base/task/post_task.h" #include "base/task/task_traits.h" #include "base/test/bind_test_util.h" #include "base/test/scoped_feature_list.h" #include "base/test/task_environment.h" +#include "components/performance_manager/performance_manager_tab_helper.h" #include "components/performance_manager/public/features.h" #include "components/performance_manager/public/performance_manager.h" #include "components/performance_manager/test_support/performance_manager_test_harness.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/navigation_handle.h" #include "content/public/browser/web_contents.h" #include "content/public/test/mock_navigation_handle.h" #include "content/public/test/navigation_simulator.h" @@ -95,6 +96,28 @@ class TabLoadingFrameNavigationPolicyTest quit_closure_.Reset(); } + // Returns the scheduled page timeout after simulating FCP. + base::TimeTicks SimulateFirstContentfulPaint(content::WebContents* contents, + base::TimeDelta fcp) { + base::RunLoop run_loop; + base::TimeTicks timeout; + + auto frame = PerformanceManager::GetFrameNodeForRenderFrameHost( + contents->GetMainFrame()); + PerformanceManager::CallOnGraph( + FROM_HERE, + base::BindLambdaForTesting( + [policy = policy(), fcp, frame, &run_loop, &timeout](Graph* graph) { + EXPECT_TRUE(frame.get()); + policy->OnFirstContentfulPaintForTesting(frame.get(), fcp); + timeout = policy->GetPageTimeoutForTesting(frame->GetPageNode()); + run_loop.Quit(); + })); + + run_loop.Run(); + return timeout; + } + void ExpectThrottledPageCount(size_t expected_throttled_page_count) { base::RunLoop run_loop; @@ -120,8 +143,8 @@ class TabLoadingFrameNavigationPolicyTest // Post back to the UI thread with the answer, where it will be // validated and the run loop exited. - base::PostTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(on_ui_thread, throttled_page_count, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(on_ui_thread, throttled_page_count, timer_running)); })); @@ -131,7 +154,8 @@ class TabLoadingFrameNavigationPolicyTest // The MechanismDelegate calls redirect here. MOCK_METHOD1(OnSetThrottlingEnabled, void(bool)); - MOCK_METHOD1(OnStopThrottling, void(content::WebContents*)); + MOCK_METHOD2(OnStopThrottling, + void(content::WebContents*, int64_t last_navigation_id)); // Accessors. TabLoadingFrameNavigationPolicy* policy() const { return policy_; } @@ -154,8 +178,8 @@ class TabLoadingFrameNavigationPolicyTest quit_closure_.Run(); } void StopThrottling(content::WebContents* contents, - int64_t last_navigation_id_unused) override { - OnStopThrottling(contents); + int64_t last_navigation_id) override { + OnStopThrottling(contents, last_navigation_id); // Time can be manually advanced as well, so we're not always in a RunLoop. // Only try to invoke the quit closure if it has been set. @@ -169,6 +193,35 @@ class TabLoadingFrameNavigationPolicyTest base::TimeTicks start_; }; +// Navigates and commits from the browser, returning the navigation id. +int64_t NavigateAndCommitFromBrowser(content::WebContents* contents, + GURL gurl) { + std::unique_ptr<content::NavigationSimulator> simulator( + content::NavigationSimulator::CreateBrowserInitiated(gurl, contents)); + simulator->Start(); + int64_t navigation_id = simulator->GetNavigationHandle()->GetNavigationId(); + simulator->Commit(); + auto* pmth = PerformanceManagerTabHelper::FromWebContents(contents); + CHECK(pmth); + EXPECT_EQ(pmth->LastNavigationId(), navigation_id); + EXPECT_EQ(pmth->LastNewDocNavigationId(), navigation_id); + return navigation_id; +} + +// Navigates and commits a same document navigation from the renderer. +// Returns the navigation id. +int64_t NavigateAndCommitSameDocFromRenderer(content::WebContents* contents, + GURL gurl) { + std::unique_ptr<content::NavigationSimulator> simulator( + content::NavigationSimulator::CreateRendererInitiated( + gurl, contents->GetMainFrame())); + simulator->CommitSameDocument(); + auto* pmth = PerformanceManagerTabHelper::FromWebContents(contents); + CHECK(pmth); + EXPECT_NE(pmth->LastNavigationId(), pmth->LastNewDocNavigationId()); + return pmth->LastNavigationId(); +} + } // namespace TEST_F(TabLoadingFrameNavigationPolicyTest, OnlyHttpContentsThrottled) { @@ -276,7 +329,7 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { // Navigate and throttle a first contents. It will expire at time T. std::unique_ptr<content::WebContents> contents1 = CreateTestWebContents(); - content::NavigationSimulator::NavigateAndCommitFromBrowser( + int64_t navigation_id = NavigateAndCommitFromBrowser( contents1.get(), GURL("http://www.foo1.com/")); EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents1.get())); ExpectThrottledPageCount(1); @@ -293,13 +346,12 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { // Navigate and throttle a second contents. It will expire at 1.5 T. std::unique_ptr<content::WebContents> contents2 = CreateTestWebContents(); - content::NavigationSimulator::NavigateAndCommitFromBrowser( - contents2.get(), GURL("https://www.foo2.com/")); + NavigateAndCommitFromBrowser(contents2.get(), GURL("https://www.foo2.com/")); EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents2.get())); ExpectThrottledPageCount(2); // Run until the first contents times out. - EXPECT_CALL(*this, OnStopThrottling(contents1.get())); + EXPECT_CALL(*this, OnStopThrottling(contents1.get(), navigation_id)); RunUntilStopThrottling(); ExpectThrottledPageCount(1); base::TimeTicks stop1 = task_environment()->GetMockTickClock()->NowTicks(); @@ -310,8 +362,8 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { // Navigate and throttle a third contents. It will expire at time 2 T. std::unique_ptr<content::WebContents> contents3 = CreateTestWebContents(); - content::NavigationSimulator::NavigateAndCommitFromBrowser( - contents3.get(), GURL("https://www.foo3.com/")); + navigation_id = NavigateAndCommitFromBrowser(contents3.get(), + GURL("https://www.foo3.com/")); EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents3.get())); ExpectThrottledPageCount(2); @@ -325,6 +377,12 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { // We are now at time 1.25 T. ASSERT_EQ(1.25, GetRelativeTime()); + // Do a same document navigation. This will cause another navigation commit, + // but the policy should remain bound to the previous navigation id. + int64_t same_doc_navigation_id = NavigateAndCommitSameDocFromRenderer( + contents3.get(), GURL("https://www.foo3.com/#somehash")); + ASSERT_NE(navigation_id, same_doc_navigation_id); + // Close the 2nd contents before the timeout expires, and expect the throttled // count to drop. Now the timer should be running for the 3rd contents. contents2.reset(); @@ -342,7 +400,7 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { ASSERT_EQ(1.6, GetRelativeTime()); // Finally, run until the third contents times out. - EXPECT_CALL(*this, OnStopThrottling(contents3.get())); + EXPECT_CALL(*this, OnStopThrottling(contents3.get(), navigation_id)); RunUntilStopThrottling(); ExpectThrottledPageCount(0); base::TimeTicks stop3 = task_environment()->GetMockTickClock()->NowTicks(); @@ -352,24 +410,117 @@ TEST_F(TabLoadingFrameNavigationPolicyTest, TimeoutWorks) { ASSERT_EQ(2.0, GetRelativeTime()); } +TEST_F(TabLoadingFrameNavigationPolicyTest, MinTimeoutUpdateWorks) { + // Navigate and throttle a contents. + std::unique_ptr<content::WebContents> contents1 = CreateTestWebContents(); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + contents1.get(), GURL("http://www.foo1.com/")); + EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents1.get())); + ExpectThrottledPageCount(1); + + // Simulate an FCP that would cause a minimum timeout to be applied. + base::TimeDelta fcp = policy()->GetMinTimeoutForTesting() / + policy()->GetFCPMultipleForTesting() - + base::TimeDelta::FromMilliseconds(100); + ASSERT_LT(base::TimeDelta(), fcp); + + // Advance time by that amount and simulate FCP. No callbacks should fire. + task_environment()->FastForwardBy(fcp); + base::TimeTicks now = task_environment()->GetMockTickClock()->NowTicks(); + base::TimeTicks timeout = SimulateFirstContentfulPaint(contents1.get(), fcp); + testing::Mock::VerifyAndClearExpectations(this); + + // The timeout should be set at the minimum timeout. + EXPECT_EQ(now + policy()->GetMinTimeoutForTesting(), timeout); +} + +TEST_F(TabLoadingFrameNavigationPolicyTest, FCPTimeoutUpdateWorks) { + // Navigate and throttle a contents. + std::unique_ptr<content::WebContents> contents1 = CreateTestWebContents(); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + contents1.get(), GURL("http://www.foo1.com/")); + EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents1.get())); + ExpectThrottledPageCount(1); + + // Simulate an FCP that will cause a timeout update. + base::TimeDelta fcp = policy()->GetMinTimeoutForTesting() + + base::TimeDelta::FromMilliseconds(100); + + // Advance time by that amount and simulate FCP. No callbacks should fire. + task_environment()->FastForwardBy(fcp); + base::TimeTicks timeout = SimulateFirstContentfulPaint(contents1.get(), fcp); + testing::Mock::VerifyAndClearExpectations(this); + + // The timeout should be set at the expected calculated timeout (to ms + // precision). + EXPECT_EQ((fcp * policy()->GetFCPMultipleForTesting()).InMilliseconds(), + (timeout - start()).InMilliseconds()); +} + +TEST_F(TabLoadingFrameNavigationPolicyTest, MaxTimeoutWorks) { + // Navigate and throttle a contents. + std::unique_ptr<content::WebContents> contents1 = CreateTestWebContents(); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + contents1.get(), GURL("http://www.foo1.com/")); + EXPECT_TRUE(policy()->ShouldThrottleWebContents(contents1.get())); + ExpectThrottledPageCount(1); + + // Calculate an FCP that would cause a calculated timeout that exceeds the + // maximum. + base::TimeDelta fcp = policy()->GetMaxTimeoutForTesting() / + policy()->GetFCPMultipleForTesting() + + base::TimeDelta::FromMilliseconds(100); + + // Advance time by that amount and simulate FCP. No callbacks should fire. + task_environment()->FastForwardBy(fcp); + base::TimeTicks timeout = SimulateFirstContentfulPaint(contents1.get(), fcp); + testing::Mock::VerifyAndClearExpectations(this); + + // The timeout should remain at the max timeout it was initially set to. + EXPECT_EQ(start() + policy()->GetMaxTimeoutForTesting(), timeout); +} + TEST(TabLoadingFrameNavigationThrottlesParams, FeatureParamsWork) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitAndEnableFeatureWithParameters( features::kTabLoadingFrameNavigationThrottles, {{"MinimumThrottleTimeoutMilliseconds", "2500"}, - {"MaximumThrottleTimeoutMilliseconds", "25000"}}); + {"MaximumThrottleTimeoutMilliseconds", "25000"}, + {"FCPMultiple", "3.14"}}); // Make sure the parsing works. auto params = features::TabLoadingFrameNavigationThrottlesParams::GetParams(); EXPECT_EQ(base::TimeDelta::FromMilliseconds(2500), params.minimum_throttle_timeout); EXPECT_EQ(base::TimeDelta::FromSeconds(25), params.maximum_throttle_timeout); + EXPECT_EQ(3.14, params.fcp_multiple); // And make sure the plumbing works. std::unique_ptr<TabLoadingFrameNavigationPolicy> policy = std::make_unique<TabLoadingFrameNavigationPolicy>(); EXPECT_EQ(params.minimum_throttle_timeout, policy->GetMinTimeoutForTesting()); EXPECT_EQ(params.maximum_throttle_timeout, policy->GetMaxTimeoutForTesting()); + EXPECT_EQ(params.fcp_multiple, policy->GetFCPMultipleForTesting()); + + // Finally make sure that the calculation is as expected. + + // An FCP of 300ms would yield 942ms, or 642ms of additional timeout. This is + // less than timeout_min_, so we should get that. + EXPECT_EQ(params.minimum_throttle_timeout, + policy->CalculateTimeoutFromFCPForTesting( + base::TimeDelta::FromMilliseconds(300))); + + // An FCP of 1000ms would yield 3140ms, or 2140ms of additional timeout. This + // is also less than timeout_min_, so we should get that. + EXPECT_EQ(params.minimum_throttle_timeout, + policy->CalculateTimeoutFromFCPForTesting( + base::TimeDelta::FromMilliseconds(1000))); + + // An FCP of 2000ms would yield 6280ms, or 4280ms of additional timeout. We + // should get that. + EXPECT_EQ(base::TimeDelta::FromMilliseconds(4280), + policy->CalculateTimeoutFromFCPForTesting( + base::TimeDelta::FromMilliseconds(2000))); } } // namespace policies |