Consider page title and favicon in CanUrgentlyDiscard

Adds UpdatedTitleOrFaviconInBackground to PageLiveState, and checks it
in PageDiscardingHelper::CanUrgentlyDiscard() and PageTimelineMonitor.

Also updates PageDiscardingHelper::DescribePageNodeData() to show the
result of calling CanUrgentlyDiscard().

R=anthonyvd

Bug: 1418410
Change-Id: I976b346805303d6c23d00aa43182345888d907fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4336201
Commit-Queue: Joe Mason <joenotcharles@google.com>
Auto-Submit: Joe Mason <joenotcharles@google.com>
Reviewed-by: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118845}

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);

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;

chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc

@@ -163,38 +163,30 @@ 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);
   mock_graph.page->SetUkmSourceId(mock_source_id);
   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) {

chrome/browser/performance_manager/policies/page_discarding_helper.cc

@@ -5,12 +5,14 @@
 #include "chrome/browser/performance_manager/policies/page_discarding_helper.h"
 
 #include <memory>
+#include <utility>
 
 #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<void(bool)> 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;
 }
 

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<void(bool)> post_discard_cb = base::DoNothing());
 
   // PageNodeObserver:
   void OnBeforePageNodeRemoved(const PageNode* page_node) override;