sessions: remove unused code paths (and small cleanup)

SessionService(FilePath) constructor was only used in one
place, and can easily be converted to use the Profile*
constructor. This converts to the Profile* constructor and
cleans up conditional code.

BUG=none
TEST=none

Change-Id: I3f486ee48db97b8e825df31a66bf276737b76c5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2644070Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846166}

chrome/browser/sessions/session_service.cc

@@ -80,34 +80,16 @@ static const int kWritesPerReset = 250;
 
 SessionService::SessionService(Profile* profile)
     : profile_(profile),
-      should_use_delayed_save_(true),
       command_storage_manager_(
           std::make_unique<sessions::CommandStorageManager>(
               sessions::CommandStorageManager::kSessionRestore,
               profile->GetPath(),
-              this)),
-      has_open_trackable_browsers_(false),
-      move_on_new_browser_(false),
-      force_browser_not_alive_with_no_windows_(false),
-      rebuild_on_next_save_(false) {
+              this)) {
   // We should never be created when incognito.
   DCHECK(!profile->IsOffTheRecord());
-  Init();
-}
-
-SessionService::SessionService(const base::FilePath& save_path)
-    : profile_(nullptr),
-      should_use_delayed_save_(false),
-      command_storage_manager_(
-          std::make_unique<sessions::CommandStorageManager>(
-              sessions::CommandStorageManager::kSessionRestore,
-              save_path,
-              this)),
-      has_open_trackable_browsers_(false),
-      move_on_new_browser_(false),
-      force_browser_not_alive_with_no_windows_(false),
-      rebuild_on_next_save_(false) {
-  Init();
+  BrowserList::AddObserver(this);
+  registrar_.Add(this, chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST,
+                 content::NotificationService::AllSources());
 }
 
 SessionService::~SessionService() {
@@ -607,12 +589,6 @@ void SessionService::TabNavigationPathEntriesDeleted(const SessionID& window_id,
   command_storage_manager_->StartSaveTimer();
 }
 
-void SessionService::Init() {
-  BrowserList::AddObserver(this);
-  registrar_.Add(this, chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST,
-                 content::NotificationService::AllSources());
-}
-
 bool SessionService::ShouldRestoreWindowOfType(
     sessions::SessionWindow::WindowType window_type) const {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -911,9 +887,8 @@ void SessionService::CommitPendingCloses() {
 }
 
 bool SessionService::IsOnlyOneTabLeft() const {
-  if (!profile() || profile()->AsTestingProfile()) {
+  if (profile()->AsTestingProfile())
     return is_only_one_tab_left_for_test_;
-  }
 
   int window_count = 0;
   for (auto* browser : *BrowserList::GetInstance()) {
@@ -933,9 +908,8 @@ bool SessionService::IsOnlyOneTabLeft() const {
 
 bool SessionService::HasOpenTrackableBrowsers(
     const SessionID& window_id) const {
-  if (!profile() || profile()->AsTestingProfile()) {
+  if (profile()->AsTestingProfile())
     return has_open_trackable_browser_for_test_;
-  }
 
   for (auto* browser : *BrowserList::GetInstance()) {
     const SessionID browser_id = browser->session_id();
@@ -990,7 +964,7 @@ bool SessionService::ShouldTrackBrowser(Browser* browser) const {
 void SessionService::MaybeDeleteSessionOnlyData() {
   // Don't try anything if we're testing.  The browser_process is not fully
   // created and DeleteSession will crash if we actually attempt it.
-  if (!profile() || profile()->AsTestingProfile())
+  if (profile()->AsTestingProfile())
     return;
 
   // Clear session data if the last window for a profile has been closed and
@@ -1032,9 +1006,6 @@ bool SessionService::GetAvailableRangeForTest(const SessionID& tab_id,
 }
 
 void SessionService::LogExitEvent() {
-  if (!profile_)
-    return;
-
   // If there are pending closes, then we have already logged the exit.
   if (!pending_window_close_ids_.empty())
     return;

chrome/browser/sessions/session_service.h

@@ -77,12 +77,8 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
  public:
   // Creates a SessionService for the specified profile.
   explicit SessionService(Profile* profile);
-  // For testing.
-  explicit SessionService(const base::FilePath& save_path);
-
   ~SessionService() override;
 
-  // This may be NULL during testing.
   Profile* profile() const { return profile_; }
 
   // Returns true if a new window opening should really be treated like the
@@ -233,9 +229,7 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
   FRIEND_TEST_ALL_PREFIXES(SessionServiceTest, WorkspaceSavedOnOpened);
   FRIEND_TEST_ALL_PREFIXES(NoStartupWindowTest, DontInitSessionServiceForApps);
 
-  typedef std::map<SessionID, std::pair<int, int>> IdToRange;
-
-  void Init();
+  using IdToRange = std::map<SessionID, std::pair<int, int>>;
 
   // Returns true if a window of given |window_type| should get
   // restored upon session restore.
@@ -348,12 +342,12 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
   // If an exit event was logged, it is removed.
   void RemoveExitEvent();
 
-  // The profile. This may be null during testing.
+  // This is always non-null.
   Profile* profile_;
 
   // Whether to use delayed save. Set to false when constructed with a FilePath
   // (which should only be used for testing).
-  bool should_use_delayed_save_;
+  bool should_use_delayed_save_ = true;
 
   std::unique_ptr<sessions::CommandStorageManager> command_storage_manager_;
 
@@ -368,26 +362,26 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
   // last tabbed browser and no more tabbed browsers are open with the same
   // profile, the window ID is added here. These IDs are only committed (which
   // marks them as closed) if the user creates a new tabbed browser.
-  typedef std::set<SessionID> PendingWindowCloseIDs;
+  using PendingWindowCloseIDs = std::set<SessionID>;
   PendingWindowCloseIDs pending_window_close_ids_;
 
   // Set of tabs that have been closed by way of the last window or last tab
   // closing, but not yet committed.
-  typedef std::set<SessionID> PendingTabCloseIDs;
+  using PendingTabCloseIDs = std::set<SessionID>;
   PendingTabCloseIDs pending_tab_close_ids_;
 
   // When a window other than the last window (see description of
   // pending_window_close_ids) is closed, the id is added to this set.
-  typedef std::set<SessionID> WindowClosingIDs;
+  using WindowClosingIDs = std::set<SessionID>;
   WindowClosingIDs window_closing_ids_;
 
   // Set of windows we're tracking changes to. This is only browsers that
   // return true from |ShouldRestoreWindowOfType|.
-  typedef std::set<SessionID> WindowsTracking;
+  using WindowsTracking = std::set<SessionID>;
   WindowsTracking windows_tracking_;
 
   // Are there any open trackable browsers?
-  bool has_open_trackable_browsers_;
+  bool has_open_trackable_browsers_ = false;
 
   // Used to override HasOpenTrackableBrowsers()
   bool has_open_trackable_browser_for_test_ = true;
@@ -399,14 +393,14 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
   // browser (has_open_trackable_browsers_ is false), then the current session
   // is made the last session. See description above class for details on
   // current/last session.
-  bool move_on_new_browser_;
+  bool move_on_new_browser_ = false;
 
   // For browser_tests, since we want to simulate the browser shutting down
   // without quitting.
-  bool force_browser_not_alive_with_no_windows_;
+  bool force_browser_not_alive_with_no_windows_ = false;
 
   // Force session commands to be rebuild before next save event.
-  bool rebuild_on_next_save_;
+  bool rebuild_on_next_save_ = false;
 
   // Don't send duplicate SetSelectedTabInWindow commands when the selected
   // tab's index hasn't changed.

chrome/browser/sessions/session_service_unittest.cc

@@ -13,7 +13,6 @@
 #include "base/callback_helpers.h"
 #include "base/containers/contains.h"
 #include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/location.h"
 #include "base/optional.h"
 #include "base/run_loop.h"
@@ -66,10 +65,7 @@ class SessionServiceTest : public BrowserWithTestWindowTest {
   void SetUp() override {
     BrowserWithTestWindowTest::SetUp();
 
-    Profile* profile = browser()->profile();
-    session_service_ = std::make_unique<SessionService>(profile);
-    path_ = profile->GetPath();
-
+    session_service_ = std::make_unique<SessionService>(browser()->profile());
     helper_.SetService(session_service_.get());
 
     service()->SetWindowType(window_id, Browser::TYPE_NORMAL);
@@ -128,7 +124,7 @@ class SessionServiceTest : public BrowserWithTestWindowTest {
       SessionID* active_window_id) {
     DestroySessionService();
 
-    session_service_ = std::make_unique<SessionService>(path_);
+    session_service_ = std::make_unique<SessionService>(browser()->profile());
     helper_.SetService(session_service_.get());
 
     SessionID* non_null_active_window_id = active_window_id;
@@ -198,10 +194,6 @@ class SessionServiceTest : public BrowserWithTestWindowTest {
 
   const SessionID window_id = SessionID::NewUnique();
 
-  // Path used in testing.
-  base::ScopedTempDir temp_dir_;
-  base::FilePath path_;
-
   std::unique_ptr<SessionService> session_service_;
   SessionServiceTestHelper helper_;
 };