Plumb base::Optional<SessionID> to CreateHistoricalTab() and AddRestoredTab

ClosedTabCache is a desktop feature to instantly restore the
closed tabs. To able to do that, each tab should be identified by
a unique id which in our case is SessionID.

To accommodate this, we need to send the SessionID whenever user
tries to restores the tab using AddRestoredTab.

This CL plumbs through CreateRestoredTab which restores the WebContents
from the cache in the event of a cache hit. Now passing SessionID as
an optional argument through the function calls for AddRestoredTab.

BUG=1100946

Change-Id: I05366c8ef3ced9b901aa5c124d85b025a7fd463f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2564812
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832718}

chrome/browser/ui/android/tab_model/android_live_tab_context.cc

@@ -120,7 +120,8 @@ sessions::LiveTab* AndroidLiveTabContext::AddRestoredTab(
     bool pin,
     bool from_last_session,
     const sessions::PlatformSpecificTabData* tab_platform_data,
-    const sessions::SerializedUserAgentOverride& user_agent_override) {
+    const sessions::SerializedUserAgentOverride& user_agent_override,
+    const SessionID* tab_id) {
   Profile* profile = tab_model_->GetProfile();
 
   // Prepare navigation history.

chrome/browser/ui/android/tab_model/android_live_tab_context.h

@@ -57,8 +57,8 @@ class AndroidLiveTabContext : public sessions::LiveTabContext {
       bool pin,
       bool from_last_session,
       const sessions::PlatformSpecificTabData* storage_namespace,
-      const sessions::SerializedUserAgentOverride& user_agent_override)
-      override;
+      const sessions::SerializedUserAgentOverride& user_agent_override,
+      const SessionID* tab_id) override;
   sessions::LiveTab* ReplaceRestoredTab(
       const std::vector<sessions::SerializedNavigationEntry>& navigations,
       base::Optional<tab_groups::TabGroupId> group,

chrome/browser/ui/browser_live_tab_context.cc

@@ -140,7 +140,8 @@ sessions::LiveTab* BrowserLiveTabContext::AddRestoredTab(
     bool pin,
     bool from_last_session,
     const sessions::PlatformSpecificTabData* tab_platform_data,
-    const sessions::SerializedUserAgentOverride& user_agent_override) {
+    const sessions::SerializedUserAgentOverride& user_agent_override,
+    const SessionID* tab_id) {
   SessionStorageNamespace* storage_namespace =
       tab_platform_data
           ? static_cast<const sessions::ContentPlatformSpecificTabData*>(

chrome/browser/ui/browser_live_tab_context.h

@@ -65,8 +65,8 @@ class BrowserLiveTabContext : public sessions::LiveTabContext {
       bool pin,
       bool from_last_session,
       const sessions::PlatformSpecificTabData* storage_namespace,
-      const sessions::SerializedUserAgentOverride& user_agent_override)
-      override;
+      const sessions::SerializedUserAgentOverride& user_agent_override,
+      const SessionID* tab_id) override;
   sessions::LiveTab* ReplaceRestoredTab(
       const std::vector<sessions::SerializedNavigationEntry>& navigations,
       base::Optional<tab_groups::TabGroupId> group,

chrome/browser/ui/browser_tab_strip_model_delegate.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/ui/unload_controller.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/sessions/content/content_live_tab.h"
+#include "components/sessions/core/session_id.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/tab_groups/tab_group_id.h"
 #include "content/public/browser/site_instance.h"
@@ -37,7 +38,7 @@ namespace chrome {
 BrowserTabStripModelDelegate::BrowserTabStripModelDelegate(Browser* browser)
     : browser_(browser) {}
 
-BrowserTabStripModelDelegate::~BrowserTabStripModelDelegate() {}
+BrowserTabStripModelDelegate::~BrowserTabStripModelDelegate() = default;
 
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserTabStripModelDelegate, TabStripModelDelegate implementation:
@@ -141,22 +142,23 @@ void BrowserTabStripModelDelegate::MoveGroupToNewWindow(
   chrome::MoveTabsToNewWindow(browser_, indices, group);
 }
 
-void BrowserTabStripModelDelegate::CreateHistoricalTab(
+base::Optional<SessionID> BrowserTabStripModelDelegate::CreateHistoricalTab(
     content::WebContents* contents) {
   // We don't create historical tabs for incognito windows or windows without
   // profiles.
   if (!browser_->profile() || browser_->profile()->IsOffTheRecord())
-    return;
+    return base::nullopt;
 
   sessions::TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
 
   // We only create historical tab entries for tabbed browser windows.
   if (service && browser_->CanSupportWindowFeature(Browser::FEATURE_TABSTRIP)) {
-    service->CreateHistoricalTab(
+    return service->CreateHistoricalTab(
         sessions::ContentLiveTab::GetForWebContents(contents),
         browser_->tab_strip_model()->GetIndexOfWebContents(contents));
   }
+  return base::nullopt;
 }
 
 bool BrowserTabStripModelDelegate::RunUnloadListenerBeforeClosing(

chrome/browser/ui/browser_tab_strip_model_delegate.h

@@ -42,7 +42,8 @@ class BrowserTabStripModelDelegate : public TabStripModelDelegate {
   bool CanMoveTabsToWindow(const std::vector<int>& indices) override;
   void MoveTabsToNewWindow(const std::vector<int>& indices) override;
   void MoveGroupToNewWindow(const tab_groups::TabGroupId& group) override;
-  void CreateHistoricalTab(content::WebContents* contents) override;
+  base::Optional<SessionID> CreateHistoricalTab(
+      content::WebContents* contents) override;
   bool RunUnloadListenerBeforeClosing(content::WebContents* contents) override;
   bool ShouldRunUnloadListenerBeforeClosing(
       content::WebContents* contents) override;

chrome/browser/ui/tabs/tab_strip_model_delegate.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/optional.h"
+#include "components/sessions/core/session_id.h"
 #include "components/tab_groups/tab_group_id.h"
 
 class Browser;
@@ -115,8 +116,10 @@ class TabStripModelDelegate {
   virtual void MoveGroupToNewWindow(const tab_groups::TabGroupId& group) = 0;
 
   // Creates an entry in the historical tab database for the specified
-  // WebContents.
-  virtual void CreateHistoricalTab(content::WebContents* contents) = 0;
+  // WebContents. Returns the tab's unique SessionID if a historical tab was
+  // created.
+  virtual base::Optional<SessionID> CreateHistoricalTab(
+      content::WebContents* contents) = 0;
 
   // Runs any unload listeners associated with the specified WebContents
   // before it is closed. If there are unload listeners that need to be run,

chrome/browser/ui/tabs/test_tab_strip_model_delegate.cc

@@ -67,8 +67,9 @@ void TestTabStripModelDelegate::MoveTabsToNewWindow(
 void TestTabStripModelDelegate::MoveGroupToNewWindow(
     const tab_groups::TabGroupId& group) {}
 
-void TestTabStripModelDelegate::CreateHistoricalTab(
+base::Optional<SessionID> TestTabStripModelDelegate::CreateHistoricalTab(
     content::WebContents* contents) {
+  return base::nullopt;
 }
 
 bool TestTabStripModelDelegate::ShouldRunUnloadListenerBeforeClosing(

chrome/browser/ui/tabs/test_tab_strip_model_delegate.h

@@ -41,7 +41,8 @@ class TestTabStripModelDelegate : public TabStripModelDelegate {
   bool CanMoveTabsToWindow(const std::vector<int>& indices) override;
   void MoveTabsToNewWindow(const std::vector<int>& indices) override;
   void MoveGroupToNewWindow(const tab_groups::TabGroupId& group) override;
-  void CreateHistoricalTab(content::WebContents* contents) override;
+  base::Optional<SessionID> CreateHistoricalTab(
+      content::WebContents* contents) override;
   bool ShouldRunUnloadListenerBeforeClosing(
       content::WebContents* contents) override;
   bool RunUnloadListenerBeforeClosing(content::WebContents* contents) override;

components/sessions/core/live_tab_context.h

@@ -61,6 +61,8 @@ class SESSIONS_EXPORT LiveTabContext {
   // Note: |tab_platform_data| may be null (e.g., if |from_last_session| is
   // true, as this data is not persisted, or if the platform does not provide
   // platform-specific data).
+  // |tab_id| is the tab's unique SessionID. Only present if a historical tab
+  // has been created by TabRestoreService.
   virtual LiveTab* AddRestoredTab(
       const std::vector<SerializedNavigationEntry>& navigations,
       int tab_index,
@@ -72,7 +74,8 @@ class SESSIONS_EXPORT LiveTabContext {
       bool pin,
       bool from_last_session,
       const PlatformSpecificTabData* tab_platform_data,
-      const sessions::SerializedUserAgentOverride& user_agent_override) = 0;
+      const sessions::SerializedUserAgentOverride& user_agent_override,
+      const SessionID* tab_id) = 0;
 
   // Note: |tab_platform_data| may be null (e.g., if |from_last_session| is
   // true, as this data is not persisted, or if the platform does not provide

components/sessions/core/tab_restore_service.h

@@ -174,8 +174,10 @@ class SESSIONS_EXPORT TabRestoreService : public KeyedService {
   virtual void RemoveObserver(TabRestoreServiceObserver* observer) = 0;
 
   // Creates a Tab to represent |live_tab| and notifies observers the list of
-  // entries has changed.
-  virtual void CreateHistoricalTab(LiveTab* live_tab, int index) = 0;
+  // entries has changed. If successful, returns the unique SessionID associated
+  // with the Tab.
+  virtual base::Optional<SessionID> CreateHistoricalTab(LiveTab* live_tab,
+                                                        int index) = 0;
 
   // TODO(blundell): Rename and fix comment.
   // Invoked when a browser is closing. If |context| is a tabbed browser with

components/sessions/core/tab_restore_service_helper.cc

@@ -29,6 +29,7 @@
 #include "components/sessions/core/live_tab.h"
 #include "components/sessions/core/live_tab_context.h"
 #include "components/sessions/core/serialized_navigation_entry.h"
+#include "components/sessions/core/session_id.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sessions/core/tab_restore_service_client.h"
 #include "components/sessions/core/tab_restore_service_observer.h"
@@ -90,24 +91,27 @@ void TabRestoreServiceHelper::RemoveObserver(
   observer_list_.RemoveObserver(observer);
 }
 
-void TabRestoreServiceHelper::CreateHistoricalTab(LiveTab* live_tab,
-                                                  int index) {
+base::Optional<SessionID> TabRestoreServiceHelper::CreateHistoricalTab(
+    LiveTab* live_tab,
+    int index) {
   if (restoring_)
-    return;
+    return base::nullopt;
 
   // If an entire window is being closed than all of the tabs have already
   // been persisted via "BrowserClosing". Ignore the subsequent tab closing
   // notifications.
   LiveTabContext* context = client_->FindLiveTabContextForTab(live_tab);
   if (closing_contexts_.find(context) != closing_contexts_.end())
-    return;
+    return base::nullopt;
 
   auto local_tab = std::make_unique<Tab>();
   PopulateTab(local_tab.get(), index, context, live_tab);
   if (local_tab->navigations.empty())
-    return;
+    return base::nullopt;
 
+  SessionID id = local_tab->id;
   AddEntry(std::move(local_tab), true, true);
+  return id;
 }
 
 void TabRestoreServiceHelper::BrowserClosing(LiveTabContext* context) {
@@ -349,7 +353,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
               tab.group_visual_data.value_or(tab_groups::TabGroupVisualData()),
               static_cast<int>(tab_i) == window.selected_tab_index, tab.pinned,
               tab.from_last_session, tab.platform_data.get(),
-              tab.user_agent_override);
+              tab.user_agent_override, nullptr);
           if (restored_tab) {
             client_->OnTabRestored(
                 tab.navigations.at(tab.current_navigation_index).virtual_url());
@@ -665,8 +669,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
         tab.extension_app_id, tab.group,
         tab.group_visual_data.value_or(tab_groups::TabGroupVisualData()),
         disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB, tab.pinned,
-        tab.from_last_session, tab.platform_data.get(),
-        tab.user_agent_override);
+        tab.from_last_session, tab.platform_data.get(), tab.user_agent_override,
+        &tab.id);
   }
   client_->OnTabRestored(
       tab.navigations.at(tab.current_navigation_index).virtual_url());

components/sessions/core/tab_restore_service_helper.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "build/build_config.h"
@@ -84,7 +85,7 @@ class SESSIONS_EXPORT TabRestoreServiceHelper
   // Helper methods used to implement TabRestoreService.
   void AddObserver(TabRestoreServiceObserver* observer);
   void RemoveObserver(TabRestoreServiceObserver* observer);
-  void CreateHistoricalTab(LiveTab* live_tab, int index);
+  base::Optional<SessionID> CreateHistoricalTab(LiveTab* live_tab, int index);
   void BrowserClosing(LiveTabContext* context);
   void BrowserClosed(LiveTabContext* context);
   void ClearEntries();

components/sessions/core/tab_restore_service_impl.cc

@@ -27,6 +27,7 @@
 #include "components/sessions/core/command_storage_manager_delegate.h"
 #include "components/sessions/core/session_command.h"
 #include "components/sessions/core/session_constants.h"
+#include "components/sessions/core/session_id.h"
 #include "components/sessions/core/snapshotting_command_storage_manager.h"
 #include "components/tab_groups/tab_group_color.h"
 #include "components/tab_groups/tab_group_id.h"
@@ -1223,8 +1224,10 @@ void TabRestoreServiceImpl::RemoveObserver(
   helper_.RemoveObserver(observer);
 }
 
-void TabRestoreServiceImpl::CreateHistoricalTab(LiveTab* live_tab, int index) {
-  helper_.CreateHistoricalTab(live_tab, index);
+base::Optional<SessionID> TabRestoreServiceImpl::CreateHistoricalTab(
+    LiveTab* live_tab,
+    int index) {
+  return helper_.CreateHistoricalTab(live_tab, index);
 }
 
 void TabRestoreServiceImpl::BrowserClosing(LiveTabContext* context) {

components/sessions/core/tab_restore_service_impl.h

@@ -10,6 +10,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/sessions/core/sessions_export.h"
 #include "components/sessions/core/tab_restore_service.h"
@@ -34,7 +35,8 @@ class SESSIONS_EXPORT TabRestoreServiceImpl : public TabRestoreService {
   // TabRestoreService:
   void AddObserver(TabRestoreServiceObserver* observer) override;
   void RemoveObserver(TabRestoreServiceObserver* observer) override;
-  void CreateHistoricalTab(LiveTab* live_tab, int index) override;
+  base::Optional<SessionID> CreateHistoricalTab(LiveTab* live_tab,
+                                                int index) override;
   void BrowserClosing(LiveTabContext* context) override;
   void BrowserClosed(LiveTabContext* context) override;
   void ClearEntries() override;

ios/chrome/browser/sessions/live_tab_context_browser_agent.h

@@ -58,8 +58,8 @@ class LiveTabContextBrowserAgent
       bool pin,
       bool from_last_session,
       const sessions::PlatformSpecificTabData* tab_platform_data,
-      const sessions::SerializedUserAgentOverride& user_agent_override)
-      override;
+      const sessions::SerializedUserAgentOverride& user_agent_override,
+      const SessionID* tab_id) override;
   sessions::LiveTab* ReplaceRestoredTab(
       const std::vector<sessions::SerializedNavigationEntry>& navigations,
       base::Optional<tab_groups::TabGroupId> group,

ios/chrome/browser/sessions/live_tab_context_browser_agent.mm

@@ -117,7 +117,8 @@ sessions::LiveTab* LiveTabContextBrowserAgent::AddRestoredTab(
     bool pin,
     bool from_last_session,
     const sessions::PlatformSpecificTabData* tab_platform_data,
-    const sessions::SerializedUserAgentOverride& user_agent_override) {
+    const sessions::SerializedUserAgentOverride& user_agent_override,
+    const SessionID* tab_id) {
   // TODO(crbug.com/661636): Handle tab-switch animation somehow...
   web_state_list_->InsertWebState(
       tab_index,

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_mediator_unittest.mm

@@ -8,8 +8,10 @@
 #include <memory>
 
 #include "base/mac/foundation_util.h"
+#include "base/optional.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/sessions/core/live_tab.h"
+#include "components/sessions/core/session_id.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/sessions/core/tab_restore_service_helper.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
@@ -65,7 +67,8 @@ class FakeTabRestoreService : public sessions::TabRestoreService {
     NOTREACHED();
   }
 
-  void CreateHistoricalTab(sessions::LiveTab* live_tab, int index) override {
+  base::Optional<SessionID> CreateHistoricalTab(sessions::LiveTab* live_tab,
+                                                int index) override {
     auto tab = std::make_unique<Tab>();
     int entry_count =
         live_tab->IsInitialBlankNavigation() ? 0 : live_tab->GetEntryCount();
@@ -75,6 +78,7 @@ class FakeTabRestoreService : public sessions::TabRestoreService {
       tab->navigations[i] = entry;
     }
     entries_.push_front(std::move(tab));
+    return base::nullopt;
   }
 
   void BrowserClosing(sessions::LiveTabContext* context) override {