Changing SessionService to have a BaseSessionService, not being one.

chrome/browser/sessions/session_service.cc

Index: chrome/browser/sessions/session_service.cc
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index 7d32d7f5fe46797b5ab1d9e0099457e5fe798135..4fee2a4483cf7e589bd6d17c0858773505597f7a 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -25,7 +25,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/base_session_service_delegate_impl.h"
-#include "chrome/browser/sessions/session_backend.h"
 #include "chrome/browser/sessions/session_command.h"
 #include "chrome/browser/sessions/session_data_deleter.h"
 #include "chrome/browser/sessions/session_restore.h"
@@ -65,12 +64,12 @@ static const int kWritesPerReset = 250;
 // SessionService -------------------------------------------------------------
 
 SessionService::SessionService(Profile* profile)
-    : BaseSessionService(
-        SESSION_RESTORE,
-        profile->GetPath(),
-        scoped_ptr<BaseSessionServiceDelegate>(
-            new BaseSessionServiceDelegateImpl(true))),
+    : BaseSessionServiceDelegateImpl(true),
       profile_(profile),
+      base_session_service_(
+        new BaseSessionService(BaseSessionService::SESSION_RESTORE,
+                               profile->GetPath(),
+                               this)),
       has_open_trackable_browsers_(false),
       move_on_new_browser_(false),
       save_delay_in_millis_(base::TimeDelta::FromMilliseconds(2500)),
@@ -84,12 +83,12 @@ SessionService::SessionService(Profile* profile)
 }
 
 SessionService::SessionService(const base::FilePath& save_path)
-    : BaseSessionService(
-        SESSION_RESTORE,
-        save_path,
-        scoped_ptr<BaseSessionServiceDelegate>(
-            new BaseSessionServiceDelegateImpl(false))),
+    : BaseSessionServiceDelegateImpl(false),
       profile_(NULL),
+      base_session_service_(
+        new BaseSessionService(BaseSessionService::SESSION_RESTORE,
+                               save_path,
+                               this)),
       has_open_trackable_browsers_(false),
       move_on_new_browser_(false),
       save_delay_in_millis_(base::TimeDelta::FromMilliseconds(2500)),
@@ -104,7 +103,7 @@ SessionService::~SessionService() {
   // The BrowserList should outlive the SessionService since it's static and
   // the SessionService is a KeyedService.
   BrowserList::RemoveObserver(this);
-  Save();
+  base_session_service_->SaveNow();
 }
 
 bool SessionService::ShouldNewWindowStartSession() {
@@ -142,11 +141,11 @@ void SessionService::MoveCurrentSessionToLastSession() {
   window_closing_ids_.clear();
   pending_window_close_ids_.clear();
 
-  Save();
+  base_session_service_->MoveCurrentSessionToLastSession();
+}
 
-  RunTaskOnBackendThread(
-      FROM_HERE, base::Bind(&SessionBackend::MoveCurrentSessionToLastSession,
-                            backend()));
+void SessionService::DeleteLastSession() {
+  base_session_service_->DeleteLastSession();
 }
 
 void SessionService::SetTabWindow(const SessionID& window_id,
@@ -430,9 +429,9 @@ void SessionService::TabRestored(WebContents* tab, bool pinned) {
                       tab,
                       -1,
                       pinned,
-                      &pending_commands(),
+                      &base_session_service_->pending_commands(),
                       NULL);
-  StartSaveTimer();
+  base_session_service_->StartSaveTimer();
 }
 
 void SessionService::SetSelectedNavigationIndex(const SessionID& window_id,
@@ -490,24 +489,24 @@ base::CancelableTaskTracker::TaskId SessionService::GetLastSession(
     base::CancelableTaskTracker* tracker) {
   // OnGotSessionCommands maps the SessionCommands to browser state, then run
   // the callback.
-  return ScheduleGetLastSessionCommands(
+  return base_session_service_->ScheduleGetLastSessionCommands(
       base::Bind(&SessionService::OnGotSessionCommands,
                  weak_factory_.GetWeakPtr(),
                  callback),
       tracker);
 }
 
-void SessionService::Save() {
-  bool had_commands = !pending_commands().empty();
-  BaseSessionService::Save();
-  if (had_commands) {
-    RecordSessionUpdateHistogramData(chrome::NOTIFICATION_SESSION_SERVICE_SAVED,
-                                     &last_updated_save_time_);
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_SESSION_SERVICE_SAVED,
-        content::Source<Profile>(profile()),
-        content::NotificationService::NoDetails());
-  }
+void SessionService::OnSavedCommands() {
+  RecordSessionUpdateHistogramData(chrome::NOTIFICATION_SESSION_SERVICE_SAVED,
+                                   &last_updated_save_time_);
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_SESSION_SERVICE_SAVED,
+      content::Source<Profile>(profile()),
+      content::NotificationService::NoDetails());
+}
+
+BaseSessionService* SessionService::GetBaseSessionServiceForTest() {
+  return base_session_service_.get();
 }
 
 void SessionService::Init() {
@@ -553,10 +552,6 @@ void SessionService::RemoveUnusedRestoreWindows(
   }
 }
 
-bool SessionService::processed_any_commands() {
-  return backend()->inited() || !pending_commands().empty();
-}
-
 bool SessionService::RestoreIfNecessary(const std::vector<GURL>& urls_to_open,
                                         Browser* browser) {
   if (ShouldNewWindowStartSession()) {
@@ -717,10 +712,11 @@ void SessionService::BuildCommandsForTab(const SessionID& window_id,
     CreateSetTabWindowCommand(window_id, session_id).release());
 
   const int current_index = tab->GetController().GetCurrentEntryIndex();
-  const int min_index = std::max(0,
-                                 current_index - max_persist_navigation_count);
+  const int min_index =
+      std::max(current_index - BaseSessionService::max_persist_navigation_count,
+               0);
   const int max_index =
-      std::min(current_index + max_persist_navigation_count,
+      std::min(current_index + BaseSessionService::max_persist_navigation_count,
                tab->GetController().GetEntryCount());
   const int pending_index = tab->GetController().GetPendingEntryIndex();
   if (tab_to_available_range) {
@@ -835,11 +831,11 @@ void SessionService::BuildCommandsFromBrowsers(
 }
 
 void SessionService::ScheduleResetCommands() {
-  set_pending_reset(true);
-  pending_commands().clear();
+  base_session_service_->set_pending_reset(true);

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

I think you should remove this and the next line and after
BuildCommandsFromBrowsers have a:
base_session_service_->ScheduleReset(); (which sets pending_resets_ and clears
pending_commands_).
base_session_service_->AddCommands(xxx);

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

I see what you are getting at, but this isn't as easy as you think it is:
BuildCommandsForBrowsers will add all commands to a vector in your suggestion
and then it will also call ScheduleCommand which will then add commands to the
default stream and even calling ReplacePendingCommand on them.

So if we want to do this more restructuring and careful thought needs to be
applied. I would suggest to postpone that.

+  base_session_service_->pending_commands().clear();
   tab_to_available_range_.clear();
   windows_tracking_.clear();
-  BuildCommandsFromBrowsers(&pending_commands(),
+  BuildCommandsFromBrowsers(&base_session_service_->pending_commands(),
                             &tab_to_available_range_,
                             &windows_tracking_);
   if (!windows_tracking_.empty()) {
@@ -848,19 +844,22 @@ void SessionService::ScheduleResetCommands() {
     has_open_trackable_browsers_ = true;
     move_on_new_browser_ = true;
   }
-  StartSaveTimer();
+  base_session_service_->StartSaveTimer();

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

StartSaveTimer shouldn't be necessary with the changes I've outlined.

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

Ditto.

 }
 
 void SessionService::ScheduleCommand(scoped_ptr<SessionCommand> command) {
   DCHECK(command);
-  if (ReplacePendingCommand(pending_commands(), &command))
+  if (ReplacePendingCommand(base_session_service_->pending_commands(),
+                            &command))
     return;
   bool is_closing_command = IsClosingCommand(command.get());
-  BaseSessionService::ScheduleCommand(command.Pass());
+  base_session_service_->ScheduleCommand(command.Pass());
   // Don't schedule a reset on tab closed/window closed. Otherwise we may
   // lose tabs/windows we want to restore from if we exit right after this.
-  if (!pending_reset() && pending_window_close_ids_.empty() &&
-      commands_since_reset() >= kWritesPerReset && is_closing_command) {
+  if (!base_session_service_->pending_reset() &&
+      pending_window_close_ids_.empty() &&
+      base_session_service_->commands_since_reset() >= kWritesPerReset &&
+      !is_closing_command) {
     ScheduleResetCommands();
   }
 }