Changing SessionService to have a BaseSessionService, not being one.

chrome/browser/sessions/session_service.cc

 // 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.
 
 #include "chrome/browser/sessions/session_service.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/threading/thread.h"
 #include "chrome/browser/background/background_mode_manager.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #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"
 #include "chrome/browser/sessions/session_service_utils.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
@@ skipping 19 matching lines @@
 using content::WebContents;
 using sessions::ContentSerializedNavigationBuilder;
 using sessions::SerializedNavigationEntry;
 
 // Every kWritesPerReset commands triggers recreating the file.
 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)),
       save_delay_in_mins_(base::TimeDelta::FromMinutes(10)),
       save_delay_in_hrs_(base::TimeDelta::FromHours(8)),
       force_browser_not_alive_with_no_windows_(false),
       weak_factory_(this) {
   // We should never be created when incognito.
   DCHECK(!profile->IsOffTheRecord());
   Init();
 }
 
 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)),
       save_delay_in_mins_(base::TimeDelta::FromMinutes(10)),
       save_delay_in_hrs_(base::TimeDelta::FromHours(8)),
       force_browser_not_alive_with_no_windows_(false),
       weak_factory_(this) {
   Init();
 }
 
 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() {
   // ChromeOS and OSX have different ideas of application lifetime than
   // the other platforms.
   // On ChromeOS opening a new window should never start a new session.
 #if defined(OS_CHROMEOS)
   if (!force_browser_not_alive_with_no_windows_)
     return false;
 #endif
@@ skipping 17 matching lines @@
 
 void SessionService::ResetFromCurrentBrowsers() {
   ScheduleResetCommands();
 }
 
 void SessionService::MoveCurrentSessionToLastSession() {
   pending_tab_close_ids_.clear();
   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,
                                   const SessionID& tab_id) {
   if (!ShouldTrackChangesToWindow(window_id))
     return;
 
   ScheduleCommand(CreateSetTabWindowCommand(window_id, tab_id).Pass());
 }
 
@@ skipping 263 matching lines @@
 
 void SessionService::TabRestored(WebContents* tab, bool pinned) {
   SessionTabHelper* session_tab_helper = SessionTabHelper::FromWebContents(tab);
   if (!ShouldTrackChangesToWindow(session_tab_helper->window_id()))
     return;
 
   BuildCommandsForTab(session_tab_helper->window_id(),
                       tab,
                       -1,
                       pinned,
-                      &pending_commands(),
+                      &base_session_service_->pending_commands(),
                       NULL);
-  StartSaveTimer();
+  base_session_service_->StartSaveTimer();
 }
 
 void SessionService::SetSelectedNavigationIndex(const SessionID& window_id,
                                                 const SessionID& tab_id,
                                                 int index) {
   if (!ShouldTrackChangesToWindow(window_id))
     return;
 
   if (tab_to_available_range_.find(tab_id.id()) !=
       tab_to_available_range_.end()) {
@@ skipping 37 matching lines @@
 
   ScheduleCommand(
     CreateSetTabExtensionAppIDCommand(tab_id, extension_app_id).Pass());
 }
 
 base::CancelableTaskTracker::TaskId SessionService::GetLastSession(
     const SessionCallback& callback,
     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() {
   // Register for the notifications we're interested in.
   registrar_.Add(this, content::NOTIFICATION_NAV_LIST_PRUNED,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_CHANGED,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                  content::NotificationService::AllSources());
@@ skipping 25 matching lines @@
                                    window->app_name.empty() ? TYPE_NORMAL :
                                                               TYPE_APP)) {
       delete window;
       window_list->erase(i++);
     } else {
       ++i;
     }
   }
 }
 
-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()) {
     // We're going from no tabbed browsers to a tabbed browser (and not in
     // process startup), restore the last session.
     if (move_on_new_browser_) {
       // Make the current session the last.
       MoveCurrentSessionToLastSession();
       move_on_new_browser_ = false;
     }
@@ skipping 140 matching lines @@
                                          bool is_pinned,
                                          ScopedVector<SessionCommand>* commands,
                                          IdToRange* tab_to_available_range) {
   DCHECK(tab && commands && window_id.id());
   SessionTabHelper* session_tab_helper = SessionTabHelper::FromWebContents(tab);
   const SessionID& session_id(session_tab_helper->session_id());
   commands->push_back(
     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) {
     (*tab_to_available_range)[session_id.id()] =
         std::pair<int, int>(min_index, max_index);
   }
 
   if (is_pinned)
     commands->push_back(CreatePinnedStateCommand(session_id, true).release());
 
@@ skipping 94 matching lines @@
     // deleted, so we ignore it.
     if (ShouldTrackBrowser(browser) && browser->tab_strip_model()->count() &&
         browser->window()) {
       BuildCommandsForBrowser(browser, commands, tab_to_available_range,
                               windows_to_track);
     }
   }
 }
 
 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()) {
     // We're lazily created on startup and won't get an initial batch of
     // SetWindowType messages. Set these here to make sure our state is correct.
     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();
   }
 }
 
 void SessionService::CommitPendingCloses() {
   for (PendingTabCloseIDs::iterator i = pending_tab_close_ids_.begin();
        i != pending_tab_close_ids_.end(); ++i) {
     ScheduleCommand(CreateTabClosedCommand(*i).Pass());
   }
   pending_tab_close_ids_.clear();
@@ skipping 211 matching lines @@
     return;
   }
 
   // Check for any open windows for the current profile that we aren't tracking.
   for (chrome::BrowserIterator it; !it.done(); it.Next()) {
     if ((*it)->profile() == profile())
       return;
   }
   DeleteSessionOnlyData(profile());
 }