Changing SessionService to have a BaseSessionService, not being one.

chrome/browser/sessions/base_session_service.h

Index: chrome/browser/sessions/base_session_service.h
diff --git a/chrome/browser/sessions/base_session_service.h b/chrome/browser/sessions/base_session_service.h
index 1a78ddf7e159a71096d8ecae3c4eee6535f36aef..e5dc2448cf46c0edb9133859fbe2fb3a32cb6ec8 100644
--- a/chrome/browser/sessions/base_session_service.h
+++ b/chrome/browser/sessions/base_session_service.h
@@ -39,24 +39,20 @@ class BaseSessionService {
       GetCommandsCallback;
 
   // Creates a new BaseSessionService. After creation you need to invoke
-  // Init.
+  // Init. |delegate| will remain owned by the creator and it is guaranteed
+  // that its lifetime surpasses this class.
   // |type| gives the type of session service, |path| the path to save files to.
   BaseSessionService(SessionType type,
                      const base::FilePath& path,
-                     scoped_ptr<BaseSessionServiceDelegate> delegate);
+                     BaseSessionServiceDelegate* delegate);
+  virtual ~BaseSessionService();
+
+  // Moves the current session to the last session.
+  void MoveCurrentSessionToLastSession();
 
   // Deletes the last session.
   void DeleteLastSession();
 
-  typedef base::Callback<void(ScopedVector<SessionCommand>)>
-      InternalGetCommandsCallback;
-
- protected:
-  virtual ~BaseSessionService();
-
-  // Returns the backend.
-  SessionBackend* backend() const { return backend_.get(); }
-
   // Returns the set of commands which were scheduled to be written. Once
   // committed to the backend, the commands are removed from here.
   ScopedVector<SessionCommand>& pending_commands() {
@@ -78,32 +74,39 @@ class BaseSessionService {
   // Starts the timer that invokes Save (if timer isn't already running).
   void StartSaveTimer();
 
-  // Saves pending commands to the backend. This is invoked from the timer
-  // scheduled by StartSaveTimer.
-  virtual void Save();
+  // Passes all pending commands to the backend for saving. Does nothing if
+  // no backend exists
+  virtual void SaveNow();
 
-  // Returns true if the entry at specified |url| should be written to disk.
-  bool ShouldTrackEntry(const GURL& url);
-
-  // Invokes SessionBackend::ReadLastSessionCommands with callback on the
-  // backend thread.
-  // If testing, SessionBackend::ReadLastSessionCommands is invoked directly.
+  // Uses the backend to load the last session commands from disc. |callback|
+  // gets called once the data has arrived.
   base::CancelableTaskTracker::TaskId ScheduleGetLastSessionCommands(
-      const InternalGetCommandsCallback& callback,
+      const GetCommandsCallback& callback,
       base::CancelableTaskTracker* tracker);
 
-  // This posts the task to the SequencedWorkerPool, or run immediately
-  // if the SequencedWorkerPool has been shutdown.
-  void RunTaskOnBackendThread(const tracked_objects::Location& from_here,
-                              const base::Closure& task);
+  // Returns true if any commands got processed yet (used by unit tests).
+  bool ProcessedAnyCommandsForTest();
 
   // Max number of navigation entries in each direction we'll persist.

[Mr4D (OOO till 08-26), 2014/10/31 19:24:01]

I do not exactly like to have this constant here - neither as function nor as
constant. It could get moved into the browser dependent part - but where?
session_command_utils? Not really. SessionService AND
PersistentTabRestoreService? Not much better. Any good ideas?

[sky, 2014/10/31 22:36:16]

I agree with you that this shouldn't be here. Not sure where to put it though.
What classes do you plan on promoting?

[Mr4D (OOO till 08-26), 2014/11/01 01:28:23]

Well I think the main thing is that this constant is an arbitrary limiter which
might make sense for Chrome - but not for example for athena.

As such this might be simply added to session_service_utils as a global constant
which can be used by SessionService and PersistentTabRestoreService. Would that
make sense?

Note: I want to make the "component cut" at the |BaseSessionSerrvice| level (+
SessionCommand creation functions).

Moved it there - if you know a better spot - let me know!

   static const int max_persist_navigation_count;
 
  private:
   friend class BetterSessionRestoreCrashTest;
+  friend class SessionServiceTestHelper;
+
+  // Saves pending commands to the backend immediately. This gets invoked by the
+  // timer scheduled by StartSaveTimer - or via SaveNow().
+  void SaveInternal();
+
+  // Returns the backend.
+  SessionBackend* backend() const { return backend_.get(); }
+
+  // This posts the task to the SequencedWorkerPool, or run immediately
+  // if the SequencedWorkerPool has been shutdown.
+  void RunTaskOnBackendThread(const tracked_objects::Location& from_here,
+                              const base::Closure& task);
 
-  // The backend.
+  // The backend object which reads and saves commands.
   scoped_refptr<SessionBackend> backend_;
 
   // Commands we need to send over to the backend.
@@ -116,7 +119,7 @@ class BaseSessionService {
   // The number of commands sent to the backend before doing a reset.
   int commands_since_reset_;
 
-  scoped_ptr<BaseSessionServiceDelegate> delegate_;
+  BaseSessionServiceDelegate* delegate_;
 
   // A token to make sure that all tasks will be serialized.
   base::SequencedWorkerPool::SequenceToken sequence_token_;