summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormkolom <mkolom@yandex-team.ru>2016-09-21 08:10:31 (GMT)
committerCommit bot <commit-bot@chromium.org>2016-09-21 08:12:24 (GMT)
commitd19beb53ae69324f842ba8efdbdd0967034f38c4 (patch)
treeef714a446b2c408633d6f9c31a9efd19ffe4e9d9
parent30e0c31970dd7d31877f455d8cdb65c8e35f880f (diff)
downloadchromium-d19beb53ae69324f842ba8efdbdd0967034f38c4.tar.gz
chromium-d19beb53ae69324f842ba8efdbdd0967034f38c4.tar.xz
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}
-rw-r--r--chrome/browser/sessions/session_service.cc7
-rw-r--r--chrome/browser/sessions/tab_restore_browsertest.cc36
-rw-r--r--components/sessions/core/in_memory_tab_restore_service.cc4
-rw-r--r--components/sessions/core/in_memory_tab_restore_service.h1
-rw-r--r--components/sessions/core/persistent_tab_restore_service.cc4
-rw-r--r--components/sessions/core/persistent_tab_restore_service.h1
-rw-r--r--components/sessions/core/tab_restore_service.h3
-rw-r--r--components/sessions/core/tab_restore_service_helper.cc4
-rw-r--r--components/sessions/core/tab_restore_service_helper.h1
9 files changed, 60 insertions, 1 deletions
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index 805e03e..3bbb27d 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/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,
diff --git a/chrome/browser/sessions/tab_restore_browsertest.cc b/chrome/browser/sessions/tab_restore_browsertest.cc
index 91c8c8c..5c383f9 100644
--- a/chrome/browser/sessions/tab_restore_browsertest.cc
+++ b/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());
+}
diff --git a/components/sessions/core/in_memory_tab_restore_service.cc b/components/sessions/core/in_memory_tab_restore_service.cc
index 902d008..cdc31f1 100644
--- a/components/sessions/core/in_memory_tab_restore_service.cc
+++ b/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() {
}
diff --git a/components/sessions/core/in_memory_tab_restore_service.h b/components/sessions/core/in_memory_tab_restore_service.h
index 784d6c6..c6174c3 100644
--- a/components/sessions/core/in_memory_tab_restore_service.h
+++ b/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:
diff --git a/components/sessions/core/persistent_tab_restore_service.cc b/components/sessions/core/persistent_tab_restore_service.cc
index 7548edb..f0689a4 100644
--- a/components/sessions/core/persistent_tab_restore_service.cc
+++ b/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();
}
diff --git a/components/sessions/core/persistent_tab_restore_service.h b/components/sessions/core/persistent_tab_restore_service.h
index 5e2a165..9d2eaa8 100644
--- a/components/sessions/core/persistent_tab_restore_service.h
+++ b/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:
diff --git a/components/sessions/core/tab_restore_service.h b/components/sessions/core/tab_restore_service.h
index 8f68b9f..a851f81 100644
--- a/components/sessions/core/tab_restore_service.h
+++ b/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
diff --git a/components/sessions/core/tab_restore_service_helper.cc b/components/sessions/core/tab_restore_service_helper.cc
index 1390ba1..05094d2 100644
--- a/components/sessions/core/tab_restore_service_helper.cc
+++ b/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_));
diff --git a/components/sessions/core/tab_restore_service_helper.h b/components/sessions/core/tab_restore_service_helper.h
index b6c3a47..3568e55 100644
--- a/components/sessions/core/tab_restore_service_helper.h
+++ b/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();