Changing SessionService to have a BaseSessionService, not being one.

chrome/browser/sessions/base_session_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_SESSIONS_BASE_SESSION_SERVICE_H_
 #define CHROME_BROWSER_SESSIONS_BASE_SESSION_SERVICE_H_
 
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ skipping 21 matching lines @@
   // backend to determine the name of the files.
   enum SessionType {
     SESSION_RESTORE,
     TAB_RESTORE
   };
 
   typedef base::Callback<void(ScopedVector<SessionCommand>)>
       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() {
     return pending_commands_;
   }
 
   // Whether the next save resets the file before writing to it.
   void set_pending_reset(bool value) { pending_reset_ = value; }
   bool pending_reset() const { return pending_reset_; }
 
   // Returns the number of commands sent down since the last reset.
   int commands_since_reset() const { return commands_since_reset_; }
 
   // Schedules a command. This adds |command| to pending_commands_ and
   // invokes StartSaveTimer to start a timer that invokes Save at a later
   // time.
   virtual void ScheduleCommand(scoped_ptr<SessionCommand> command);
 
   // 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);
+  // Uses the backend to load the last session commands from disc. |callback|
+  // gets called once the data has arrived.
+  base::CancelableTaskTracker::TaskId ScheduleGetLastSessionCommands(
+      const GetCommandsCallback& callback,
+      base::CancelableTaskTracker* tracker);
 
-  // Invokes SessionBackend::ReadLastSessionCommands with callback on the
-  // backend thread.
-  // If testing, SessionBackend::ReadLastSessionCommands is invoked directly.
-  base::CancelableTaskTracker::TaskId ScheduleGetLastSessionCommands(
-      const InternalGetCommandsCallback& callback,
-      base::CancelableTaskTracker* tracker);
+  // 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);
 
-  // Max number of navigation entries in each direction we'll persist.
-  static const int max_persist_navigation_count;
-
- private:
-  friend class BetterSessionRestoreCrashTest;
-
-  // The backend.
+  // The backend object which reads and saves commands.
   scoped_refptr<SessionBackend> backend_;
 
   // Commands we need to send over to the backend.
   ScopedVector<SessionCommand> pending_commands_;
 
   // Whether the backend file should be recreated the next time we send
   // over the commands.
   bool pending_reset_;
 
   // 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_;
 
   // Used to invoke Save.
   base::WeakPtrFactory<BaseSessionService> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BaseSessionService);
 };
 
 #endif  // CHROME_BROWSER_SESSIONS_BASE_SESSION_SERVICE_H_