Exclude tabs that are pinned or with dev tools from discarding

Excluding pinned tabs and tabs with dev tools from proactive discarding.

Bug: 1406237
Change-Id: Iff1e3c55e9c2fd21c627ade03f80299517bb20bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4237748
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Reviewed-by: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Commit-Queue: Eshwar Stalin <estalin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104713}

chrome/browser/performance_manager/decorators/helpers/page_live_state_decorator_helper.cc

@@ -64,6 +64,24 @@ class ActiveTabObserver : public TabStripModelObserver,
         PageLiveStateDecorator::SetIsActiveTab(selection.new_contents, true);
       }
     }
+
+    if (change.type() == TabStripModelChange::kInserted) {
+      for (const TabStripModelChange::ContentsWithIndex& tab :
+           change.GetInsert()->contents) {
+        // Pinned tabs can be restored from previous session in pinned state
+        // and hence won't trigger a pinned state changed event
+        PageLiveStateDecorator::SetIsPinnedTab(
+            tab.contents, tab_strip_model->IsTabPinned(tab.index));
+      }
+    }
+  }
+
+  void TabPinnedStateChanged(TabStripModel* tab_strip_model,
+                             content::WebContents* contents,
+                             int index) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    PageLiveStateDecorator::SetIsPinnedTab(contents,
+                                           tab_strip_model->IsTabPinned(index));
   }
 
   // BrowserListObserver:
@@ -197,11 +215,15 @@ PageLiveStateDecoratorHelper::PageLiveStateDecoratorHelper() {
 #if !BUILDFLAG(IS_ANDROID)
   active_tab_observer_ = std::make_unique<ActiveTabObserver>();
 #endif  // !BUILDFLAG(IS_ANDROID)
+
+  content::DevToolsAgentHost::AddObserver(this);
 }
 
 PageLiveStateDecoratorHelper::~PageLiveStateDecoratorHelper() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  content::DevToolsAgentHost::RemoveObserver(this);
+
   MediaCaptureDevicesDispatcher::GetInstance()
       ->GetMediaStreamCaptureIndicator()
       ->RemoveObserver(this);
@@ -253,6 +275,26 @@ void PageLiveStateDecoratorHelper::OnIsCapturingDisplayChanged(
                                                       is_capturing_display);
 }
 
+void PageLiveStateDecoratorHelper::DevToolsAgentHostAttached(
+    content::DevToolsAgentHost* agent_host) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (agent_host->GetType() == content::DevToolsAgentHost::kTypePage &&
+      agent_host->GetWebContents() != nullptr) {
+    PageLiveStateDecorator::SetIsDevToolsOpen(agent_host->GetWebContents(),
+                                              true);
+  }
+}
+
+void PageLiveStateDecoratorHelper::DevToolsAgentHostDetached(
+    content::DevToolsAgentHost* agent_host) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (agent_host->GetType() == content::DevToolsAgentHost::kTypePage &&
+      agent_host->GetWebContents() != nullptr) {
+    PageLiveStateDecorator::SetIsDevToolsOpen(agent_host->GetWebContents(),
+                                              false);
+  }
+}
+
 void PageLiveStateDecoratorHelper::OnPageNodeCreatedForWebContents(
     content::WebContents* web_contents) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chrome/browser/performance_manager/decorators/helpers/page_live_state_decorator_helper.h

@@ -9,6 +9,7 @@
 #include "base/sequence_checker.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "components/performance_manager/public/performance_manager_main_thread_observer.h"
+#include "content/public/browser/devtools_agent_host.h"
 
 namespace performance_manager {
 
@@ -18,7 +19,8 @@ class ActiveTabObserver;
 
 class PageLiveStateDecoratorHelper
     : public MediaStreamCaptureIndicator::Observer,
-      public PerformanceManagerMainThreadObserverDefaultImpl {
+      public PerformanceManagerMainThreadObserverDefaultImpl,
+      public content::DevToolsAgentHostObserver {
  public:
   PageLiveStateDecoratorHelper();
   ~PageLiveStateDecoratorHelper() override;
@@ -39,6 +41,12 @@ class PageLiveStateDecoratorHelper
   void OnIsCapturingDisplayChanged(content::WebContents* contents,
                                    bool is_capturing_display) override;
 
+  // content::DevToolsAgentHostObserver:
+  void DevToolsAgentHostAttached(
+      content::DevToolsAgentHost* agent_host) override;
+  void DevToolsAgentHostDetached(
+      content::DevToolsAgentHost* agent_host) override;
+
   // PerformanceManagerMainThreadObserver:
   void OnPageNodeCreatedForWebContents(
       content::WebContents* web_contents) override;

chrome/browser/performance_manager/decorators/helpers/page_live_state_decorator_helper_unittest.cc

@@ -343,6 +343,26 @@ TEST_F(PageLiveStateDecoratorHelperTabsTest, IsActiveTab) {
       other_contents, &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
       &PageLiveStateDecorator::Data::IsActiveTab, true);
 }
+
+TEST_F(PageLiveStateDecoratorHelperTabsTest, IsPinnedTab) {
+  // Create a tab, it's associated PageNode should be the active one.
+  AddTab(browser(), GURL("http://foo/1"));
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetWebContentsAt(0);
+  testing::TestPageNodePropertyOnPMSequence(
+      contents, &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
+      &PageLiveStateDecorator::Data::IsPinnedTab, false);
+
+  browser()->tab_strip_model()->SetTabPinned(0, true);
+  testing::TestPageNodePropertyOnPMSequence(
+      contents, &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
+      &PageLiveStateDecorator::Data::IsPinnedTab, true);
+
+  browser()->tab_strip_model()->SetTabPinned(0, false);
+  testing::TestPageNodePropertyOnPMSequence(
+      contents, &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
+      &PageLiveStateDecorator::Data::IsPinnedTab, false);
+}
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 }  // namespace performance_manager

chrome/browser/performance_manager/policies/page_discarding_helper.cc

@@ -357,10 +357,16 @@ PageDiscardingHelper::CanUrgentlyDiscard(
       return CanUrgentlyDiscardResult::kProtected;
     if (live_state_data->IsActiveTab())
       return CanUrgentlyDiscardResult::kProtected;
+    if (live_state_data->IsPinnedTab()) {
+      return CanUrgentlyDiscardResult::kProtected;
+    }
     if (live_state_data->IsContentSettingTypeAllowed(
             ContentSettingsType::NOTIFICATIONS)) {
       return CanUrgentlyDiscardResult::kProtected;
     }
+    if (live_state_data->IsDevToolsOpen()) {
+      return CanUrgentlyDiscardResult::kProtected;
+    }
 #if !BUILDFLAG(IS_CHROMEOS)
     // TODO(sebmarchand): Skip this check if the Entreprise memory limit is set.
     if (live_state_data->WasDiscarded())
@@ -374,8 +380,6 @@ PageDiscardingHelper::CanUrgentlyDiscard(
   if (page_node->HadFormInteraction())
     return CanUrgentlyDiscardResult::kProtected;
 
-  // TODO(sebmarchand): Do not discard pages if they're connected to DevTools.
-
   // TODO(sebmarchand): Do not discard crashed tabs.
 
   return CanUrgentlyDiscardResult::kEligible;

chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc

@@ -286,6 +286,22 @@ TEST_F(PageDiscardingHelperTest, TestCannotDiscardPageOnNoDiscardList) {
           page_node()));
 }
 
+TEST_F(PageDiscardingHelperTest, TestCannotDiscardIsPinnedTab) {
+  PageLiveStateDecorator::Data::GetOrCreateForPageNode(page_node())
+      ->SetIsPinnedTabForTesting(true);
+  EXPECT_FALSE(
+      PageDiscardingHelper::GetFromGraph(graph())->CanUrgentlyDiscardForTesting(
+          page_node()));
+}
+
+TEST_F(PageDiscardingHelperTest, TestCannotDiscardIsDevToolsOpen) {
+  PageLiveStateDecorator::Data::GetOrCreateForPageNode(page_node())
+      ->SetIsDevToolsOpenForTesting(true);
+  EXPECT_FALSE(
+      PageDiscardingHelper::GetFromGraph(graph())->CanUrgentlyDiscardForTesting(
+          page_node()));
+}
+
 // Tests UrgentlyDiscardMultiplePages.
 
 TEST_F(PageDiscardingHelperTest, UrgentlyDiscardMultiplePagesNoCandidate) {

chrome/browser/performance_manager/policies/page_freezing_policy.h

@@ -119,7 +119,9 @@ class PageFreezingPolicy : public GraphObserver,
   void OnIsAutoDiscardableChanged(const PageNode* page_node) override {}
   void OnWasDiscardedChanged(const PageNode* page_node) override {}
   void OnIsActiveTabChanged(const PageNode* page_node) override {}
+  void OnIsPinnedTabChanged(const PageNode* page_node) override {}
   void OnContentSettingsChanged(const PageNode* page_node) override {}
+  void OnIsDevToolsOpenChanged(const PageNode* page_node) override {}
 
   // Helper function that either calls SubmitNegativeVote() or
   // InvalidateNegativeVote() when the value of a property changes.

components/performance_manager/decorators/page_live_state_decorator.cc

@@ -75,11 +75,19 @@ class PageLiveStateDataImpl
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return is_active_tab_;
   }
+  bool IsPinnedTab() const override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return is_pinned_tab_;
+  }
   bool IsContentSettingTypeAllowed(ContentSettingsType type) const override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     auto it = content_settings_.find(type);
     return it != content_settings_.end() && it->second == CONTENT_SETTING_ALLOW;
   }
+  bool IsDevToolsOpen() const override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return is_dev_tools_open_;
+  }
 
   void SetIsConnectedToUSBDeviceForTesting(bool value) override {
     set_is_connected_to_usb_device(value);
@@ -111,10 +119,16 @@ class PageLiveStateDataImpl
   void SetIsActiveTabForTesting(bool value) override {
     set_is_active_tab(value);
   }
+  void SetIsPinnedTabForTesting(bool value) override {
+    set_is_pinned_tab(value);
+  }
   void SetContentSettingsForTesting(
       const std::map<ContentSettingsType, ContentSetting>& settings) override {
     set_content_settings(settings);
   }
+  void SetIsDevToolsOpenForTesting(bool value) override {
+    set_is_dev_tools_open(value);
+  }
 
   void set_is_connected_to_usb_device(bool is_connected_to_usb_device) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -197,6 +211,16 @@ class PageLiveStateDataImpl
     for (auto& obs : observers_)
       obs.OnIsActiveTabChanged(page_node_);
   }
+  void set_is_pinned_tab(bool is_pinned_tab) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    if (is_pinned_tab_ == is_pinned_tab) {
+      return;
+    }
+    is_pinned_tab_ = is_pinned_tab;
+    for (auto& obs : observers_) {
+      obs.OnIsPinnedTabChanged(page_node_);
+    }
+  }
   void set_content_settings(
       std::map<ContentSettingsType, ContentSetting> settings) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -208,6 +232,16 @@ class PageLiveStateDataImpl
     for (auto& obs : observers_)
       obs.OnContentSettingsChanged(page_node_);
   }
+  void set_is_dev_tools_open(bool is_dev_tools_open) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    if (is_dev_tools_open_ == is_dev_tools_open) {
+      return;
+    }
+    is_dev_tools_open_ = is_dev_tools_open;
+    for (auto& obs : observers_) {
+      obs.OnIsDevToolsOpenChanged(page_node_);
+    }
+  }
 
  private:
   // Make the impl our friend so it can access the constructor and any
@@ -230,8 +264,10 @@ class PageLiveStateDataImpl
   bool is_auto_discardable_ GUARDED_BY_CONTEXT(sequence_checker_) = true;
   bool was_discarded_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
   bool is_active_tab_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
+  bool is_pinned_tab_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
   std::map<ContentSettingsType, ContentSetting> content_settings_
       GUARDED_BY_CONTEXT(sequence_checker_);
+  bool is_dev_tools_open_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
 
   const raw_ptr<const PageNode> page_node_;
 };
@@ -346,6 +382,13 @@ void PageLiveStateDecorator::SetIsActiveTab(content::WebContents* contents,
       contents, &PageLiveStateDataImpl::set_is_active_tab, is_active_tab);
 }
 
+// static
+void PageLiveStateDecorator::SetIsPinnedTab(content::WebContents* contents,
+                                            bool is_pinned_tab) {
+  SetPropertyForWebContentsPageNode(
+      contents, &PageLiveStateDataImpl::set_is_pinned_tab, is_pinned_tab);
+}
+
 // static
 void PageLiveStateDecorator::SetContentSettings(
     content::WebContents* contents,
@@ -354,6 +397,14 @@ void PageLiveStateDecorator::SetContentSettings(
       contents, &PageLiveStateDataImpl::set_content_settings, settings);
 }
 
+// static
+void PageLiveStateDecorator::SetIsDevToolsOpen(content::WebContents* contents,
+                                               bool is_dev_tools_open) {
+  SetPropertyForWebContentsPageNode(
+      contents, &PageLiveStateDataImpl::set_is_dev_tools_open,
+      is_dev_tools_open);
+}
+
 void PageLiveStateDecorator::OnPassedToGraph(Graph* graph) {
   graph->GetNodeDataDescriberRegistry()->RegisterDescriber(this,
                                                            kDescriberName);

components/performance_manager/decorators/page_live_state_decorator_unittest.cc

@@ -47,7 +47,9 @@ class TestPageLiveStateObserver : public PageLiveStateObserver {
     kOnIsAutoDiscardableChanged,
     kOnWasDiscardedChanged,
     kOnIsActiveTabChanged,
+    kOnIsPinnedTabChanged,
     kOnContentSettingsChanged,
+    kOnIsDevToolsOpenChanged,
   };
 
   void OnIsConnectedToUSBDeviceChanged(const PageNode* page_node) override {
@@ -93,10 +95,18 @@ class TestPageLiveStateObserver : public PageLiveStateObserver {
     latest_function_called_ = ObserverFunction::kOnIsActiveTabChanged;
     page_node_passed_ = page_node;
   }
+  void OnIsPinnedTabChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsPinnedTabChanged;
+    page_node_passed_ = page_node;
+  }
   void OnContentSettingsChanged(const PageNode* page_node) override {
     latest_function_called_ = ObserverFunction::kOnContentSettingsChanged;
     page_node_passed_ = page_node;
   }
+  void OnIsDevToolsOpenChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsDevToolsOpenChanged;
+    page_node_passed_ = page_node;
+  }
 
   ObserverFunction latest_function_called_ = ObserverFunction::kNone;
   raw_ptr<const PageNode> page_node_passed_ = nullptr;
@@ -310,6 +320,16 @@ TEST_F(PageLiveStateDecoratorTest, OnIsActiveTabChanged) {
       TestPageLiveStateObserver::ObserverFunction::kOnIsActiveTabChanged);
 }
 
+TEST_F(PageLiveStateDecoratorTest, OnIsPinnedTabChanged) {
+  testing::EndToEndBooleanPropertyTest(
+      web_contents(), &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
+      &PageLiveStateDecorator::Data::IsPinnedTab,
+      &PageLiveStateDecorator::SetIsPinnedTab,
+      /*default_state=*/false);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsPinnedTabChanged);
+}
+
 TEST_F(PageLiveStateDecoratorTest, OnContentSettingsChanged) {
   base::WeakPtr<PageNode> node =
       PerformanceManager::GetPrimaryPageNodeForWebContents(web_contents());
@@ -438,6 +458,16 @@ TEST_F(PageLiveStateDecoratorTest, GetContentSettingsOnNavigation) {
   VerifyObserverExpectationOnPMSequence(
       TestPageLiveStateObserver::ObserverFunction::kOnContentSettingsChanged);
 }
+
+TEST_F(PageLiveStateDecoratorTest, OnIsDevToolsOpenChanged) {
+  testing::EndToEndBooleanPropertyTest(
+      web_contents(), &PageLiveStateDecorator::Data::GetOrCreateForPageNode,
+      &PageLiveStateDecorator::Data::IsDevToolsOpen,
+      &PageLiveStateDecorator::SetIsDevToolsOpen,
+      /*default_state=*/false);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsDevToolsOpenChanged);
+}
 #endif
 
 }  // namespace performance_manager