[PM] Add a mechanism to emit freezing votes from the UI thread.
Bug: 1144025
Change-Id: I7371085cfca50eb94c55e637597fd8acdf3dae25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593767
Commit-Queue: François Doray <[email protected]>
Reviewed-by: François Doray <[email protected]>
Cr-Commit-Position: refs/heads/master@{#838088}
diff --git a/chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc b/chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
index db72792d..769c765b 100644
--- a/chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
+++ b/chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
@@ -48,7 +48,9 @@
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#if !defined(OS_ANDROID)
+#include "chrome/browser/performance_manager/mechanisms/page_freezer.h"
#include "chrome/browser/performance_manager/policies/page_discarding_helper.h"
+#include "chrome/browser/performance_manager/policies/page_freezing_policy.h"
#include "chrome/browser/performance_manager/policies/urgent_page_discarding_policy.h"
#include "chrome/browser/tab_contents/form_interaction_tab_helper.h"
#endif // !defined(OS_ANDROID)
@@ -148,6 +150,12 @@
graph->PassToGraph(std::make_unique<
performance_manager::policies::HighPMFDiscardPolicy>());
}
+
+ // The freezing policy isn't enabled on Android yet as it doesn't play well
+ // with the freezing logic already in place in renderers. This logic should be
+ // moved to PerformanceManager, this is tracked in https://crbug.com/1156803.
+ graph->PassToGraph(
+ std::make_unique<performance_manager::policies::PageFreezingPolicy>());
#endif // !defined(OS_ANDROID)
graph->PassToGraph(
diff --git a/chrome/browser/performance_manager/policies/page_freezing_policy.cc b/chrome/browser/performance_manager/policies/page_freezing_policy.cc
index 151ca103..a624a3a4 100644
--- a/chrome/browser/performance_manager/policies/page_freezing_policy.cc
+++ b/chrome/browser/performance_manager/policies/page_freezing_policy.cc
@@ -220,7 +220,7 @@
->GetRegisteredObjectAs<freezing::FreezingVoteAggregator>()
->GetVotingChannel());
} else {
- DCHECK(!iter->second->IsValid());
+ DCHECK(!iter->second->HasVoteForContext(page_node));
}
// Submit the negative freezing vote.
iter->second->SubmitVote(page_node,
diff --git a/chrome/browser/resources/discards/discards_tab.js b/chrome/browser/resources/discards/discards_tab.js
index 0f53157b..61b45fc 100644
--- a/chrome/browser/resources/discards/discards_tab.js
+++ b/chrome/browser/resources/discards/discards_tab.js
@@ -220,6 +220,8 @@
return pageLifecycleStateFromVisibilityAndFocus();
case LifecycleUnitState.THROTTLED:
return pageLifecycleStateFromVisibilityAndFocus() + ' (throttled)';
+ case LifecycleUnitState.FROZEN:
+ return 'frozen';
case LifecycleUnitState.DISCARDED:
return 'discarded (' + this.discardReasonToString_(reason) + ')' +
((reason === LifecycleUnitDiscardReason.URGENT) ? ' at ' +
diff --git a/components/performance_manager/BUILD.gn b/components/performance_manager/BUILD.gn
index 5f3e977..3727500 100644
--- a/components/performance_manager/BUILD.gn
+++ b/components/performance_manager/BUILD.gn
@@ -46,6 +46,7 @@
"execution_context_priority/root_vote_observer.cc",
"execution_context_priority/root_vote_observer.h",
"features.cc",
+ "freezing/freezing.cc",
"freezing/freezing_vote_aggregator.cc",
"freezing/freezing_vote_aggregator.h",
"graph/frame_node.cc",
@@ -295,6 +296,7 @@
if (!is_android) {
sources += [
"decorators/site_data_recorder_unittest.cc",
+ "freezing/freezing_unittest.cc",
"persistence/site_data/exponential_moving_average_unittest.cc",
"persistence/site_data/leveldb_site_data_store_unittest.cc",
"persistence/site_data/non_recording_site_data_cache_unittest.cc",
diff --git a/components/performance_manager/embedder/graph_features_helper.h b/components/performance_manager/embedder/graph_features_helper.h
index 679aab3..e20ca22 100644
--- a/components/performance_manager/embedder/graph_features_helper.h
+++ b/components/performance_manager/embedder/graph_features_helper.h
@@ -28,6 +28,7 @@
bool execution_context_registry : 1;
bool frame_node_impl_describer : 1;
bool frame_visibility_decorator : 1;
+ bool freezing_vote_decorator : 1;
bool page_live_state_decorator : 1;
bool page_load_tracker_decorator : 1;
bool page_node_impl_describer : 1;
@@ -64,6 +65,11 @@
return *this;
}
+ constexpr GraphFeaturesHelper& EnableFreezingVoteDecorator() {
+ flags_.freezing_vote_decorator = true;
+ return *this;
+ }
+
constexpr GraphFeaturesHelper& EnablePageLiveStateDecorator() {
flags_.page_live_state_decorator = true;
return *this;
@@ -120,6 +126,7 @@
EnableExecutionContextRegistry();
EnableFrameNodeImplDescriber();
EnableFrameVisibilityDecorator();
+ EnableFreezingVoteDecorator();
EnablePageLiveStateDecorator();
EnablePageLoadTrackerDecorator();
EnablePageNodeImplDescriber();
diff --git a/components/performance_manager/freezing/freezing.cc b/components/performance_manager/freezing/freezing.cc
new file mode 100644
index 0000000..90bb84f7
--- /dev/null
+++ b/components/performance_manager/freezing/freezing.cc
@@ -0,0 +1,141 @@
+// Copyright 2020 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 "components/performance_manager/public/freezing/freezing.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/sequence_checker.h"
+#include "base/sequenced_task_runner.h"
+#include "base/task/post_task.h"
+#include "components/performance_manager/freezing/freezing_vote_aggregator.h"
+#include "components/performance_manager/performance_manager_impl.h"
+#include "components/performance_manager/public/graph/page_node.h"
+#include "components/performance_manager/public/performance_manager.h"
+#include "content/public/browser/web_contents.h"
+
+namespace performance_manager {
+
+namespace freezing {
+
+namespace {
+
+// The counterpart of a FreezingVoteToken that lives on the PM sequence.
+class FreezingVoteTokenPMImpl : public PageNode::ObserverDefaultImpl {
+ public:
+ FreezingVoteTokenPMImpl(content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason);
+ ~FreezingVoteTokenPMImpl() override;
+ FreezingVoteTokenPMImpl(const FreezingVoteTokenPMImpl& other) = delete;
+ FreezingVoteTokenPMImpl& operator=(const FreezingVoteTokenPMImpl&) = delete;
+
+ // PageNodeObserver:
+ void OnBeforePageNodeRemoved(const PageNode* page_node) override;
+
+ private:
+ const PageNode* page_node_ = nullptr;
+ Graph* graph_ = nullptr;
+
+ // Voting channel wrapper. This objects should only be used on the PM
+ // sequence.
+ std::unique_ptr<FreezingVotingChannelWrapper> voter_;
+
+ SEQUENCE_CHECKER(sequence_checker_);
+};
+
+// Concrete implementation of a FreezingVoteToken.
+class FreezingVoteTokenImpl : public FreezingVoteToken {
+ public:
+ FreezingVoteTokenImpl(content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason);
+ ~FreezingVoteTokenImpl() override;
+ FreezingVoteTokenImpl(const FreezingVoteTokenImpl& other) = delete;
+ FreezingVoteTokenImpl& operator=(const FreezingVoteTokenImpl&) = delete;
+
+ private:
+ // Voting channel wrapper. This objects should only be used on the PM
+ // sequence.
+ std::unique_ptr<FreezingVoteTokenPMImpl, base::OnTaskRunnerDeleter> pm_impl_;
+};
+
+} // namespace
+
+FreezingVoteToken::FreezingVoteToken() = default;
+FreezingVoteToken::~FreezingVoteToken() = default;
+
+FreezingVoteTokenPMImpl::FreezingVoteTokenPMImpl(content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason) {
+ DETACH_FROM_SEQUENCE(sequence_checker_);
+ // Register the vote on the PM sequence.
+ PerformanceManager::CallOnGraph(
+ FROM_HERE,
+ base::BindOnce(
+ [](base::WeakPtr<PageNode> page_node, FreezingVoteValue vote_value,
+ const char* vote_reason, FreezingVoteTokenPMImpl* voter_pm_impl,
+ Graph* graph) {
+ voter_pm_impl->voter_ =
+ std::make_unique<FreezingVotingChannelWrapper>();
+ voter_pm_impl->graph_ = graph;
+ graph->AddPageNodeObserver(voter_pm_impl);
+ voter_pm_impl->voter_->SetVotingChannel(
+ graph->GetRegisteredObjectAs<FreezingVoteAggregator>()
+ ->GetVotingChannel());
+ if (page_node) {
+ voter_pm_impl->voter_->SubmitVote(page_node.get(),
+ {vote_value, vote_reason});
+ voter_pm_impl->page_node_ = page_node.get();
+ }
+ },
+ PerformanceManager::GetPageNodeForWebContents(content), vote_value,
+ // It's safe to use Unretained because |vote_reason| is a static
+ // string.
+ base::Unretained(vote_reason),
+ // It's safe to use Unretained because |this| can only be deleted
+ // from a task running on the PM sequence after this callback.
+ base::Unretained(this)));
+}
+
+FreezingVoteTokenPMImpl::~FreezingVoteTokenPMImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (graph_)
+ graph_->RemovePageNodeObserver(this);
+}
+
+void FreezingVoteTokenPMImpl::OnBeforePageNodeRemoved(
+ const PageNode* page_node) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (page_node == page_node_) {
+ // Invalidate the vote if its associated page node is destroyed. This can
+ // happen if a freezing vote token is released after the destruction of the
+ // WebContents it's associated with.
+ voter_->InvalidateVote(page_node);
+ page_node_ = nullptr;
+ graph_->RemovePageNodeObserver(this);
+ graph_ = nullptr;
+ }
+}
+
+FreezingVoteTokenImpl::FreezingVoteTokenImpl(content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason)
+ : pm_impl_(new FreezingVoteTokenPMImpl(content, vote_value, vote_reason),
+ base::OnTaskRunnerDeleter(PerformanceManager::GetTaskRunner())) {
+}
+
+FreezingVoteTokenImpl::~FreezingVoteTokenImpl() = default;
+
+std::unique_ptr<FreezingVoteToken> EmitFreezingVoteForWebContents(
+ content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason) {
+ return std::make_unique<FreezingVoteTokenImpl>(content, vote_value,
+ vote_reason);
+}
+
+} // namespace freezing
+} // namespace performance_manager
\ No newline at end of file
diff --git a/components/performance_manager/freezing/freezing_unittest.cc b/components/performance_manager/freezing/freezing_unittest.cc
new file mode 100644
index 0000000..4d4e8fd8
--- /dev/null
+++ b/components/performance_manager/freezing/freezing_unittest.cc
@@ -0,0 +1,103 @@
+// Copyright 2020 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 "components/performance_manager/public/freezing/freezing.h"
+
+#include "components/performance_manager/public/graph/page_node.h"
+#include "components/performance_manager/public/performance_manager.h"
+#include "components/performance_manager/test_support/performance_manager_test_harness.h"
+#include "components/performance_manager/test_support/test_harness_helper.h"
+#include "content/public/test/test_renderer_host.h"
+#include "content/public/test/web_contents_tester.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace performance_manager {
+namespace freezing {
+
+namespace {
+
+constexpr char kCanFreeze[] = "Can freeze";
+constexpr char kCannotFreeze[] = "Cannot freeze";
+
+// Check that the freezing vote attached to the page node associated with
+// |content| has the expected value.
+void ExpectFreezingVote(content::WebContents* content,
+ base::Optional<FreezingVote> expected_vote) {
+ base::RunLoop run_loop;
+ auto quit_closure = run_loop.QuitClosure();
+ PerformanceManager::CallOnGraph(
+ FROM_HERE,
+ base::BindOnce(
+ [](base::WeakPtr<PageNode> page_node, base::OnceClosure quit_closure,
+ base::Optional<FreezingVote> expected_vote) {
+ EXPECT_TRUE(page_node);
+ auto vote = page_node->GetFreezingVote();
+ EXPECT_EQ(expected_vote, vote);
+ std::move(quit_closure).Run();
+ },
+ PerformanceManager::GetPageNodeForWebContents(content),
+ std::move(quit_closure), expected_vote));
+ run_loop.Run();
+}
+
+} // namespace
+
+class FreezingTest : public PerformanceManagerTestHarness {
+ public:
+ FreezingTest() = default;
+ ~FreezingTest() override = default;
+ FreezingTest(const FreezingTest& other) = delete;
+ FreezingTest& operator=(const FreezingTest&) = delete;
+
+ void SetUp() override {
+ GetGraphFeaturesHelper().EnableFreezingVoteDecorator();
+ PerformanceManagerTestHarness::SetUp();
+ SetContents(CreateTestWebContents());
+ }
+};
+
+TEST_F(FreezingTest, FreezingToken) {
+ content::WebContentsTester* web_contents_tester =
+ content::WebContentsTester::For(web_contents());
+ EXPECT_TRUE(web_contents_tester);
+ web_contents_tester->NavigateAndCommit(GURL("https:/foo.com"));
+
+ {
+ // Emit a positive freezing vote, this should make the page node freezable.
+ auto token = EmitFreezingVoteForWebContents(
+ web_contents(), FreezingVoteValue::kCanFreeze, kCanFreeze);
+ ExpectFreezingVote(web_contents(),
+ FreezingVote(FreezingVoteValue::kCanFreeze, kCanFreeze));
+ }
+ // Once the freezing vote token is destroyed the vote should be invalidated.
+ ExpectFreezingVote(web_contents(), base::nullopt);
+
+ // Same test but for a negative freezing vote.
+ {
+ auto token = EmitFreezingVoteForWebContents(
+ web_contents(), FreezingVoteValue::kCannotFreeze, kCannotFreeze);
+ ExpectFreezingVote(
+ web_contents(),
+ FreezingVote(FreezingVoteValue::kCannotFreeze, kCannotFreeze));
+ }
+ ExpectFreezingVote(web_contents(), base::nullopt);
+}
+
+TEST_F(FreezingTest, WebContentsDestroyedBeforeToken) {
+ content::WebContentsTester* web_contents_tester =
+ content::WebContentsTester::For(web_contents());
+ EXPECT_TRUE(web_contents_tester);
+ web_contents_tester->NavigateAndCommit(GURL("https:/foo.com"));
+
+ // Emit a positive freezing vote, this should make the page node freezable.
+ auto token = EmitFreezingVoteForWebContents(
+ web_contents(), FreezingVoteValue::kCanFreeze, kCanFreeze);
+ ExpectFreezingVote(web_contents(),
+ FreezingVote(FreezingVoteValue::kCanFreeze, kCanFreeze));
+ DeleteContents();
+ base::RunLoop().RunUntilIdle();
+}
+
+} // namespace freezing
+} // namespace performance_manager
\ No newline at end of file
diff --git a/components/performance_manager/graph_features_helper.cc b/components/performance_manager/graph_features_helper.cc
index d0dd4f32..7a56dd2 100644
--- a/components/performance_manager/graph_features_helper.cc
+++ b/components/performance_manager/graph_features_helper.cc
@@ -8,6 +8,7 @@
#include "build/build_config.h"
#include "components/performance_manager/decorators/frame_visibility_decorator.h"
+#include "components/performance_manager/decorators/freezing_vote_decorator.h"
#include "components/performance_manager/decorators/page_load_tracker_decorator.h"
#include "components/performance_manager/execution_context/execution_context_registry_impl.h"
#include "components/performance_manager/execution_context_priority/execution_context_priority_decorator.h"
@@ -42,6 +43,8 @@
Install<FrameNodeImplDescriber>(graph);
if (flags_.frame_visibility_decorator)
Install<FrameVisibilityDecorator>(graph);
+ if (flags_.freezing_vote_decorator)
+ Install<FreezingVoteDecorator>(graph);
if (flags_.page_live_state_decorator)
Install<PageLiveStateDecorator>(graph);
if (flags_.page_load_tracker_decorator)
diff --git a/components/performance_manager/graph_features_helper_unittest.cc b/components/performance_manager/graph_features_helper_unittest.cc
index 5c5e5a3..cf0d6b1 100644
--- a/components/performance_manager/graph_features_helper_unittest.cc
+++ b/components/performance_manager/graph_features_helper_unittest.cc
@@ -50,7 +50,7 @@
execution_context::ExecutionContextRegistry::GetFromGraph(&graph));
EXPECT_FALSE(v8_memory::V8ContextTracker::GetFromGraph(&graph));
- size_t graph_owned_count = 10;
+ size_t graph_owned_count = 11;
#if !defined(OS_ANDROID)
// The SiteDataRecorder is not available on Android.
graph_owned_count++;
@@ -60,7 +60,7 @@
features.EnableDefault();
features.ConfigureGraph(&graph);
EXPECT_EQ(graph_owned_count, graph.GraphOwnedCountForTesting());
- EXPECT_EQ(2u, graph.GraphRegisteredCountForTesting());
+ EXPECT_EQ(3u, graph.GraphRegisteredCountForTesting());
EXPECT_EQ(8u, graph.NodeDataDescriberCountForTesting());
// Ensure the GraphRegistered objects can be queried directly.
EXPECT_TRUE(
diff --git a/components/performance_manager/public/freezing/freezing.h b/components/performance_manager/public/freezing/freezing.h
index ff6980bf..10eaade 100644
--- a/components/performance_manager/public/freezing/freezing.h
+++ b/components/performance_manager/public/freezing/freezing.h
@@ -10,6 +10,10 @@
#include "components/performance_manager/public/voting/voting.h"
+namespace content {
+class WebContents;
+}
+
namespace performance_manager {
class PageNode;
@@ -30,6 +34,28 @@
voting::VoteConsumerDefaultImpl<FreezingVote>;
using FreezingVotingChannelWrapper = voting::VotingChannelWrapper<FreezingVote>;
+// A freezing vote token, instances of this are meant to be retrieved by calling
+// |EmitFreezingVoteForWebContents|.
+class FreezingVoteToken {
+ public:
+ FreezingVoteToken(const FreezingVoteToken& other) = delete;
+ FreezingVoteToken& operator=(const FreezingVoteToken&) = delete;
+ virtual ~FreezingVoteToken() = 0;
+
+ protected:
+ FreezingVoteToken();
+};
+
+// Allows emiting a freezing vote for a WebContents. The vote's lifetime will
+// follow the lifetime of this object, as soon as it's released the vote will be
+// invalidated.
+//
+// NOTE: |vote_reason| *must* be a static string.
+std::unique_ptr<FreezingVoteToken> EmitFreezingVoteForWebContents(
+ content::WebContents* content,
+ FreezingVoteValue vote_value,
+ const char* vote_reason);
+
} // namespace freezing
} // namespace performance_manager
diff --git a/components/performance_manager/public/voting/voting.h b/components/performance_manager/public/voting/voting.h
index 7e47e5c..071ba2af 100644
--- a/components/performance_manager/public/voting/voting.h
+++ b/components/performance_manager/public/voting/voting.h
@@ -486,6 +486,9 @@
// Returns true if the underlying VotingChannel is valid.
bool IsValid() const;
+ // Checks whether or not there's a vote associated with |context|.
+ bool HasVoteForContext(const ContextType* context);
+
VoterId<VoteImpl> voter_id() const { return voting_channel_.voter_id(); }
private:
@@ -1034,6 +1037,12 @@
return voting_channel_.IsValid();
}
+template <class VoteImpl>
+bool VotingChannelWrapper<VoteImpl>::HasVoteForContext(
+ const ContextType* context) {
+ return base::Contains(vote_receipts_, context);
+}
+
} // namespace voting
} // namespace performance_manager