Convert SessionService to use new CancelableTaskTracker

BUG=155883


Review URL: https://chromiumcodereview.appspot.com/11446033

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

8 files changed, 101 insertions, 160 deletions

diff --git a/chrome/browser/sessions/base_session_service.cc b/chrome/browser/sessions/base_session_service.cc
index a30981e..a05d30b 100644
--- a/chrome/browser/sessions/base_session_service.cc
+++ b/chrome/browser/sessions/base_session_service.cc
@@ -22,17 +22,6 @@
 using content::BrowserThread;
 using content::NavigationEntry;
 
-// InternalGetCommandsRequest -------------------------------------------------
-
-BaseSessionService::InternalGetCommandsRequest::InternalGetCommandsRequest(
-    const CallbackType& callback)
-    : CancelableRequest<InternalGetCommandsCallback>(callback) {
-}
-
-BaseSessionService::InternalGetCommandsRequest::~InternalGetCommandsRequest() {
-  STLDeleteElements(&commands);
-}
-
 // BaseSessionService ---------------------------------------------------------
 
 namespace {
@@ -51,6 +40,24 @@ void WriteStringToPickle(Pickle& pickle, int* bytes_written, int max_bytes,
   }
 }
 
+// Helper used by ScheduleGetLastSessionCommands. It runs callback on TaskRunner
+// thread if it's not canceled.
+void RunIfNotCanceled(
+    const CancelableTaskTracker::IsCanceledCallback& is_canceled,
+    base::TaskRunner* task_runner,
+    const BaseSessionService::InternalGetCommandsCallback& callback,
+    ScopedVector<SessionCommand> commands) {
+  if (is_canceled.Run())
+    return;
+
+  if (task_runner->RunsTasksOnCurrentThread()) {
+    callback.Run(commands.Pass());
+  } else {
+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(callback, base::Passed(&commands)));
+  }
+}
+
 }  // namespace
 
 // Delay between when a command is received, and when we save it to the
@@ -259,16 +266,22 @@ bool BaseSessionService::ShouldTrackEntry(const GURL& url) {
   return url.is_valid();
 }
 
-BaseSessionService::Handle BaseSessionService::ScheduleGetLastSessionCommands(
-    InternalGetCommandsRequest* request,
-    CancelableRequestConsumerBase* consumer) {
-  scoped_refptr<InternalGetCommandsRequest> request_wrapper(request);
-  AddRequest(request, consumer);
+CancelableTaskTracker::TaskId
+    BaseSessionService::ScheduleGetLastSessionCommands(
+    const InternalGetCommandsCallback& callback,
+    CancelableTaskTracker* tracker) {
+  CancelableTaskTracker::IsCanceledCallback is_canceled;
+  CancelableTaskTracker::TaskId id = tracker->NewTrackedTaskId(&is_canceled);
+
+  InternalGetCommandsCallback callback_runner =
+      base::Bind(&RunIfNotCanceled,
+                 is_canceled, base::MessageLoopProxy::current(), callback);
+
   RunTaskOnBackendThread(
       FROM_HERE,
       base::Bind(&SessionBackend::ReadLastSessionCommands, backend(),
-                 request_wrapper));
-  return request->handle();
+                 is_canceled, callback_runner));
+  return id;
 }
 
 bool BaseSessionService::RunTaskOnBackendThread(
diff --git a/chrome/browser/sessions/base_session_service.h b/chrome/browser/sessions/base_session_service.h
index 529f9b5..9e1a0d7 100644
--- a/chrome/browser/sessions/base_session_service.h
+++ b/chrome/browser/sessions/base_session_service.h
@@ -11,9 +11,11 @@
 #include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/common/cancelable_request.h"
 #include "chrome/browser/sessions/session_id.h"
+#include "chrome/common/cancelable_task_tracker.h"
 #include "googleurl/src/gurl.h"
 
 class Profile;
@@ -48,29 +50,9 @@ class BaseSessionService : public CancelableRequestProvider {
   // Deletes the last session.
   void DeleteLastSession();
 
-  class InternalGetCommandsRequest;
-
-  typedef base::Callback<void(Handle,
-                              scoped_refptr<InternalGetCommandsRequest>)>
+  typedef base::Callback<void(ScopedVector<SessionCommand>)>
       InternalGetCommandsCallback;
 
-  // Callback used when fetching the last session. The last session consists
-  // of a vector of SessionCommands.
-  class InternalGetCommandsRequest :
-      public CancelableRequest<InternalGetCommandsCallback> {
-   public:
-    explicit InternalGetCommandsRequest(const CallbackType& callback);
-
-    // The commands. The backend fills this in for us.
-    std::vector<SessionCommand*> commands;
-
-   protected:
-    virtual ~InternalGetCommandsRequest();
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(InternalGetCommandsRequest);
-  };
-
  protected:
   virtual ~BaseSessionService();
 
@@ -160,11 +142,12 @@ class BaseSessionService : public CancelableRequestProvider {
   // Returns true if the entry at specified |url| should be written to disk.
   bool ShouldTrackEntry(const GURL& url);
 
-  // Invokes ReadLastSessionCommands with request on the backend thread.
-  // If testing, ReadLastSessionCommands is invoked directly.
-  Handle ScheduleGetLastSessionCommands(
-      InternalGetCommandsRequest* request,
-      CancelableRequestConsumerBase* consumer);
+  // Invokes SessionBackend::ReadLastSessionCommands with callback on the
+  // backend thread.
+  // If testing, SessionBackend::ReadLastSessionCommands is invoked directly.
+  CancelableTaskTracker::TaskId ScheduleGetLastSessionCommands(
+      const InternalGetCommandsCallback& callback,
+      CancelableTaskTracker* tracker);
 
   // In production, this posts the task to the FILE thread.  For
   // tests, it immediately runs the specified task on the current
diff --git a/chrome/browser/sessions/persistent_tab_restore_service.cc b/chrome/browser/sessions/persistent_tab_restore_service.cc
index 3579647..3ddc168 100644
--- a/chrome/browser/sessions/persistent_tab_restore_service.cc
+++ b/chrome/browser/sessions/persistent_tab_restore_service.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
+#include "chrome/common/cancelable_task_tracker.h"
 #include "content/public/browser/session_storage_namespace.h"
 
 namespace {
@@ -178,14 +179,11 @@ class PersistentTabRestoreService::Delegate
   // Invoked when we've loaded the session commands that identify the previously
   // closed tabs. This creates entries, adds them to staging_entries_, and
   // invokes LoadState.
-  void OnGotLastSessionCommands(
-      Handle handle,
-      scoped_refptr<InternalGetCommandsRequest> request);
+  void OnGotLastSessionCommands(ScopedVector<SessionCommand> commands);
 
-  // Populates |loaded_entries| with Entries from |request|.
-  void CreateEntriesFromCommands(
-      scoped_refptr<InternalGetCommandsRequest> request,
-      std::vector<Entry*>* loaded_entries);
+  // Populates |loaded_entries| with Entries from |commands|.
+  void CreateEntriesFromCommands(const std::vector<SessionCommand*>& commands,
+                                 std::vector<Entry*>* loaded_entries);
 
   // Validates all entries in |entries|, deleting any with no navigations. This
   // also deletes any entries beyond the max number of entries we can hold.
@@ -195,8 +193,7 @@ class PersistentTabRestoreService::Delegate
   // previous session. This creates and add entries to |staging_entries_| and
   // invokes LoadStateChanged. |ignored_active_window| is ignored because we
   // don't need to restore activation.
-  void OnGotPreviousSession(Handle handle,
-                            std::vector<SessionWindow*>* windows,
+  void OnGotPreviousSession(ScopedVector<SessionWindow> windows,
                             SessionID::id_type ignored_active_window);
 
   // Converts a SessionWindow into a Window, returning true on success. We use 0
@@ -233,11 +230,8 @@ class PersistentTabRestoreService::Delegate
   // LoadStateChanged is invoked, which adds these entries to entries_.
   std::vector<Entry*> staging_entries_;
 
-  // Used when loading previous tabs/session.
-  CancelableRequestConsumer load_consumer_;
-
-  // Used when loading open tabs/session when recovering from a crash.
-  CancelableRequestConsumer crash_consumer_;
+  // Used when loading previous tabs/session and open tabs/session.
+  CancelableTaskTracker cancelable_task_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(Delegate);
 };
@@ -348,8 +342,8 @@ void PersistentTabRestoreService::Delegate::LoadTabsFromLastSession() {
     // that we need to load the windows from session service (which will have
     // saved them).
     session_service->GetLastSession(
-        &crash_consumer_,
-        base::Bind(&Delegate::OnGotPreviousSession, base::Unretained(this)));
+        base::Bind(&Delegate::OnGotPreviousSession, base::Unretained(this)),
+        &cancelable_task_tracker_);
   } else {
     load_state_ |= LOADED_LAST_SESSION;
   }
@@ -359,10 +353,8 @@ void PersistentTabRestoreService::Delegate::LoadTabsFromLastSession() {
   // this won't contain the tabs/window that were open at the point of the
   // crash (the call to GetLastSession above requests those).
   ScheduleGetLastSessionCommands(
-      new InternalGetCommandsRequest(
-          base::Bind(&Delegate::OnGotLastSessionCommands,
-                     base::Unretained(this))),
-      &load_consumer_);
+      base::Bind(&Delegate::OnGotLastSessionCommands, base::Unretained(this)),
+      &cancelable_task_tracker_);
 }
 
 bool PersistentTabRestoreService::Delegate::IsLoaded() const {
@@ -550,10 +542,9 @@ int PersistentTabRestoreService::Delegate::GetSelectedNavigationIndexToPersist(
 }
 
 void PersistentTabRestoreService::Delegate::OnGotLastSessionCommands(
-    Handle handle,
-    scoped_refptr<InternalGetCommandsRequest> request) {
+    ScopedVector<SessionCommand> commands) {
   std::vector<Entry*> entries;
-  CreateEntriesFromCommands(request, &entries);
+  CreateEntriesFromCommands(commands.get(), &entries);
   // Closed tabs always go to the end.
   staging_entries_.insert(staging_entries_.end(), entries.begin(),
                           entries.end());
@@ -562,13 +553,11 @@ void PersistentTabRestoreService::Delegate::OnGotLastSessionCommands(
 }
 
 void PersistentTabRestoreService::Delegate::CreateEntriesFromCommands(
-    scoped_refptr<InternalGetCommandsRequest> request,
+    const std::vector<SessionCommand*>& commands,
     std::vector<Entry*>* loaded_entries) {
-  if (request->canceled() ||
-      tab_restore_service_helper_->entries().size() == kMaxEntries)
+  if (tab_restore_service_helper_->entries().size() == kMaxEntries)
     return;
 
-  std::vector<SessionCommand*>& commands = request->commands;
   // Iterate through the commands populating entries and id_to_entry.
   ScopedVector<Entry> entries;
   IDToEntry id_to_entry;
@@ -780,11 +769,10 @@ void PersistentTabRestoreService::Delegate::ValidateAndDeleteEmptyEntries(
 }
 
 void PersistentTabRestoreService::Delegate::OnGotPreviousSession(
-    Handle handle,
-    std::vector<SessionWindow*>* windows,
+    ScopedVector<SessionWindow> windows,
     SessionID::id_type ignored_active_window) {
   std::vector<Entry*> entries;
-  CreateEntriesFromWindows(windows, &entries);
+  CreateEntriesFromWindows(&windows.get(), &entries);
   // Previous session tabs go first.
   staging_entries_.insert(staging_entries_.begin(), entries.begin(),
                           entries.end());
diff --git a/chrome/browser/sessions/session_backend.cc b/chrome/browser/sessions/session_backend.cc
index 8540a79..7829d8b 100644
--- a/chrome/browser/sessions/session_backend.cc
+++ b/chrome/browser/sessions/session_backend.cc
@@ -244,12 +244,16 @@ void SessionBackend::AppendCommands(
 }
 
 void SessionBackend::ReadLastSessionCommands(
-    scoped_refptr<BaseSessionService::InternalGetCommandsRequest> request) {
-  if (request->canceled())
+    const CancelableTaskTracker::IsCanceledCallback& is_canceled,
+    const BaseSessionService::InternalGetCommandsCallback& callback) {
+  if (is_canceled.Run())
     return;
+
   Init();
-  ReadLastSessionCommandsImpl(&(request->commands));
-  request->ForwardResult(request->handle(), request);
+
+  ScopedVector<SessionCommand> commands;
+  ReadLastSessionCommandsImpl(&(commands.get()));
+  callback.Run(commands.Pass());
 }
 
 bool SessionBackend::ReadLastSessionCommandsImpl(
@@ -294,15 +298,6 @@ void SessionBackend::MoveCurrentSessionToLastSession() {
   ResetFile();
 }
 
-void SessionBackend::ReadCurrentSessionCommands(
-    scoped_refptr<BaseSessionService::InternalGetCommandsRequest> request) {
-  if (request->canceled())
-    return;
-  Init();
-  ReadCurrentSessionCommandsImpl(&(request->commands));
-  request->ForwardResult(request->handle(), request);
-}
-
 bool SessionBackend::ReadCurrentSessionCommandsImpl(
     std::vector<SessionCommand*>* commands) {
   Init();
diff --git a/chrome/browser/sessions/session_backend.h b/chrome/browser/sessions/session_backend.h
index 498b43f..c5e79e2 100644
--- a/chrome/browser/sessions/session_backend.h
+++ b/chrome/browser/sessions/session_backend.h
@@ -11,6 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/sessions/base_session_service.h"
 #include "chrome/browser/sessions/session_command.h"
+#include "chrome/common/cancelable_task_tracker.h"
 
 namespace net {
 class FileStream;
@@ -66,7 +67,8 @@ class SessionBackend : public base::RefCountedThreadSafe<SessionBackend> {
   // Invoked from the service to read the commands that make up the last
   // session, invokes ReadLastSessionCommandsImpl to do the work.
   void ReadLastSessionCommands(
-      scoped_refptr<BaseSessionService::InternalGetCommandsRequest> request);
+      const CancelableTaskTracker::IsCanceledCallback& is_canceled,
+      const BaseSessionService::InternalGetCommandsCallback& callback);
 
   // Reads the commands from the last file.
   //
@@ -82,11 +84,6 @@ class SessionBackend : public base::RefCountedThreadSafe<SessionBackend> {
   // browsers are running.
   void MoveCurrentSessionToLastSession();
 
-  // Invoked from the service to read the commands that make up the current
-  // session, invokes ReadCurrentSessionCommandsImpl to do the work.
-  void ReadCurrentSessionCommands(
-      scoped_refptr<BaseSessionService::InternalGetCommandsRequest> request);
-
   // Reads the commands from the current file.
   //
   // On success, the read commands are added to commands. It is up to the
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index b644b65..3f40cc2 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -14,6 +14,7 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
 #include "base/platform_file.h"
 #include "base/stl_util.h"
@@ -33,6 +34,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/ntp/app_launcher_handler.h"
+#include "chrome/common/cancelable_task_tracker.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/dom_storage_context.h"
@@ -523,8 +525,8 @@ class SessionRestoreImpl : public content::NotificationObserver {
         SessionServiceFactory::GetForProfile(profile_);
     DCHECK(session_service);
     session_service->GetLastSession(
-        &request_consumer_,
-        base::Bind(&SessionRestoreImpl::OnGotSession, base::Unretained(this)));
+        base::Bind(&SessionRestoreImpl::OnGotSession, base::Unretained(this)),
+        &cancelable_task_tracker_);
 
     if (synchronous_) {
       {
@@ -711,8 +713,7 @@ class SessionRestoreImpl : public content::NotificationObserver {
     return browser;
   }
 
-  void OnGotSession(SessionService::Handle handle,
-                    std::vector<SessionWindow*>* windows,
+  void OnGotSession(ScopedVector<SessionWindow> windows,
                     SessionID::id_type active_window_id) {
     base::TimeDelta time_to_got_sessions =
         base::TimeTicks::Now() - restore_started_;
@@ -728,13 +729,13 @@ class SessionRestoreImpl : public content::NotificationObserver {
 #endif
     if (synchronous_) {
       // See comment above windows_ as to why we don't process immediately.
-      windows_.swap(*windows);
+      windows_.swap(windows.get());
       active_window_id_ = active_window_id;
       MessageLoop::current()->QuitNow();
       return;
     }
 
-    ProcessSessionWindows(windows, active_window_id);
+    ProcessSessionWindows(&windows.get(), active_window_id);
   }
 
   Browser* ProcessSessionWindows(std::vector<SessionWindow*>* windows,
@@ -1103,7 +1104,7 @@ class SessionRestoreImpl : public content::NotificationObserver {
   std::vector<GURL> urls_to_open_;
 
   // Used to get the session.
-  CancelableRequestConsumer request_consumer_;
+  CancelableTaskTracker cancelable_task_tracker_;
 
   // Responsible for loading the tabs.
   scoped_refptr<TabLoader> tab_loader_;
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index 98e7ee7..e91c225 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -13,7 +13,6 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
-#include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
@@ -83,29 +82,6 @@ static const int kWritesPerReset = 250;
 
 namespace {
 
-// The callback from GetLastSession is internally routed to SessionService
-// first and then the caller. This is done so that the SessionWindows can be
-// recreated from the SessionCommands and the SessionWindows passed to the
-// caller. The following class is used for this.
-class InternalSessionRequest
-    : public BaseSessionService::InternalGetCommandsRequest {
- public:
-  InternalSessionRequest(
-      const CallbackType& callback,
-      const SessionService::SessionCallback& real_callback)
-      : BaseSessionService::InternalGetCommandsRequest(callback),
-        real_callback(real_callback) {
-  }
-
-  // The callback supplied to GetLastSession.
-  SessionService::SessionCallback real_callback;
-
- private:
-  ~InternalSessionRequest() {}
-
-  DISALLOW_COPY_AND_ASSIGN(InternalSessionRequest);
-};
-
 // Various payload structures.
 struct ClosedPayload {
   SessionID::id_type id;
@@ -500,15 +476,15 @@ void SessionService::SetTabUserAgentOverride(
       kCommandSetTabUserAgentOverride, tab_id.id(), user_agent_override));
 }
 
-SessionService::Handle SessionService::GetLastSession(
-    CancelableRequestConsumerBase* consumer,
-    const SessionCallback& callback) {
+CancelableTaskTracker::TaskId SessionService::GetLastSession(
+    const SessionCallback& callback,
+    CancelableTaskTracker* tracker) {
+  // OnGotSessionCommands maps the SessionCommands to browser state, then run
+  // the callback.
   return ScheduleGetLastSessionCommands(
-      new InternalSessionRequest(
-          base::Bind(&SessionService::OnGotSessionCommands,
-                     base::Unretained(this)),
-          callback),
-      consumer);
+      base::Bind(&SessionService::OnGotSessionCommands,
+                 base::Unretained(this), callback),
+      tracker);
 }
 
 void SessionService::Save() {
@@ -857,17 +833,14 @@ SessionCommand* SessionService::CreateSetActiveWindowCommand(
 }
 
 void SessionService::OnGotSessionCommands(
-    Handle handle,
-    scoped_refptr<InternalGetCommandsRequest> request) {
-  if (request->canceled())
-    return;
-
+    const SessionCallback& callback,
+    ScopedVector<SessionCommand> commands) {
   ScopedVector<SessionWindow> valid_windows;
   SessionID::id_type active_window_id = 0;
+
   RestoreSessionFromCommands(
-      request->commands, &(valid_windows.get()), &active_window_id);
-  static_cast<InternalSessionRequest*>(request.get())->real_callback.Run(
-      request->handle(), &(valid_windows.get()), active_window_id);
+      commands.get(), &valid_windows.get(), &active_window_id);
+  callback.Run(valid_windows.Pass(), active_window_id);
 }
 
 void SessionService::RestoreSessionFromCommands(
diff --git a/chrome/browser/sessions/session_service.h b/chrome/browser/sessions/session_service.h
index 6078806..280a8e8 100644
--- a/chrome/browser/sessions/session_service.h
+++ b/chrome/browser/sessions/session_service.h
@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/memory/scoped_vector.h"
 #include "base/time.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
@@ -18,6 +19,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
+#include "chrome/common/cancelable_task_tracker.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "ui/base/ui_base_types.h"
@@ -180,25 +182,16 @@ class SessionService : public BaseSessionService,
                                const SessionID& tab_id,
                                const std::string& user_agent_override);
 
-  // Callback from GetSavedSession of GetLastSession.
-  //
-  // The contents of the supplied vector are deleted after the callback is
-  // notified. To take ownership of the vector clear it before returning.
-  //
-  // The session ID is the id of the window that was last active.
-  typedef base::Callback<void(Handle,
-                              std::vector<SessionWindow*>*,
-                              SessionID::id_type)> SessionCallback;
+  // Callback from GetLastSession.
+  // The second parameter is the id of the window that was last active.
+  typedef base::Callback<void(ScopedVector<SessionWindow>, SessionID::id_type)>
+      SessionCallback;
 
   // Fetches the contents of the last session, notifying the callback when
   // done. If the callback is supplied an empty vector of SessionWindows
   // it means the session could not be restored.
-  //
-  // The created request does NOT directly invoke the callback, rather the
-  // callback invokes OnGotSessionCommands from which we map the
-  // SessionCommands to browser state, then notify the callback.
-  Handle GetLastSession(CancelableRequestConsumerBase* consumer,
-                        const SessionCallback& callback);
+  CancelableTaskTracker::TaskId GetLastSession(const SessionCallback& callback,
+                                               CancelableTaskTracker* tracker);
 
   // Overridden from BaseSessionService because we want some UMA reporting on
   // session update activities.
@@ -284,11 +277,9 @@ class SessionService : public BaseSessionService,
 
   SessionCommand* CreateSetActiveWindowCommand(const SessionID& window_id);
 
-  // Callback from the backend for getting the commands from the save file.
-  // Converts the commands in SessionWindows and notifies the real callback.
-  void OnGotSessionCommands(
-      Handle handle,
-      scoped_refptr<InternalGetCommandsRequest> request);
+  // Converts |commands| to SessionWindows and notifies the callback.
+  void OnGotSessionCommands(const SessionCallback& callback,
+                            ScopedVector<SessionCommand> commands);
 
   // Converts the commands into SessionWindows. On return any valid
   // windows are added to valid_windows. It is up to the caller to delete