diff --git a/chrome/browser/performance_manager/metrics/page_timeline_monitor.cc b/chrome/browser/performance_manager/metrics/page_timeline_monitor.cc index 641d5e524f9dc..94aabb820d7cb 100644 --- a/chrome/browser/performance_manager/metrics/page_timeline_monitor.cc +++ b/chrome/browser/performance_manager/metrics/page_timeline_monitor.cc @@ -114,6 +114,7 @@ void PageTimelineMonitor::CollectSlice() { bool has_notification_permission = false; bool is_capturing_media = false; bool is_connected_to_device = false; + bool updated_title_or_favicon_in_background = false; const auto* page_live_state_data = PageLiveStateDecorator::Data::FromPageNode(page_node); @@ -130,6 +131,8 @@ void PageTimelineMonitor::CollectSlice() { is_connected_to_device = page_live_state_data->IsConnectedToUSBDevice() || page_live_state_data->IsConnectedToBluetoothDevice(); + updated_title_or_favicon_in_background = + page_live_state_data->UpdatedTitleOrFaviconInBackground(); } ukm::builders::PerformanceManager_PageTimelineState builder(source_id); @@ -147,7 +150,7 @@ void PageTimelineMonitor::CollectSlice() { .SetTotalForegroundTime(ukm::GetSemanticBucketMinForDurationTiming( curr_info->total_foreground_milliseconds)) .SetChangedFaviconOrTitleInBackground( - curr_info->updated_title_or_favicon_in_background) + updated_title_or_favicon_in_background) .SetHasNotificationPermission(has_notification_permission) .SetIsCapturingMedia(is_capturing_media) .SetIsConnectedToDevice(is_connected_to_device) @@ -287,29 +290,6 @@ void PageTimelineMonitor::OnTypeChanged(const PageNode* page_node, } } -void PageTimelineMonitor::OnTitleUpdated(const PageNode* page_node) { - if (page_node->GetType() != performance_manager::PageType::kTab) - return; - - DCHECK(base::Contains(page_node_info_map_, page_node)); - if (page_node_info_map_[page_node]->GetPageState() == - PageState::kBackground) { - page_node_info_map_[page_node]->updated_title_or_favicon_in_background = - true; - } -} -void PageTimelineMonitor::OnFaviconUpdated(const PageNode* page_node) { - if (page_node->GetType() != performance_manager::PageType::kTab) - return; - - DCHECK(base::Contains(page_node_info_map_, page_node)); - if (page_node_info_map_[page_node]->GetPageState() == - PageState::kBackground) { - page_node_info_map_[page_node]->updated_title_or_favicon_in_background = - true; - } -} - void PageTimelineMonitor::OnAboutToBeDiscarded(const PageNode* page_node, const PageNode* new_page_node) { auto old_it = page_node_info_map_.find(page_node); diff --git a/chrome/browser/performance_manager/metrics/page_timeline_monitor.h b/chrome/browser/performance_manager/metrics/page_timeline_monitor.h index 86212c5039d0e..e74b2310baf71 100644 --- a/chrome/browser/performance_manager/metrics/page_timeline_monitor.h +++ b/chrome/browser/performance_manager/metrics/page_timeline_monitor.h @@ -51,8 +51,6 @@ class PageTimelineMonitor : public PageNode::ObserverDefaultImpl, void OnPageLifecycleStateChanged(const PageNode* page_node) override; void OnTypeChanged(const PageNode* page_node, PageType previous_state) override; - void OnTitleUpdated(const PageNode* page_node) override; - void OnFaviconUpdated(const PageNode* page_node) override; void OnAboutToBeDiscarded(const PageNode* page_node, const PageNode* new_page_node) override; @@ -78,7 +76,6 @@ class PageTimelineMonitor : public PageNode::ObserverDefaultImpl, bool currently_visible; PageNode::LifecycleState current_lifecycle; base::TimeTicks time_of_most_recent_state_change; - bool updated_title_or_favicon_in_background{false}; base::TimeTicks time_of_last_foreground_millisecond_update; int total_foreground_milliseconds{0}; int tab_id; diff --git a/chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc b/chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc index 4c3588bc53d2d..2d90bdc0250ed 100644 --- a/chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc +++ b/chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc @@ -163,7 +163,7 @@ TEST_F(PageTimelineMonitorUnitTest, TestOnlyRecordTabs) { EXPECT_EQ(entries2.size(), 0UL); } -TEST_F(PageTimelineMonitorUnitTest, TestUpdateFaviconInBackground) { +TEST_F(PageTimelineMonitorUnitTest, TestUpdateTitleOrFaviconInBackground) { MockSinglePageInSingleProcessGraph mock_graph(graph()); ukm::SourceId mock_source_id = ukm::NoURLSourceId(); mock_graph.page->SetType(performance_manager::PageType::kTab); @@ -171,30 +171,22 @@ TEST_F(PageTimelineMonitorUnitTest, TestUpdateFaviconInBackground) { mock_graph.page->SetIsVisible(false); mock_graph.page->SetLifecycleStateForTesting(mojom::LifecycleState::kRunning); - monitor()->OnIsVisibleChanged(mock_graph.page.get()); - monitor()->OnPageLifecycleStateChanged(mock_graph.page.get()); - monitor()->OnFaviconUpdated(mock_graph.page.get()); - - EXPECT_TRUE(monitor() - ->page_node_info_map_[mock_graph.page.get()] - ->updated_title_or_favicon_in_background); -} - -TEST_F(PageTimelineMonitorUnitTest, TestUpdateTitleInBackground) { - MockSinglePageInSingleProcessGraph mock_graph(graph()); - ukm::SourceId mock_source_id = ukm::NoURLSourceId(); - mock_graph.page->SetType(performance_manager::PageType::kTab); - mock_graph.page->SetUkmSourceId(mock_source_id); - mock_graph.page->SetIsVisible(false); - mock_graph.page->SetLifecycleStateForTesting(mojom::LifecycleState::kRunning); + // Collect one slice before updating, one after. + TriggerCollectSlice(); - monitor()->OnIsVisibleChanged(mock_graph.page.get()); - monitor()->OnPageLifecycleStateChanged(mock_graph.page.get()); - monitor()->OnTitleUpdated(mock_graph.page.get()); + PageLiveStateDecorator::Data* data = + PageLiveStateDecorator::Data::GetOrCreateForPageNode( + mock_graph.page.get()); + data->SetUpdatedTitleOrFaviconInBackgroundForTesting(true); - EXPECT_TRUE(monitor() - ->page_node_info_map_[mock_graph.page.get()] - ->updated_title_or_favicon_in_background); + TriggerCollectSlice(); + auto entries = test_ukm_recorder()->GetEntriesByName( + ukm::builders::PerformanceManager_PageTimelineState::kEntryName); + EXPECT_EQ(entries.size(), 2UL); + test_ukm_recorder()->ExpectEntryMetric( + entries[0], "ChangedFaviconOrTitleInBackground", false); + test_ukm_recorder()->ExpectEntryMetric( + entries[1], "ChangedFaviconOrTitleInBackground", true); } TEST_F(PageTimelineMonitorUnitTest, TestUpdateLifecycleState) { diff --git a/chrome/browser/performance_manager/policies/page_discarding_helper.cc b/chrome/browser/performance_manager/policies/page_discarding_helper.cc index 7066a0ba8c2a2..31ce278238bfe 100644 --- a/chrome/browser/performance_manager/policies/page_discarding_helper.cc +++ b/chrome/browser/performance_manager/policies/page_discarding_helper.cc @@ -5,12 +5,14 @@ #include "chrome/browser/performance_manager/policies/page_discarding_helper.h" #include +#include #include "base/containers/flat_map.h" #include "base/containers/flat_set.h" #include "base/feature_list.h" #include "base/metrics/histogram_macros.h" #include "base/sequence_checker.h" +#include "base/strings/string_piece.h" #include "base/time/time.h" #include "build/build_config.h" #include "build/chromeos_buildflags.h" @@ -22,6 +24,7 @@ #include "components/performance_manager/public/graph/frame_node.h" #include "components/performance_manager/public/graph/graph_operations.h" #include "components/performance_manager/public/graph/node_data_describer_registry.h" +#include "components/performance_manager/public/graph/node_data_describer_util.h" #include "components/performance_manager/public/graph/page_node.h" #include "components/performance_manager/public/graph/process_node.h" #include "components/url_matcher/url_matcher.h" @@ -211,13 +214,16 @@ void PageDiscardingHelper::DiscardMultiplePages( void PageDiscardingHelper::ImmediatelyDiscardSpecificPage( const PageNode* page_node, - ::mojom::LifecycleUnitDiscardReason discard_reason) { + ::mojom::LifecycleUnitDiscardReason discard_reason, + base::OnceCallback post_discard_cb) { // Pass 0 TimeDelta to bypass the minimum time in background check. if (CanUrgentlyDiscard(page_node, /* minimum_time_in_background */ base::TimeDelta()) == CanUrgentlyDiscardResult::kEligible) { page_discarder_->DiscardPageNodes({page_node}, discard_reason, - base::DoNothing()); + std::move(post_discard_cb)); + } else { + std::move(post_discard_cb).Run(false); } } @@ -388,6 +394,9 @@ PageDiscardingHelper::CanUrgentlyDiscard( if (live_state_data->IsDevToolsOpen()) { return CanUrgentlyDiscardResult::kProtected; } + if (live_state_data->UpdatedTitleOrFaviconInBackground()) { + return CanUrgentlyDiscardResult::kProtected; + } #if !BUILDFLAG(IS_CHROMEOS) // TODO(sebmarchand): Skip this check if the Entreprise memory limit is set. if (live_state_data->WasDiscarded()) { @@ -431,14 +440,30 @@ bool PageDiscardingHelper::IsPageOptedOutOfDiscarding( base::Value::Dict PageDiscardingHelper::DescribePageNodeData( const PageNode* node) const { - auto* data = DiscardAttemptMarker::Get(PageNodeImpl::FromNode(node)); - if (data == nullptr) { - return base::Value::Dict(); + base::StringPiece can_discard; + switch (CanUrgentlyDiscard(node, base::TimeDelta())) { + case CanUrgentlyDiscardResult::kEligible: + can_discard = "eligible"; + break; + case CanUrgentlyDiscardResult::kProtected: + can_discard = "protected"; + break; + case CanUrgentlyDiscardResult::kMarked: + can_discard = "marked"; + break; } base::Value::Dict ret; - ret.Set("has_discard_attempt_marker", base::Value("true")); - + ret.Set("can_urgently_discard", can_discard); + auto it = last_change_to_non_audible_time_.find(node); + if (it != last_change_to_non_audible_time_.end()) { + ret.Set("non_audible_change_time", TimeDeltaFromNowToValue(it->second)); + } + auto* main_frame = node->GetMainFrameNode(); + if (main_frame) { + ret.Set("opted_out", IsPageOptedOutOfDiscarding(node->GetBrowserContextID(), + main_frame->GetURL())); + } return ret; } diff --git a/chrome/browser/performance_manager/policies/page_discarding_helper.h b/chrome/browser/performance_manager/policies/page_discarding_helper.h index 8438bdc4974e8..4c6b67a7c65c2 100644 --- a/chrome/browser/performance_manager/policies/page_discarding_helper.h +++ b/chrome/browser/performance_manager/policies/page_discarding_helper.h @@ -7,6 +7,7 @@ #include "base/containers/flat_map.h" #include "base/functional/callback_forward.h" +#include "base/functional/callback_helpers.h" #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" #include "base/time/time.h" @@ -124,7 +125,8 @@ class PageDiscardingHelper : public GraphOwned, void ImmediatelyDiscardSpecificPage( const PageNode* page_node, - ::mojom::LifecycleUnitDiscardReason discard_reason); + ::mojom::LifecycleUnitDiscardReason discard_reason, + base::OnceCallback post_discard_cb = base::DoNothing()); // PageNodeObserver: void OnBeforePageNodeRemoved(const PageNode* page_node) override; diff --git a/chrome/browser/performance_manager/policies/page_discarding_helper_browsertest.cc b/chrome/browser/performance_manager/policies/page_discarding_helper_browsertest.cc new file mode 100644 index 0000000000000..aae794dc51220 --- /dev/null +++ b/chrome/browser/performance_manager/policies/page_discarding_helper_browsertest.cc @@ -0,0 +1,171 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/performance_manager/policies/page_discarding_helper.h" + +#include "base/memory/weak_ptr.h" +#include "base/run_loop.h" +#include "base/strings/strcat.h" +#include "base/strings/utf_string_conversions.h" +#include "base/test/bind.h" +#include "chrome/browser/resource_coordinator/lifecycle_unit_state.mojom.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/performance_manager/public/graph/graph.h" +#include "components/performance_manager/public/graph/page_node.h" +#include "components/performance_manager/public/performance_manager.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_types.h" +#include "content/public/browser/page_navigator.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "content/public/common/referrer.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/browser_test_utils.h" +#include "content/public/test/test_utils.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "ui/base/page_transition_types.h" +#include "ui/base/window_open_disposition.h" +#include "url/gurl.h" + +namespace performance_manager::policies { + +namespace { + +class FaviconWatcher final : public content::WebContentsObserver { + public: + explicit FaviconWatcher(content::WebContents* web_contents) + : content::WebContentsObserver(web_contents) {} + ~FaviconWatcher() final = default; + + FaviconWatcher(const FaviconWatcher&) = delete; + FaviconWatcher& operator=(const FaviconWatcher&) = delete; + + void Wait() { run_loop_.Run(); } + + private: + // WebContentsObserver + void DidUpdateFaviconURL( + content::RenderFrameHost* render_frame_host, + const std::vector& candidates) final { + run_loop_.Quit(); + } + + base::RunLoop run_loop_; +}; + +class PageDiscardingHelperBrowserTest : public InProcessBrowserTest { + protected: + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + ASSERT_TRUE(embedded_test_server()->Start()); + } + + // Opens a new page in the background, and returns its index in the tab strip. + int OpenNewBackgroundPage() { + content::WindowedNotificationObserver load( + content::NOTIFICATION_NAV_ENTRY_COMMITTED, + content::NotificationService::AllSources()); + // Load a page with title and favicon so that some tests can manipulate + // them. + content::OpenURLParams page( + embedded_test_server()->GetURL("/favicon/title2_with_favicon.html"), + content::Referrer(), WindowOpenDisposition::NEW_BACKGROUND_TAB, + ui::PAGE_TRANSITION_TYPED, false); + content::WebContents* contents = browser()->OpenURL(page); + + // Wait for the page and the initial favicon to finish loading. + FaviconWatcher favicon_watcher(contents); + load.Wait(); + favicon_watcher.Wait(); + + return browser()->tab_strip_model()->GetIndexOfWebContents(contents); + } + + void UpdatePageTitle(int index) { + constexpr char16_t kNewTitle[] = u"New title"; + content::WebContents* contents = + browser()->tab_strip_model()->GetWebContentsAt(index); + content::TitleWatcher title_watcher(contents, kNewTitle); + ASSERT_TRUE(content::ExecJs( + contents, base::StrCat({"document.title = '", + base::UTF16ToASCII(kNewTitle), "'"}))); + EXPECT_EQ(title_watcher.WaitAndGetTitle(), kNewTitle); + } + + void UpdateFavicon(int index) { + content::WebContents* contents = + browser()->tab_strip_model()->GetWebContentsAt(index); + // Change the favicon link from "icon.png" to "icon.svg". + FaviconWatcher favicon_watcher(contents); + ASSERT_TRUE(content::ExecJs( + contents, + "document.getElementsByTagName('link')[0].href = 'icon.svg'")); + favicon_watcher.Wait(); + } + + void ExpectImmediateDiscard(int index, bool expected_result) { + base::WeakPtr page_node = + PerformanceManager::GetPrimaryPageNodeForWebContents( + browser()->tab_strip_model()->GetWebContentsAt(index)); + base::RunLoop run_loop; + PerformanceManager::CallOnGraph( + FROM_HERE, base::BindLambdaForTesting([&](Graph* graph) { + ASSERT_TRUE(page_node); + auto* helper = PageDiscardingHelper::GetFromGraph(graph); + ASSERT_TRUE(helper); + helper->ImmediatelyDiscardSpecificPage( + page_node.get(), ::mojom::LifecycleUnitDiscardReason::URGENT, + base::BindLambdaForTesting([&](bool success) { + EXPECT_EQ(success, expected_result); + run_loop.Quit(); + })); + })); + run_loop.Run(); + + EXPECT_EQ( + browser()->tab_strip_model()->GetWebContentsAt(index)->WasDiscarded(), + expected_result); + } +}; + +IN_PROC_BROWSER_TEST_F(PageDiscardingHelperBrowserTest, DiscardSpecificPage) { + // Background pages can be discarded. + const int index1 = OpenNewBackgroundPage(); + ExpectImmediateDiscard(index1, true); + + // Foreground page should be blocked. + const int index2 = OpenNewBackgroundPage(); + browser()->tab_strip_model()->ActivateTabAt(index2); + ExpectImmediateDiscard(index2, false); + + // Updating the title while in the background should block the discard. + const int index3 = OpenNewBackgroundPage(); + UpdatePageTitle(index3); + ExpectImmediateDiscard(index3, false); + + // Updating the page title while in the foreground should not. + const int index4 = OpenNewBackgroundPage(); + browser()->tab_strip_model()->ActivateTabAt(index4); + UpdatePageTitle(index4); + browser()->tab_strip_model()->ActivateTabAt(index3); + ExpectImmediateDiscard(index4, true); + + // Updating the favicon while in the background should block the discard. + const int index5 = OpenNewBackgroundPage(); + UpdateFavicon(index5); + ExpectImmediateDiscard(index5, false); + + // Updating the favicon while in the foreground should not. + const int index6 = OpenNewBackgroundPage(); + browser()->tab_strip_model()->ActivateTabAt(index6); + UpdateFavicon(index6); + browser()->tab_strip_model()->ActivateTabAt(index5); + ExpectImmediateDiscard(index6, true); +} + +} // namespace + +} // namespace performance_manager::policies diff --git a/chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc b/chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc index 31c9d6f631f88..5b6fc647e0a25 100644 --- a/chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc +++ b/chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc @@ -310,6 +310,15 @@ TEST_F(PageDiscardingHelperTest, TestCannotDiscardIsDevToolsOpen) { page_node())); } +TEST_F(PageDiscardingHelperTest, + TestCannotDiscardUpdatedTitleOrFaviconInBackground) { + PageLiveStateDecorator::Data::GetOrCreateForPageNode(page_node()) + ->SetUpdatedTitleOrFaviconInBackgroundForTesting(true); + EXPECT_FALSE( + PageDiscardingHelper::GetFromGraph(graph())->CanUrgentlyDiscardForTesting( + page_node())); +} + // Tests DiscardMultiplePages. TEST_F(PageDiscardingHelperTest, DiscardMultiplePagesNoCandidate) { diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index a9df65e0aa57c..651c05eebc034 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -2050,6 +2050,7 @@ if (!is_android) { "../browser/performance_manager/page_load_tracker_decorator_browsertest.cc", "../browser/performance_manager/page_node_browsertest.cc", "../browser/performance_manager/policies/bfcache_policy_browsertest.cc", + "../browser/performance_manager/policies/page_discarding_helper_browsertest.cc", "../browser/performance_timeline_browsertest.cc", "../browser/permissions/permission_delegation_browsertest.cc", "../browser/permissions/permission_manager_browsertest.cc", diff --git a/components/performance_manager/decorators/page_live_state_decorator.cc b/components/performance_manager/decorators/page_live_state_decorator.cc index 37b205690e7ca..b5ac3b9fe2570 100644 --- a/components/performance_manager/decorators/page_live_state_decorator.cc +++ b/components/performance_manager/decorators/page_live_state_decorator.cc @@ -88,6 +88,10 @@ class PageLiveStateDataImpl DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); return is_dev_tools_open_; } + bool UpdatedTitleOrFaviconInBackground() const override { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + return updated_title_or_favicon_in_background_; + } void SetIsConnectedToUSBDeviceForTesting(bool value) override { set_is_connected_to_usb_device(value); @@ -129,6 +133,9 @@ class PageLiveStateDataImpl void SetIsDevToolsOpenForTesting(bool value) override { set_is_dev_tools_open(value); } + void SetUpdatedTitleOrFaviconInBackgroundForTesting(bool value) override { + set_updated_title_or_favicon_in_background(value); + } void set_is_connected_to_usb_device(bool is_connected_to_usb_device) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -242,6 +249,10 @@ class PageLiveStateDataImpl obs.OnIsDevToolsOpenChanged(page_node_); } } + void set_updated_title_or_favicon_in_background(bool updated) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + updated_title_or_favicon_in_background_ = updated; + } private: // Make the impl our friend so it can access the constructor and any @@ -268,6 +279,8 @@ class PageLiveStateDataImpl std::map content_settings_ GUARDED_BY_CONTEXT(sequence_checker_); bool is_dev_tools_open_ GUARDED_BY_CONTEXT(sequence_checker_) = false; + bool updated_title_or_favicon_in_background_ + GUARDED_BY_CONTEXT(sequence_checker_) = false; const raw_ptr page_node_; }; @@ -435,6 +448,8 @@ base::Value::Dict PageLiveStateDecorator::DescribePageNodeData( ret.Set("IsActiveTab", data->IsActiveTab()); ret.Set("IsPinnedTab", data->IsPinnedTab()); ret.Set("IsDevToolsOpen", data->IsDevToolsOpen()); + ret.Set("UpdatedTitleOrFaviconInBackground", + data->UpdatedTitleOrFaviconInBackground()); ret.Set("IsNotificationsAllowed", data->IsContentSettingTypeAllowed( ContentSettingsType::NOTIFICATIONS)); @@ -465,6 +480,20 @@ void PageLiveStateDecorator::OnMainFrameUrlChanged(const PageNode* page_node) { #endif } +void PageLiveStateDecorator::OnTitleUpdated(const PageNode* page_node) { + if (!page_node->IsVisible()) { + PageLiveStateDataImpl::GetOrCreate(PageNodeImpl::FromNode(page_node)) + ->set_updated_title_or_favicon_in_background(true); + } +} + +void PageLiveStateDecorator::OnFaviconUpdated(const PageNode* page_node) { + if (!page_node->IsVisible()) { + PageLiveStateDataImpl::GetOrCreate(PageNodeImpl::FromNode(page_node)) + ->set_updated_title_or_favicon_in_background(true); + } +} + void PageLiveStateDecorator::OnContentSettingsReceived( const GURL& url, base::WeakPtr page_node, diff --git a/components/performance_manager/decorators/page_live_state_decorator_unittest.cc b/components/performance_manager/decorators/page_live_state_decorator_unittest.cc index cacdd0063fa30..d2c582d459034 100644 --- a/components/performance_manager/decorators/page_live_state_decorator_unittest.cc +++ b/components/performance_manager/decorators/page_live_state_decorator_unittest.cc @@ -8,9 +8,13 @@ #include "base/functional/bind.h" #include "base/memory/raw_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/run_loop.h" #include "base/task/sequenced_task_runner.h" #include "base/task/thread_pool.h" +#include "base/test/bind.h" +#include "components/performance_manager/graph/page_node_impl.h" +#include "components/performance_manager/public/performance_manager.h" #include "components/performance_manager/test_support/decorators_utils.h" #include "components/performance_manager/test_support/graph_test_harness.h" #include "components/performance_manager/test_support/performance_manager_test_harness.h" @@ -468,6 +472,57 @@ TEST_F(PageLiveStateDecoratorTest, OnIsDevToolsOpenChanged) { VerifyObserverExpectationOnPMSequence( TestPageLiveStateObserver::ObserverFunction::kOnIsDevToolsOpenChanged); } -#endif + +#endif // !BUILDFLAG(IS_ANDROID) + +TEST_F(PageLiveStateDecoratorTest, UpdateTitleInBackground) { + base::WeakPtr node = + PerformanceManager::GetPrimaryPageNodeForWebContents(web_contents()); + base::RunLoop run_loop; + PerformanceManager::CallOnGraph( + FROM_HERE, base::BindLambdaForTesting([&]() { + ASSERT_TRUE(node); + auto* node_impl = PageNodeImpl::FromNode(node.get()); + auto* data = + PageLiveStateDecorator::Data::GetOrCreateForPageNode(node.get()); + + // Updating the title while the node is visible does nothing. + node_impl->SetIsVisible(true); + node_impl->OnTitleUpdated(); + EXPECT_EQ(data->UpdatedTitleOrFaviconInBackground(), false); + + node_impl->SetIsVisible(false); + node_impl->OnTitleUpdated(); + EXPECT_EQ(data->UpdatedTitleOrFaviconInBackground(), true); + + run_loop.Quit(); + })); + run_loop.Run(); +} + +TEST_F(PageLiveStateDecoratorTest, UpdateFaviconInBackground) { + base::WeakPtr node = + PerformanceManager::GetPrimaryPageNodeForWebContents(web_contents()); + base::RunLoop run_loop; + PerformanceManager::CallOnGraph( + FROM_HERE, base::BindLambdaForTesting([&]() { + ASSERT_TRUE(node); + auto* node_impl = PageNodeImpl::FromNode(node.get()); + auto* data = + PageLiveStateDecorator::Data::GetOrCreateForPageNode(node.get()); + + // Updating the favicon while the node is visible does nothing. + node_impl->SetIsVisible(true); + node_impl->OnFaviconUpdated(); + EXPECT_EQ(data->UpdatedTitleOrFaviconInBackground(), false); + + node_impl->SetIsVisible(false); + node_impl->OnFaviconUpdated(); + EXPECT_EQ(data->UpdatedTitleOrFaviconInBackground(), true); + + run_loop.Quit(); + })); + run_loop.Run(); +} } // namespace performance_manager diff --git a/components/performance_manager/performance_manager_tab_helper.cc b/components/performance_manager/performance_manager_tab_helper.cc index 644720afe6a7e..8f35883b43c34 100644 --- a/components/performance_manager/performance_manager_tab_helper.cc +++ b/components/performance_manager/performance_manager_tab_helper.cc @@ -384,7 +384,9 @@ void PerformanceManagerTabHelper::DidFinishNavigation( void PerformanceManagerTabHelper::TitleWasSet(content::NavigationEntry* entry) { DCHECK(primary_page_); - // TODO(siggi): This logic belongs in the policy layer rather than here. + // TODO(crbug.com/1418410): This logic belongs in the policy layer rather than + // here. If a page has no element on first load, the first change of + // title will be ignored no matter much later it happens. if (!primary_page_->first_time_title_set) { primary_page_->first_time_title_set = true; return; @@ -466,7 +468,9 @@ void PerformanceManagerTabHelper::DidUpdateFaviconURL( if (!render_frame_host->IsActive()) return; - // TODO(siggi): This logic belongs in the policy layer rather than here. + // TODO(crbug.com/1418410): This logic belongs in the policy layer rather than + // here. If a page has no favicon on first load, the first change of favicon + // will be ignored no matter much later it happens. if (!primary_page_->first_time_favicon_set) { primary_page_->first_time_favicon_set = true; return; diff --git a/components/performance_manager/public/decorators/page_live_state_decorator.h b/components/performance_manager/public/decorators/page_live_state_decorator.h index c3cef858ed534..94b2934a1f305 100644 --- a/components/performance_manager/public/decorators/page_live_state_decorator.h +++ b/components/performance_manager/public/decorators/page_live_state_decorator.h @@ -117,6 +117,8 @@ class PageLiveStateDecorator : public GraphOwnedDefaultImpl, // PageNode::ObserverDefaultImpl implementation: void OnMainFrameUrlChanged(const PageNode* page_node) override; + void OnTitleUpdated(const PageNode* page_node) override; + void OnFaviconUpdated(const PageNode* page_node) override; void OnContentSettingsReceived( const GURL& url, @@ -152,6 +154,10 @@ class PageLiveStateDecorator::Data { virtual bool IsContentSettingTypeAllowed(ContentSettingsType type) const = 0; virtual bool IsDevToolsOpen() const = 0; + // TODO(https://crbug.com/1418410): Add a notifier for this to + // PageLiveStateObserver. + virtual bool UpdatedTitleOrFaviconInBackground() const = 0; + static const Data* FromPageNode(const PageNode* page_node); static Data* GetOrCreateForPageNode(const PageNode* page_node); @@ -169,6 +175,7 @@ class PageLiveStateDecorator::Data { virtual void SetContentSettingsForTesting( const std::map<ContentSettingsType, ContentSetting>& settings) = 0; virtual void SetIsDevToolsOpenForTesting(bool value) = 0; + virtual void SetUpdatedTitleOrFaviconInBackgroundForTesting(bool value) = 0; protected: base::ObserverList<PageLiveStateObserver> observers_