[TabManager] Keep track of session-restoring tabs.

We need to keep track of tabs which are being restored by session restore (instead of other mechanisms like reopening closed tabs).

This CL adds new API to SessionRestoreObserver and makes TabManager to keep track of the session-restoring tabs.

Bug: 731901
Change-Id: I0d415a689d2b8e49b58cb2af6623ad7fad7ea50a
Reviewed-on: https://chromium-review.googlesource.com/588168Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Zhen Wang <zhenw@chromium.org>
Commit-Queue: Duc Bui <ducbui@google.com>
Cr-Commit-Position: refs/heads/master@{#491395}

chrome/browser/resource_coordinator/tab_manager.cc

@@ -200,6 +200,10 @@ class TabManager::TabManagerSessionRestoreObserver
     tab_manager_->OnSessionRestoreFinishedLoadingTabs();
   }
 
+  void OnWillRestoreTab(WebContents* web_contents) override {
+    tab_manager_->OnWillRestoreTab(web_contents);
+  }
+
  private:
   TabManager* tab_manager_;
 };
@@ -590,6 +594,10 @@ void TabManager::OnSessionRestoreFinishedLoadingTabs() {
   is_session_restore_loading_tabs_ = false;
 }
 
+bool TabManager::IsTabInSessionRestore(WebContents* web_contents) const {
+  return GetWebContentsData(web_contents)->is_in_session_restore();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // TabManager, private:
 
@@ -1147,6 +1155,12 @@ bool TabManager::CanLoadNextTab() const {
   return false;
 }
 
+void TabManager::OnWillRestoreTab(WebContents* web_contents) {
+  WebContentsData* data = GetWebContentsData(web_contents);
+  DCHECK(!data->is_in_session_restore());
+  data->SetIsInSessionRestore(true);
+}
+
 void TabManager::OnDidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   auto it = pending_navigations_.begin();

chrome/browser/resource_coordinator/tab_manager.h

@@ -218,6 +218,10 @@ class TabManager : public TabStripModelObserver,
     return is_session_restore_loading_tabs_;
   }
 
+  // Returns true if the tab was created by session restore and has not finished
+  // the first navigation.
+  bool IsTabInSessionRestore(content::WebContents* web_contents) const;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest, PurgeBackgroundRenderer);
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest, ActivateTabResetPurgeState);
@@ -412,6 +416,7 @@ class TabManager : public TabStripModelObserver,
 
   void OnSessionRestoreStartedLoadingTabs();
   void OnSessionRestoreFinishedLoadingTabs();
+  void OnWillRestoreTab(content::WebContents* web_contents);
 
   // Returns true if TabManager can start loading next tab.
   bool CanLoadNextTab() const;

chrome/browser/resource_coordinator/tab_manager_web_contents_data.cc

@@ -63,6 +63,7 @@ void TabManager::WebContentsData::DidStartNavigation(
 
 void TabManager::WebContentsData::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
+  SetIsInSessionRestore(false);
   g_browser_process->GetTabManager()->OnDidFinishNavigation(navigation_handle);
 }
 
@@ -83,6 +84,7 @@ void TabManager::WebContentsData::WebContentsDestroyed() {
   }
 
   SetTabLoadingState(TAB_IS_NOT_LOADING);
+  SetIsInSessionRestore(false);
   g_browser_process->GetTabManager()->OnWebContentsDestroyed(web_contents());
 }
 
@@ -209,7 +211,8 @@ TabManager::WebContentsData::Data::Data()
       last_inactive_time(TimeTicks::UnixEpoch()),
       engagement_score(-1.0),
       is_auto_discardable(true),
-      tab_loading_state(TAB_IS_NOT_LOADING) {}
+      tab_loading_state(TAB_IS_NOT_LOADING),
+      is_in_session_restore(false) {}
 
 bool TabManager::WebContentsData::Data::operator==(const Data& right) const {
   return is_discarded == right.is_discarded &&

chrome/browser/resource_coordinator/tab_manager_web_contents_data.h

@@ -137,6 +137,12 @@ class TabManager::WebContentsData
     return tab_data_.tab_loading_state;
   }
 
+  void SetIsInSessionRestore(bool is_in_session_restore) {
+    tab_data_.is_in_session_restore = is_in_session_restore;
+  }
+
+  bool is_in_session_restore() const { return tab_data_.is_in_session_restore; }
+
  private:
   // Needed to access tab_data_.
   FRIEND_TEST_ALL_PREFIXES(TabManagerWebContentsDataTest, CopyState);
@@ -168,6 +174,9 @@ class TabManager::WebContentsData
     bool is_auto_discardable;
     // Current loading state of this tab.
     TabLoadingState tab_loading_state;
+    // True if the tab was created by session restore. Remains true until the
+    // end of the first navigation or the tab is closed.
+    bool is_in_session_restore;
   };
 
   // Returns either the system's clock or the test clock. See |test_tick_clock_|

chrome/browser/resource_coordinator/tab_manager_web_contents_data_unittest.cc

@@ -11,6 +11,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 using content::WebContents;
 using content::WebContentsTester;
@@ -41,7 +42,7 @@ class TabManagerWebContentsDataTest : public ChromeRenderViewHostTestHarness {
   TabManager::WebContentsData* CreateWebContentsAndTabData(
       std::unique_ptr<WebContents>* web_contents) {
     web_contents->reset(
-        WebContents::Create(WebContents::CreateParams(profile())));
+        WebContentsTester::CreateTestWebContents(browser_context(), nullptr));
     TabManager::WebContentsData::CreateForWebContents(web_contents->get());
     return TabManager::WebContentsData::FromWebContents(web_contents->get());
   }
@@ -52,6 +53,8 @@ class TabManagerWebContentsDataTest : public ChromeRenderViewHostTestHarness {
   base::SimpleTestTickClock test_clock_;
 };
 
+const char kDefaultUrl[] = "https://www.google.com";
+
 }  // namespace
 
 TEST_F(TabManagerWebContentsDataTest, DiscardState) {
@@ -212,4 +215,24 @@ TEST_F(TabManagerWebContentsDataTest, HistogramsInactiveToReloadTime) {
   histograms.ExpectBucketCount(kHistogramName, 12000, 1);
 }
 
+TEST_F(TabManagerWebContentsDataTest, IsInSessionRestoreWithTabLoading) {
+  EXPECT_FALSE(tab_data()->is_in_session_restore());
+  tab_data()->SetIsInSessionRestore(true);
+  EXPECT_TRUE(tab_data()->is_in_session_restore());
+
+  WebContents* contents = tab_data()->web_contents();
+  WebContentsTester::For(contents)->NavigateAndCommit(GURL(kDefaultUrl));
+  WebContentsTester::For(contents)->TestSetIsLoading(false);
+  EXPECT_FALSE(tab_data()->is_in_session_restore());
+}
+
+TEST_F(TabManagerWebContentsDataTest, IsInSessionRestoreWithTabClose) {
+  EXPECT_FALSE(tab_data()->is_in_session_restore());
+  tab_data()->SetIsInSessionRestore(true);
+  EXPECT_TRUE(tab_data()->is_in_session_restore());
+
+  tab_data()->WebContentsDestroyed();
+  EXPECT_FALSE(tab_data()->is_in_session_restore());
+}
+
 }  // namespace resource_coordinator

chrome/browser/sessions/session_restore.cc

@@ -225,6 +225,7 @@ class SessionRestoreImpl : public content::NotificationObserver {
       web_contents = chrome::ReplaceRestoredTab(
           browser, tab.navigations, selected_index, true, tab.extension_app_id,
           nullptr, tab.user_agent_override);
+      SessionRestore::OnWillRestoreTab(web_contents);
     } else {
       int tab_index =
           use_new_window ? 0 : browser->tab_strip_model()->active_index() + 1;
@@ -233,6 +234,7 @@ class SessionRestoreImpl : public content::NotificationObserver {
           tab.extension_app_id,
           disposition == WindowOpenDisposition::NEW_FOREGROUND_TAB,  // selected
           tab.pinned, true, nullptr, tab.user_agent_override);
+      SessionRestore::OnWillRestoreTab(web_contents);
       // Start loading the tab immediately.
       web_contents->GetController().LoadIfNecessary();
     }
@@ -604,6 +606,8 @@ class SessionRestoreImpl : public content::NotificationObserver {
     if (!web_contents)
       return;
 
+    SessionRestore::OnWillRestoreTab(web_contents);
+
     // Sanitize the last active time.
     base::TimeDelta delta = highest_time - tab.last_active_time;
     web_contents->SetLastActiveTime(now - delta);
@@ -874,6 +878,12 @@ void SessionRestore::NotifySessionRestoreStartedLoadingTabs() {
     observer.OnSessionRestoreStartedLoadingTabs();
 }
 
+// static
+void SessionRestore::OnWillRestoreTab(content::WebContents* web_contents) {
+  for (auto& observer : *observers())
+    observer.OnWillRestoreTab(web_contents);
+}
+
 // static
 base::CallbackList<void(int)>*
     SessionRestore::on_session_restored_callbacks_ = nullptr;

chrome/browser/sessions/session_restore.h

@@ -152,6 +152,9 @@ class SessionRestore {
   // the first session restore.
   static void NotifySessionRestoreStartedLoadingTabs();
 
+  // Is called when session restore is going to restore a tab.
+  static void OnWillRestoreTab(content::WebContents* web_contents);
+
   // Contains all registered observers for session restore events.
   static SessionRestoreObserverList* observers_;
 

chrome/browser/sessions/session_restore_observer.h

@@ -5,6 +5,10 @@
 #ifndef CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_
 #define CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_
 
+namespace content {
+class WebContents;
+}
+
 // Observer of events during session restore. This observer does not cover
 // SessionRestoreImpl::RestoreForeignTab() which restores a single foreign tab.
 class SessionRestoreObserver {
@@ -22,6 +26,10 @@ class SessionRestoreObserver {
   // of memory pressure). This is called on the last session restore when
   // multiple concurrent session restores (on all profiles) occur.
   virtual void OnSessionRestoreFinishedLoadingTabs() {}
+
+  // OnWillRestoreTab() is called right after a tab is created by session
+  // restore.
+  virtual void OnWillRestoreTab(content::WebContents* web_contents) {}
 };
 
 #endif  // CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_

chrome/browser/sessions/session_restore_observer_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/sessions/session_restore_observer.h"
 
+#include <set>
+#include <vector>
+
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/resource_coordinator/tab_manager.h"
 #include "chrome/browser/sessions/session_restore.h"
@@ -35,19 +38,30 @@ class MockSessionRestoreObserver : public SessionRestoreObserver {
     return session_restore_events_;
   }
 
+  const std::set<content::WebContents*>& tabs_restoring() const {
+    return tabs_restoring_;
+  }
+
   // SessionRestoreObserver implementation:
   void OnSessionRestoreStartedLoadingTabs() override {
     session_restore_events_.emplace_back(
         SessionRestoreEvent::STARTED_LOADING_TABS);
   }
-
   void OnSessionRestoreFinishedLoadingTabs() override {
     session_restore_events_.emplace_back(
         SessionRestoreEvent::FINISHED_LOADING_TABS);
   }
+  void OnWillRestoreTab(content::WebContents* contents) override {
+    tabs_restoring_.emplace(contents);
+  }
+
+  void OnDidRestoreTab(content::WebContents* contents) {
+    tabs_restoring_.erase(contents);
+  }
 
  private:
   std::vector<SessionRestoreEvent> session_restore_events_;
+  std::set<content::WebContents*> tabs_restoring_;
 
   DISALLOW_COPY_AND_ASSIGN(MockSessionRestoreObserver);
 };
@@ -77,6 +91,7 @@ class SessionRestoreObserverTest : public ChromeRenderViewHostTestHarness {
   void LoadWebContents(content::WebContents* contents) {
     WebContentsTester::For(contents)->NavigateAndCommit(GURL(kDefaultUrl));
     WebContentsTester::For(contents)->TestSetIsLoading(false);
+    mock_observer_.OnDidRestoreTab(contents);
   }
 
   const std::vector<MockSessionRestoreObserver::SessionRestoreEvent>&
@@ -88,6 +103,10 @@ class SessionRestoreObserverTest : public ChromeRenderViewHostTestHarness {
     return session_restore_events().size();
   }
 
+  size_t number_of_tabs_restoring() const {
+    return mock_observer_.tabs_restoring().size();
+  }
+
  private:
   MockSessionRestoreObserver mock_observer_;
   std::vector<RestoredTab> restored_tabs_;
@@ -97,35 +116,53 @@ class SessionRestoreObserverTest : public ChromeRenderViewHostTestHarness {
 
 TEST_F(SessionRestoreObserverTest, SingleSessionRestore) {
   SessionRestore::NotifySessionRestoreStartedLoadingTabs();
+  SessionRestore::OnWillRestoreTab(web_contents());
   RestoreTabs();
+
   ASSERT_EQ(1u, number_of_session_restore_events());
   EXPECT_EQ(
       MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS,
       session_restore_events()[0]);
+  EXPECT_EQ(1u, number_of_tabs_restoring());
 
   LoadWebContents(web_contents());
+
   ASSERT_EQ(2u, number_of_session_restore_events());
   EXPECT_EQ(
       MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS,
       session_restore_events()[1]);
+  EXPECT_EQ(0u, number_of_tabs_restoring());
 }
 
 TEST_F(SessionRestoreObserverTest, SequentialSessionRestores) {
-  const int number_of_session_restores = 3;
+  const size_t number_of_session_restores = 3;
   size_t event_index = 0;
-  for (int i = 0; i < number_of_session_restores; ++i) {
+  std::vector<std::unique_ptr<content::WebContents>> different_test_contents;
+
+  for (size_t i = 0; i < number_of_session_restores; ++i) {
+    different_test_contents.emplace_back(
+        WebContentsTester::CreateTestWebContents(browser_context(), nullptr));
+    content::WebContents* test_contents = different_test_contents.back().get();
+    std::vector<RestoredTab> restored_tabs{
+        RestoredTab(test_contents, false, false, false)};
+
     SessionRestore::NotifySessionRestoreStartedLoadingTabs();
-    RestoreTabs();
+    SessionRestore::OnWillRestoreTab(test_contents);
+    TabLoader::RestoreTabs(restored_tabs, base::TimeTicks());
+
     ASSERT_EQ(event_index + 1, number_of_session_restore_events());
     EXPECT_EQ(
         MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS,
         session_restore_events()[event_index++]);
+    EXPECT_EQ(1u, number_of_tabs_restoring());
+
+    LoadWebContents(test_contents);
 
-    LoadWebContents(web_contents());
     ASSERT_EQ(event_index + 1, number_of_session_restore_events());
     EXPECT_EQ(
         MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS,
         session_restore_events()[event_index++]);
+    EXPECT_EQ(0u, number_of_tabs_restoring());
   }
 }
 
@@ -136,12 +173,16 @@ TEST_F(SessionRestoreObserverTest, ConcurrentSessionRestores) {
   another_restored_tabs.emplace_back(test_contents.get(), false, false, false);
 
   SessionRestore::NotifySessionRestoreStartedLoadingTabs();
+  SessionRestore::OnWillRestoreTab(web_contents());
+  SessionRestore::OnWillRestoreTab(test_contents.get());
   RestoreTabs();
   TabLoader::RestoreTabs(another_restored_tabs, base::TimeTicks());
+
   ASSERT_EQ(1u, number_of_session_restore_events());
   EXPECT_EQ(
       MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS,
       session_restore_events()[0]);
+  EXPECT_EQ(2u, number_of_tabs_restoring());
 
   LoadWebContents(web_contents());
   LoadWebContents(test_contents.get());
@@ -150,6 +191,7 @@ TEST_F(SessionRestoreObserverTest, ConcurrentSessionRestores) {
   EXPECT_EQ(
       MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS,
       session_restore_events()[1]);
+  EXPECT_EQ(0u, number_of_tabs_restoring());
 }
 
 TEST_F(SessionRestoreObserverTest, TabManagerShouldObserveSessionRestore) {
@@ -163,13 +205,17 @@ TEST_F(SessionRestoreObserverTest, TabManagerShouldObserveSessionRestore) {
   resource_coordinator::TabManager* tab_manager =
       g_browser_process->GetTabManager();
   EXPECT_FALSE(tab_manager->IsSessionRestoreLoadingTabs());
+  EXPECT_FALSE(tab_manager->IsTabInSessionRestore(test_contents.get()));
 
   SessionRestore::NotifySessionRestoreStartedLoadingTabs();
+  SessionRestore::OnWillRestoreTab(test_contents.get());
   EXPECT_TRUE(tab_manager->IsSessionRestoreLoadingTabs());
+  EXPECT_TRUE(tab_manager->IsTabInSessionRestore(test_contents.get()));
   TabLoader::RestoreTabs(restored_tabs, base::TimeTicks());
 
   WebContentsTester::For(test_contents.get())
       ->NavigateAndCommit(GURL("about:blank"));
   WebContentsTester::For(test_contents.get())->TestSetIsLoading(false);
   EXPECT_FALSE(tab_manager->IsSessionRestoreLoadingTabs());
+  EXPECT_FALSE(tab_manager->IsTabInSessionRestore(test_contents.get()));
 }