~Browser: Don't create TabRestoreService on exit.

Interaction between http://codereview.chromium.org/11377 and
http://codereview.chromium.org/6901031 caused TabRestoreService getting created on exit.

It was easy to break TabRestoreService, e.g., like this:
http://codereview.chromium.org/8769013 (Adding a call to SessionBackend::Init()
to BaseSessionService ctor.) This broke the "Recently closed" menu: crbug.com/110785.

After this CL, TabRestoreService is no longer created at exit and it is
hopefully more difficult to break. This also removes the workaround added by
https://codereview.chromium.org/9361056.

BUG=NONE
TEST=crbug.com/110785 no longer reproduces even if the fix is removed (see steps in the bug).


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

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

4 files changed, 19 insertions, 34 deletions

diff --git a/chrome/browser/sessions/base_session_service.cc b/chrome/browser/sessions/base_session_service.cc
index 421c464..f391211 100644
--- a/chrome/browser/sessions/base_session_service.cc
+++ b/chrome/browser/sessions/base_session_service.cc
@@ -89,15 +89,8 @@ BaseSessionService::BaseSessionService(SessionType type,
       profile_ ? profile_->GetPath() : path_);
   DCHECK(backend_.get());
 
-  if (!RunningInProduction()) {
-    // We seem to be running as part of a test, in which case we need
-    // to explicitly initialize the backend.  In production, the
-    // backend will automatically initialize itself just in time.
-    //
-    // Note that it's important not to initialize too early in
-    // production; this can cause e.g. http://crbug.com/110785.
-    backend_->Init();
-  }
+  RunTaskOnBackendThread(FROM_HERE,
+                         base::Bind(&SessionBackend::Init, backend_));
 }
 
 BaseSessionService::~BaseSessionService() {
@@ -325,14 +318,10 @@ BaseSessionService::Handle BaseSessionService::ScheduleGetLastSessionCommands(
   return request->handle();
 }
 
-bool BaseSessionService::RunningInProduction() const {
-  return profile_ && BrowserThread::IsMessageLoopValid(BrowserThread::FILE);
-}
-
 bool BaseSessionService::RunTaskOnBackendThread(
     const tracked_objects::Location& from_here,
     const base::Closure& task) {
-  if (RunningInProduction()) {
+  if (profile_ && BrowserThread::IsMessageLoopValid(BrowserThread::FILE)) {
     return BrowserThread::PostTask(BrowserThread::FILE, from_here, task);
   } else {
     // Fall back to executing on the main thread if the file thread
diff --git a/chrome/browser/sessions/base_session_service.h b/chrome/browser/sessions/base_session_service.h
index 21e848e..50b922a 100644
--- a/chrome/browser/sessions/base_session_service.h
+++ b/chrome/browser/sessions/base_session_service.h
@@ -162,11 +162,6 @@ class BaseSessionService : public CancelableRequestProvider,
       InternalGetCommandsRequest* request,
       CancelableRequestConsumerBase* consumer);
 
-  // Returns true if we appear to be running in production, false if
-  // we appear to be running as part of a unit test or if the FILE
-  // thread has gone away.
-  bool RunningInProduction() const;
-
   // In production, this posts the task to the FILE thread.  For
   // tests, it immediately runs the specified task on the current
   // thread.
diff --git a/chrome/browser/sessions/session_service_factory.h b/chrome/browser/sessions/session_service_factory.h
index ed7b14b..15d3860 100644
--- a/chrome/browser/sessions/session_service_factory.h
+++ b/chrome/browser/sessions/session_service_factory.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -21,13 +21,14 @@ class SessionServiceFactory : public ProfileKeyedServiceFactory {
   // 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.
+  // This returns NULL if the profile is incognito. 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.
+  // doesn't exist. This returns NULL if the profile is incognito or if session
+  // service has been explicitly shutdown (browser is exiting). Callers should
+  // always check the return value for NULL.
   static SessionService* GetForProfileIfExisting(Profile* profile);
 
   // If |profile| has a session service, it is shut down. To properly record the
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 5199768..3aa426d 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -446,6 +446,16 @@ Browser::~Browser() {
 
   BrowserList::RemoveBrowser(this);
 
+  SessionService* session_service =
+      SessionServiceFactory::GetForProfile(profile_);
+  if (session_service)
+    session_service->WindowClosed(session_id_);
+
+  TabRestoreService* tab_restore_service =
+      TabRestoreServiceFactory::GetForProfile(profile());
+  if (tab_restore_service)
+    tab_restore_service->BrowserClosed(tab_restore_service_delegate());
+
 #if !defined(OS_MACOSX)
   if (!BrowserList::HasBrowserWithProfile(profile_)) {
     // We're the last browser window with this profile. We need to nuke the
@@ -461,16 +471,6 @@ Browser::~Browser() {
   }
 #endif
 
-  SessionService* session_service =
-      SessionServiceFactory::GetForProfile(profile_);
-  if (session_service)
-    session_service->WindowClosed(session_id_);
-
-  TabRestoreService* tab_restore_service =
-      TabRestoreServiceFactory::GetForProfile(profile());
-  if (tab_restore_service)
-    tab_restore_service->BrowserClosed(tab_restore_service_delegate());
-
   profile_pref_registrar_.RemoveAll();
   local_pref_registrar_.RemoveAll();