Profile shouldn't own Session/TabRestore services.

BaseSessionService is now a ProfileKeyedService. Its subclasses,
SessionService and TabRestoreService, are now accessed through
factories.

BUG=77155
TEST=existing tests


Review URL: http://codereview.chromium.org/6901031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83325 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 226 insertions, 17 deletions

diff --git a/chrome/browser/sessions/base_session_service.h b/chrome/browser/sessions/base_session_service.h
index 062461f..16eff04 100644
--- a/chrome/browser/sessions/base_session_service.h
+++ b/chrome/browser/sessions/base_session_service.h
@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
+#include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "content/browser/cancelable_request.h"
 
@@ -27,7 +28,8 @@ class Thread;
 // session service. It contains commonality needed by both, in particular
 // it manages a set of SessionCommands that are periodically sent to a
 // SessionBackend.
-class BaseSessionService : public CancelableRequestProvider {
+class BaseSessionService : public CancelableRequestProvider,
+                           public ProfileKeyedService {
  public:
   // Identifies the type of session service this is. This is used by the
   // backend to determine the name of the files.
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 1b3b5f7..4b06737 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_service.h"
+#include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
@@ -422,7 +423,8 @@ class SessionRestoreImpl : public NotificationObserver {
   }
 
   Browser* Restore() {
-    SessionService* session_service = profile_->GetSessionService();
+    SessionService* session_service =
+        SessionServiceFactory::GetForProfile(profile_);
     DCHECK(session_service);
     SessionService::SessionCallback* callback =
         NewCallback(this, &SessionRestoreImpl::OnGotSession);
@@ -727,7 +729,8 @@ class SessionRestoreImpl : public NotificationObserver {
   // Invokes TabRestored on the SessionService for all tabs in browser after
   // initial_count.
   void NotifySessionServiceOfRestoredTabs(Browser* browser, int initial_count) {
-    SessionService* session_service = profile_->GetSessionService();
+    SessionService* session_service =
+        SessionServiceFactory::GetForProfile(profile_);
     for (int i = initial_count; i < browser->tab_count(); ++i)
       session_service->TabRestored(&browser->GetTabContentsAt(i)->controller(),
                                    browser->tabstrip_model()->IsTabPinned(i));
@@ -789,7 +792,7 @@ static Browser* Restore(Profile* profile,
   // Always restore from the original profile (incognito profiles have no
   // session service).
   profile = profile->GetOriginalProfile();
-  if (!profile->GetSessionService()) {
+  if (!SessionServiceFactory::GetForProfile(profile)) {
     NOTREACHED();
     return NULL;
   }
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index b8279cf..54d9fb4 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -6,6 +6,7 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -126,7 +127,8 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreIndividualTabFromWindow) {
   browser()->AddSelectedTabWithURL(url3, PageTransition::LINK);
   ui_test_utils::WaitForNavigationInCurrentTab(browser());
 
-  TabRestoreService* service = browser()->profile()->GetTabRestoreService();
+  TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(browser()->profile());
   service->ClearEntries();
 
   browser()->window()->Close();
@@ -165,7 +167,8 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, WindowWithOneTab) {
   // Add a single tab.
   ui_test_utils::NavigateToURL(browser(), url);
 
-  TabRestoreService* service = browser()->profile()->GetTabRestoreService();
+  TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(browser()->profile());
   service->ClearEntries();
   EXPECT_EQ(0U, service->entries().size());
 
diff --git a/chrome/browser/sessions/session_service_factory.cc b/chrome/browser/sessions/session_service_factory.cc
new file mode 100644
index 0000000..3a93dfb
--- /dev/null
+++ b/chrome/browser/sessions/session_service_factory.cc
@@ -0,0 +1,56 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sessions/session_service_factory.h"
+
+#include "chrome/browser/profiles/profile_dependency_manager.h"
+#include "chrome/browser/sessions/session_service.h"
+
+// static
+SessionService* SessionServiceFactory::GetForProfile(Profile* profile) {
+  return static_cast<SessionService*>(
+      GetInstance()->GetServiceForProfile(profile, true));
+}
+
+// static
+SessionService* SessionServiceFactory::GetForProfileIfExisting(
+    Profile* profile) {
+  return static_cast<SessionService*>(
+      GetInstance()->GetServiceForProfile(profile, false));
+}
+
+// static
+void SessionServiceFactory::ShutdownForProfile(Profile* profile) {
+  // We're about to exit, force creation of the session service if it hasn't
+  // been created yet. We do this to ensure session state matches the point in
+  // time the user exited.
+  SessionServiceFactory* factory = GetInstance();
+  factory->GetServiceForProfile(profile, true);
+
+  // Shut down and remove the reference to the session service, and replace it
+  // with an explicit NULL to prevent it being recreated on the next access.
+  factory->ProfileShutdown(profile);
+  factory->ProfileDestroyed(profile);
+  factory->Associate(profile, NULL);
+}
+
+SessionServiceFactory* SessionServiceFactory::GetInstance() {
+  return Singleton<SessionServiceFactory>::get();
+}
+
+SessionServiceFactory::SessionServiceFactory()
+    : ProfileKeyedServiceFactory(
+        ProfileDependencyManager::GetInstance()) {
+}
+
+SessionServiceFactory::~SessionServiceFactory() {
+}
+
+ProfileKeyedService* SessionServiceFactory::BuildServiceInstanceFor(
+    Profile* profile) const {
+  SessionService* service = NULL;
+  service = new SessionService(profile);
+  service->ResetFromCurrentBrowsers();
+  return service;
+}
diff --git a/chrome/browser/sessions/session_service_factory.h b/chrome/browser/sessions/session_service_factory.h
new file mode 100644
index 0000000..213d2b6
--- /dev/null
+++ b/chrome/browser/sessions/session_service_factory.h
@@ -0,0 +1,60 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SESSIONS_SESSION_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_SESSIONS_SESSION_SERVICE_FACTORY_H_
+
+#include "base/memory/singleton.h"
+#include "chrome/browser/profiles/profile_keyed_service_factory.h"
+#include "chrome/browser/sessions/session_service.h"
+
+class Profile;
+
+// Singleton that owns all SessionServices and associates them with
+// Profiles. Listens for the Profile's destruction notification and cleans up
+// the associated SessionService.
+class SessionServiceFactory : public ProfileKeyedServiceFactory {
+ public:
+  // Returns the session service for |profile|. This may return NULL. If this
+  // profile supports a session service (it isn't incognito), and the session
+  // service hasn't yet been created, this forces creation of the session
+  // service.
+  //
+  // This returns NULL in two situations: the profile is incognito, or the
+  // session service has been explicitly shutdown (browser is exiting). Callers
+  // should always check the return value for NULL.
+  static SessionService* GetForProfile(Profile* profile);
+
+  // Returns the session service for |profile|, but do not create it if it
+  // doesn't exist.
+  static SessionService* GetForProfileIfExisting(Profile* profile);
+
+  // If |profile| has a session service, it is shut down. To properly record the
+  // current state this forces creation of the session service, then shuts it
+  // down.
+  static void ShutdownForProfile(Profile* profile);
+
+#if defined(UNIT_TEST)
+  // For test use: force setting of the session service for a given profile.
+  // This will delete a previous session service for this profile if it exists.
+  static void SetForTestProfile(Profile* profile, SessionService* service) {
+    GetInstance()->ProfileShutdown(profile);
+    GetInstance()->ProfileDestroyed(profile);
+    GetInstance()->Associate(profile, service);
+  }
+#endif
+
+  static SessionServiceFactory* GetInstance();
+
+ private:
+  friend struct DefaultSingletonTraits<SessionServiceFactory>;
+
+  SessionServiceFactory();
+  virtual ~SessionServiceFactory();
+
+  // ProfileKeyedServiceFactory:
+  virtual ProfileKeyedService* BuildServiceInstanceFor(Profile* profile) const;
+};
+
+#endif  // CHROME_BROWSER_SESSIONS_SESSION_SERVICE_FACTORY_H_
diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index 1e81bb1..4076dee 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/extensions/extension_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_service.h"
+#include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/session_command.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/sessions/tab_restore_service_delegate.h"
@@ -420,13 +421,14 @@ void TabRestoreService::LoadTabsFromLastSession() {
 
   load_state_ = LOADING;
 
+  SessionService* session_service =
+      SessionServiceFactory::GetForProfile(profile());
   if (!profile()->restored_last_session() &&
       !profile()->DidLastSessionExitCleanly() &&
-      profile()->GetSessionService()) {
+      session_service) {
     // The previous session crashed and wasn't restored. Load the tabs/windows
     // that were open at the point of crash from the session service.
-    profile()->GetSessionService()->GetLastSession(
-        &load_consumer_,
+    session_service->GetLastSession(&load_consumer_,
         NewCallback(this, &TabRestoreService::OnGotPreviousSession));
   } else {
     load_state_ |= LOADED_LAST_SESSION;
diff --git a/chrome/browser/sessions/tab_restore_service_browsertest.cc b/chrome/browser/sessions/tab_restore_service_browsertest.cc
index 28aa062..d1af01b 100644
--- a/chrome/browser/sessions/tab_restore_service_browsertest.cc
+++ b/chrome/browser/sessions/tab_restore_service_browsertest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/sessions/session_service.h"
+#include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/test/render_view_test.h"
 #include "chrome/test/testing_profile.h"
@@ -82,7 +83,8 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
   // Adds a window with one tab and url to the profile's session service.
   // If |pinned| is true, the tab is marked as pinned in the session service.
   void AddWindowWithOneTabToSessionService(bool pinned) {
-    SessionService* session_service = profile()->GetSessionService();
+    SessionService* session_service =
+        SessionServiceFactory::GetForProfile(profile());
     SessionID tab_id;
     SessionID window_id;
     session_service->SetWindowType(window_id, Browser::TYPE_NORMAL);
@@ -103,7 +105,7 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
   void CreateSessionServiceWithOneWindow(bool pinned) {
     // The profile takes ownership of this.
     SessionService* session_service = new SessionService(profile());
-    profile()->set_session_service(session_service);
+    SessionServiceFactory::SetForTestProfile(profile(), session_service);
 
     AddWindowWithOneTabToSessionService(pinned);
 
@@ -287,7 +289,8 @@ TEST_F(TabRestoreServiceTest, DontLoadTwice) {
 TEST_F(TabRestoreServiceTest, LoadPreviousSession) {
   CreateSessionServiceWithOneWindow(false);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   service_->LoadTabsFromLastSession();
 
@@ -310,7 +313,8 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSession) {
 TEST_F(TabRestoreServiceTest, DontLoadAfterRestore) {
   CreateSessionServiceWithOneWindow(false);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   profile()->set_restored_last_session(true);
 
@@ -324,7 +328,8 @@ TEST_F(TabRestoreServiceTest, DontLoadAfterRestore) {
 TEST_F(TabRestoreServiceTest, DontLoadAfterCleanExit) {
   CreateSessionServiceWithOneWindow(false);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   profile()->set_last_session_exited_cleanly(true);
 
@@ -336,7 +341,8 @@ TEST_F(TabRestoreServiceTest, DontLoadAfterCleanExit) {
 TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) {
   CreateSessionServiceWithOneWindow(false);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   AddThreeNavigations();
 
@@ -378,7 +384,8 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) {
 TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabsPinned) {
   CreateSessionServiceWithOneWindow(true);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   AddThreeNavigations();
 
@@ -421,7 +428,8 @@ TEST_F(TabRestoreServiceTest, ManyWindowsInSessionService) {
   for (size_t i = 0; i < TabRestoreService::kMaxEntries; ++i)
     AddWindowWithOneTabToSessionService(false);
 
-  profile()->GetSessionService()->MoveCurrentSessionToLastSession();
+  SessionServiceFactory::GetForProfile(profile())->
+      MoveCurrentSessionToLastSession();
 
   AddThreeNavigations();
 
diff --git a/chrome/browser/sessions/tab_restore_service_factory.cc b/chrome/browser/sessions/tab_restore_service_factory.cc
new file mode 100644
index 0000000..73f793f
--- /dev/null
+++ b/chrome/browser/sessions/tab_restore_service_factory.cc
@@ -0,0 +1,40 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
+
+#include "chrome/browser/profiles/profile_dependency_manager.h"
+#include "chrome/browser/sessions/tab_restore_service.h"
+
+// static
+TabRestoreService* TabRestoreServiceFactory::GetForProfile(Profile* profile) {
+  return static_cast<TabRestoreService*>(
+      GetInstance()->GetServiceForProfile(profile, true));
+}
+
+// static
+void TabRestoreServiceFactory::ResetForProfile(Profile* profile) {
+  TabRestoreServiceFactory* factory = GetInstance();
+  factory->ProfileShutdown(profile);
+  factory->ProfileDestroyed(profile);
+}
+
+TabRestoreServiceFactory* TabRestoreServiceFactory::GetInstance() {
+  return Singleton<TabRestoreServiceFactory>::get();
+}
+
+TabRestoreServiceFactory::TabRestoreServiceFactory()
+    : ProfileKeyedServiceFactory(
+        ProfileDependencyManager::GetInstance()) {
+}
+
+TabRestoreServiceFactory::~TabRestoreServiceFactory() {
+}
+
+ProfileKeyedService* TabRestoreServiceFactory::BuildServiceInstanceFor(
+    Profile* profile) const {
+  TabRestoreService* service = NULL;
+  service = new TabRestoreService(profile);
+  return service;
+}
diff --git a/chrome/browser/sessions/tab_restore_service_factory.h b/chrome/browser/sessions/tab_restore_service_factory.h
new file mode 100644
index 0000000..d20720f9
--- /dev/null
+++ b/chrome/browser/sessions/tab_restore_service_factory.h
@@ -0,0 +1,35 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SESSIONS_TAB_RESTORE_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_SESSIONS_TAB_RESTORE_SERVICE_FACTORY_H_
+
+#include "base/memory/singleton.h"
+#include "chrome/browser/profiles/profile_keyed_service_factory.h"
+
+class TabRestoreService;
+class Profile;
+
+// Singleton that owns all TabRestoreServices and associates them with
+// Profiles. Listens for the Profile's destruction notification and cleans up
+// the associated TabRestoreService.
+class TabRestoreServiceFactory : public ProfileKeyedServiceFactory {
+ public:
+  static TabRestoreService* GetForProfile(Profile* profile);
+
+  static void ResetForProfile(Profile* profile);
+
+  static TabRestoreServiceFactory* GetInstance();
+
+ private:
+  friend struct DefaultSingletonTraits<TabRestoreServiceFactory>;
+
+  TabRestoreServiceFactory();
+  virtual ~TabRestoreServiceFactory();
+
+  // ProfileKeyedServiceFactory:
+  virtual ProfileKeyedService* BuildServiceInstanceFor(Profile* profile) const;
+};
+
+#endif  // CHROME_BROWSER_SESSIONS_TAB_RESTORE_SERVICE_FACTORY_H_