Fix tabs duplication when restoring last closed window.

Restoration of a last closed window is done by TabRestoreService.
However if that window is going to be the first window for a profile,
SessionService will perform restoration and there will be conflict
between services. It leads to tabs duplication.

BUG=500083

R=sky@chromium.org

Review-Url: https://codereview.chromium.org/2345763002
Cr-Commit-Position: refs/heads/master@{#420006}

chrome/browser/sessions/session_service.cc

@@ -34,6 +34,7 @@
 #include "chrome/browser/sessions/session_restore.h"
 #include "chrome/browser/sessions/session_service_utils.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -43,6 +44,7 @@
 #include "components/sessions/core/session_command.h"
 #include "components/sessions/core/session_constants.h"
 #include "components/sessions/core/session_types.h"
+#include "components/sessions/core/tab_restore_service.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
@@ -588,7 +590,10 @@ bool SessionService::RestoreIfNecessary(const std::vector<GURL>& urls_to_open,
     }
     SessionStartupPref pref = StartupBrowserCreator::GetSessionStartupPref(
         *base::CommandLine::ForCurrentProcess(), profile());
-    if (pref.type == SessionStartupPref::LAST) {
+    sessions::TabRestoreService* tab_restore_service =
+        TabRestoreServiceFactory::GetForProfileIfExisting(profile());
+    if (pref.type == SessionStartupPref::LAST &&
+        (!tab_restore_service || !tab_restore_service->IsRestoring())) {
       SessionRestore::RestoreSession(
           profile(), browser,
           browser ? 0 : SessionRestore::ALWAYS_CREATE_TABBED_BROWSER,

chrome/browser/sessions/tab_restore_browsertest.cc

@@ -13,6 +13,10 @@
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/lifetime/keep_alive_types.h"
+#include "chrome/browser/lifetime/scoped_keep_alive.h"
+#include "chrome/browser/prefs/session_startup_pref.h"
+#include "chrome/browser/sessions/session_restore_test_helper.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -716,3 +720,35 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreOnStartup) {
   EXPECT_EQ(url1_,
             browser()->tab_strip_model()->GetWebContentsAt(1)->GetURL());
 }
+
+// Check that TabRestoreService and SessionService do not try to restore the
+// same thing.
+IN_PROC_BROWSER_TEST_F(TabRestoreTest,
+                       RestoreFirstBrowserWhenSessionServiceEnabled) {
+  // Do not exit from test when last browser is closed.
+  ScopedKeepAlive keep_alive(KeepAliveOrigin::SESSION_RESTORE,
+                             KeepAliveRestartOption::DISABLED);
+
+  // Enable session service.
+  SessionStartupPref pref(SessionStartupPref::LAST);
+  Profile* profile = browser()->profile();
+  SessionStartupPref::SetStartupPref(profile, pref);
+
+  // Add tabs and close browser.
+  AddSomeTabs(browser(), 3);
+  // 1st tab is about:blank added by InProcessBrowserTest.
+  EXPECT_EQ(4, browser()->tab_strip_model()->count());
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED,
+      content::NotificationService::AllSources());
+  chrome::CloseWindow(browser());
+  observer.Wait();
+
+  SessionRestoreTestHelper helper;
+  // Restore browser (this is what Cmd-Shift-T does on Mac).
+  chrome::OpenWindowWithRestoredTabs(profile);
+  if (SessionRestore::IsRestoring(profile))
+    helper.Wait();
+  Browser* browser = GetBrowser(0);
+  EXPECT_EQ(4, browser->tab_strip_model()->count());
+}

components/sessions/core/in_memory_tab_restore_service.cc

@@ -81,6 +81,10 @@ void InMemoryTabRestoreService::DeleteLastSession() {
   // See comment above.
 }
 
+bool InMemoryTabRestoreService::IsRestoring() const {
+  return helper_.IsRestoring();
+}
+
 void InMemoryTabRestoreService::Shutdown() {
 }
 

components/sessions/core/in_memory_tab_restore_service.h

@@ -50,6 +50,7 @@ class SESSIONS_EXPORT InMemoryTabRestoreService : public TabRestoreService {
   void LoadTabsFromLastSession() override;
   bool IsLoaded() const override;
   void DeleteLastSession() override;
+  bool IsRestoring() const override;
   void Shutdown() override;
 
  private:

components/sessions/core/persistent_tab_restore_service.cc

@@ -950,6 +950,10 @@ void PersistentTabRestoreService::DeleteLastSession() {
   return delegate_->DeleteLastSession();
 }
 
+bool PersistentTabRestoreService::IsRestoring() const {
+  return helper_.IsRestoring();
+}
+
 void PersistentTabRestoreService::Shutdown() {
   return delegate_->Shutdown();
 }

components/sessions/core/persistent_tab_restore_service.h

@@ -46,6 +46,7 @@ class SESSIONS_EXPORT PersistentTabRestoreService : public TabRestoreService {
   void LoadTabsFromLastSession() override;
   bool IsLoaded() const override;
   void DeleteLastSession() override;
+  bool IsRestoring() const override;
   void Shutdown() override;
 
  private:

components/sessions/core/tab_restore_service.h

@@ -182,6 +182,9 @@ class SESSIONS_EXPORT TabRestoreService : public KeyedService {
 
   // Deletes the last session.
   virtual void DeleteLastSession() = 0;
+
+  // Returns true if we're in the process of restoring some entries.
+  virtual bool IsRestoring() const = 0;
 };
 
 // A class that is used to associate platform-specific data with

components/sessions/core/tab_restore_service_helper.cc

@@ -260,6 +260,10 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
   return live_tabs;
 }
 
+bool TabRestoreServiceHelper::IsRestoring() const {
+  return restoring_;
+}
+
 void TabRestoreServiceHelper::NotifyTabsChanged() {
   FOR_EACH_OBSERVER(TabRestoreServiceObserver, observer_list_,
                     TabRestoreServiceChanged(tab_restore_service_));

components/sessions/core/tab_restore_service_helper.h

@@ -82,6 +82,7 @@ class SESSIONS_EXPORT TabRestoreServiceHelper {
   std::vector<LiveTab*> RestoreEntryById(LiveTabContext* context,
                                          SessionID::id_type id,
                                          WindowOpenDisposition disposition);
+  bool IsRestoring() const;
 
   // Notifies observers the tabs have changed.
   void NotifyTabsChanged();