sessions: combines session storage managers

The current system truncates the file at various points. This means that
if chrome happens to crash during that time, the user won't have
anything to restore. The fix will be to not truncate the file, and
instead create a new one rather than truncating. As a result of this,
snapshotting and non-snapshotting will effectively become the same. This
patch combines the two classes. This patch doesn't actually change the
logic yet, it just combines the two.

BUG=648266
TEST=covered by tests

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

chrome/browser/sessions/session_service.cc

@@ -44,10 +44,10 @@
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "components/sessions/content/content_serialized_navigation_builder.h"
 #include "components/sessions/content/session_tab_helper.h"
+#include "components/sessions/core/command_storage_manager.h"
 #include "components/sessions/core/session_command.h"
 #include "components/sessions/core/session_constants.h"
 #include "components/sessions/core/session_types.h"
-#include "components/sessions/core/snapshotting_command_storage_manager.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/tab_groups/tab_group_id.h"
 #include "components/tab_groups/tab_group_visual_data.h"
@@ -79,8 +79,8 @@ SessionService::SessionService(Profile* profile)
     : profile_(profile),
       should_use_delayed_save_(true),
       command_storage_manager_(
-          std::make_unique<sessions::SnapshottingCommandStorageManager>(
-              sessions::SnapshottingCommandStorageManager::SESSION_RESTORE,
+          std::make_unique<sessions::CommandStorageManager>(
+              sessions::CommandStorageManager::kSessionRestore,
               profile->GetPath(),
               this)),
       has_open_trackable_browsers_(false),
@@ -96,8 +96,8 @@ SessionService::SessionService(const base::FilePath& save_path)
     : profile_(nullptr),
       should_use_delayed_save_(false),
       command_storage_manager_(
-          std::make_unique<sessions::SnapshottingCommandStorageManager>(
-              sessions::SnapshottingCommandStorageManager::SESSION_RESTORE,
+          std::make_unique<sessions::CommandStorageManager>(
+              sessions::CommandStorageManager::kSessionRestore,
               save_path,
               this)),
       has_open_trackable_browsers_(false),

chrome/browser/sessions/session_service.h

@@ -40,10 +40,10 @@ class WebContents;
 }  // namespace content
 
 namespace sessions {
+class CommandStorageManager;
 class SessionCommand;
 struct SessionTab;
 struct SessionWindow;
-class SnapshottingCommandStorageManager;
 }  // namespace sessions
 
 // SessionService ------------------------------------------------------------
@@ -337,8 +337,7 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
   // (which should only be used for testing).
   bool should_use_delayed_save_;
 
-  std::unique_ptr<sessions::SnapshottingCommandStorageManager>
-      command_storage_manager_;
+  std::unique_ptr<sessions::CommandStorageManager> command_storage_manager_;
 
   // Maps from session tab id to the range of navigation entries that has
   // been written to disk.

components/sessions/BUILD.gn

@@ -114,10 +114,6 @@ source_set("shared") {
     "core/session_service_commands.h",
     "core/session_types.cc",
     "core/session_types.h",
-    "core/snapshotting_command_storage_backend.cc",
-    "core/snapshotting_command_storage_backend.h",
-    "core/snapshotting_command_storage_manager.cc",
-    "core/snapshotting_command_storage_manager.h",
     "core/tab_restore_service.cc",
     "core/tab_restore_service.h",
     "core/tab_restore_service_client.cc",
@@ -190,7 +186,6 @@ source_set("unit_tests") {
     "core/command_storage_backend_unittest.cc",
     "core/serialized_navigation_entry_unittest.cc",
     "core/session_id_generator_unittest.cc",
-    "core/snapshotting_session_backend_unittest.cc",
   ]
 
   if (!is_ios) {

components/sessions/core/command_storage_backend.cc

@@ -10,14 +10,22 @@
 #include <utility>
 
 #include "base/files/file.h"
+#include "base/files/file_enumerator.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
+#include "components/sessions/core/session_constants.h"
 #include "crypto/aead.h"
 
 namespace sessions {
 
+using SessionType = CommandStorageManager::SessionType;
+
 namespace {
 
 // File version number.
@@ -261,6 +269,41 @@ bool SessionFileReader::FillBuffer() {
   return true;
 }
 
+base::FilePath::StringType TimestampToString(const base::Time time) {
+#if defined(OS_POSIX) || defined(OS_FUCHSIA)
+  return base::NumberToString(time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+#elif defined(OS_WIN)
+  return base::NumberToString16(
+      time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+#endif
+}
+
+base::FilePath::StringType GetSessionFilename(
+    const CommandStorageManager::SessionType type,
+    const base::FilePath::StringType& timestamp_str) {
+  DCHECK_NE(CommandStorageManager::SessionType::kOther, type);
+  if (type == CommandStorageManager::kTabRestore)
+    return base::JoinString({kTabSessionFileNamePrefix, timestamp_str},
+                            FILE_PATH_LITERAL("_"));
+
+  return base::JoinString({kSessionFileNamePrefix, timestamp_str},
+                          FILE_PATH_LITERAL("_"));
+}
+
+base::FilePath GetLegacySessionPath(CommandStorageManager::SessionType type,
+                                    const base::FilePath& base_path,
+                                    bool current) {
+  DCHECK_NE(CommandStorageManager::SessionType::kOther, type);
+  base::FilePath session_path = base_path;
+
+  if (type == CommandStorageManager::kTabRestore) {
+    return session_path.Append(current ? kLegacyCurrentTabSessionFileName
+                                       : kLegacyLastTabSessionFileName);
+  }
+  return session_path.Append(current ? kLegacyCurrentSessionFileName
+                                     : kLegacyLastSessionFileName);
+}
+
 }  // namespace
 
 // CommandStorageBackend
@@ -275,8 +318,18 @@ const SessionCommand::size_type
 
 CommandStorageBackend::CommandStorageBackend(
     scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
-    const base::FilePath& path)
-    : RefCountedDeleteOnSequence(owning_task_runner), path_(path) {}
+    const base::FilePath& path,
+    SessionType type)
+    : RefCountedDeleteOnSequence(owning_task_runner),
+      type_(type),
+      supplied_path_(path),
+      timestamp_(base::Time::Now()) {
+  // This is invoked on the main thread, don't do file access here.
+  if (type == CommandStorageManager::kOther)
+    current_path_ = path;
+  else
+    current_path_ = FilePathFromTime(type, path, timestamp_);
+}
 
 // static
 bool CommandStorageBackend::IsValidFile(const base::FilePath& path) {
@@ -327,7 +380,85 @@ CommandStorageBackend::ReadCurrentSessionCommands(
     const std::vector<uint8_t>& crypto_key) {
   InitIfNecessary();
 
-  return ReadCommandsFromFile(path_, crypto_key);
+  return ReadCommandsFromFile(current_path_, crypto_key);
+}
+
+// static
+bool CommandStorageBackend::TimestampFromPath(const base::FilePath& path,
+                                              base::Time& timestamp_result) {
+  auto parts =
+      base::SplitString(path.BaseName().value(), FILE_PATH_LITERAL("_"),
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (parts.size() != 2u)
+    return false;
+
+  int64_t result = 0u;
+  if (!base::StringToInt64(parts[1], &result))
+    return false;
+
+  timestamp_result = base::Time::FromDeltaSinceWindowsEpoch(
+      base::TimeDelta::FromMicroseconds(result));
+  return true;
+}
+
+// static
+base::FilePath CommandStorageBackend::FilePathFromTime(
+    const SessionType type,
+    const base::FilePath& base_path,
+    base::Time time) {
+  DCHECK_NE(CommandStorageManager::SessionType::kOther, type);
+  return base_path.Append(kSessionsDirectory)
+      .Append(GetSessionFilename(type, TimestampToString(time)));
+}
+
+std::vector<std::unique_ptr<SessionCommand>>
+CommandStorageBackend::ReadLastSessionCommands() {
+  InitIfNecessary();
+
+  if (last_session_info_)
+    return ReadCommandsFromFile(last_session_info_->path,
+                                std::vector<uint8_t>());
+  return {};
+}
+
+void CommandStorageBackend::DeleteLastSession() {
+  InitIfNecessary();
+  if (last_session_info_) {
+    base::DeleteFile(last_session_info_->path);
+    last_session_info_.reset();
+  }
+}
+
+void CommandStorageBackend::MoveCurrentSessionToLastSession() {
+  // TODO(sky): make this work for kOther.
+  DCHECK_NE(CommandStorageManager::SessionType::kOther, type_);
+
+  InitIfNecessary();
+  CloseFile();
+  DeleteLastSession();
+
+  // Move current session to last.
+  if (base::PathExists(current_path_))
+    last_session_info_ = SessionInfo{current_path_, timestamp_};
+  else
+    last_session_info_.reset();
+
+  // Create new file, ensuring the timestamp is after the previous.
+  // Due to clock changes, there might be an existing session with a later
+  // time. This is especially true on Windows, which uses the local time as
+  // the system clock.
+  base::Time new_timestamp = base::Time::Now();
+  if (!last_session_info_ || last_session_info_->timestamp < new_timestamp) {
+    timestamp_ = new_timestamp;
+  } else {
+    timestamp_ =
+        last_session_info_->timestamp + base::TimeDelta::FromMicroseconds(1);
+  }
+  SetPath(FilePathFromTime(type_, supplied_path_, timestamp_));
+
+  // Create and open the file for the current session.
+  DCHECK(!base::PathExists(current_path_));
+  TruncateFile();
 }
 
 bool CommandStorageBackend::AppendCommandsToFile(
@@ -355,16 +486,37 @@ void CommandStorageBackend::InitIfNecessary() {
     return;
 
   inited_ = true;
-  base::CreateDirectory(path_.DirName());
-  DoInit();
+  base::CreateDirectory(current_path_.DirName());
+
+  if (type_ == CommandStorageManager::kOther)
+    return;
+
+  DetermineLastSessionFile();
+  if (last_session_info_) {
+    // Check that the last session's timestamp is before the current file's.
+    // This might not be true if the system clock has changed.
+    if (last_session_info_->timestamp > timestamp_) {
+      timestamp_ =
+          last_session_info_->timestamp + base::TimeDelta::FromMicroseconds(1);
+      SetPath(FilePathFromTime(type_, supplied_path_, timestamp_));
+    }
+  }
+
+  // Best effort delete all sessions except the current & last.
+  DeleteLastSessionFiles();
+
+  // Create and open the file for the current session.
+  DCHECK(!base::PathExists(current_path_));
+  TruncateFile();
 }
 
 void CommandStorageBackend::SetPath(const base::FilePath& path) {
   // Do not change the path if the file is open
   DCHECK(!file_);
-  path_ = path;
+  current_path_ = path;
 }
 
+// static
 std::vector<std::unique_ptr<sessions::SessionCommand>>
 CommandStorageBackend::ReadCommandsFromFile(
     const base::FilePath& path,
@@ -390,7 +542,7 @@ void CommandStorageBackend::TruncateFile() {
       file_.reset();
   }
   if (!file_)
-    file_ = OpenAndWriteHeader(path_);
+    file_ = OpenAndWriteHeader(current_path_);
   commands_written_ = 0;
 }
 
@@ -492,4 +644,67 @@ bool CommandStorageBackend::AppendEncryptedCommandToFile(
   return true;
 }
 
+void CommandStorageBackend::DetermineLastSessionFile() {
+  last_session_info_.reset();
+
+  // Determine the session with the most recent timestamp that
+  // does not match the current session path.
+  for (const SessionInfo& session : GetSessionFiles()) {
+    if (session.path != current_path_) {
+      if (!last_session_info_ ||
+          session.timestamp > last_session_info_->timestamp)
+        last_session_info_ = session;
+    }
+  }
+
+  // If no last session was found, use the legacy session if present.
+  // The legacy session is considered to have a timestamp of 0, before any
+  // new session.
+  if (!last_session_info_) {
+    base::FilePath legacy_session =
+        GetLegacySessionPath(type_, supplied_path_, true);
+    if (base::PathExists(legacy_session))
+      last_session_info_ = SessionInfo{legacy_session, base::Time()};
+  }
+}
+
+void CommandStorageBackend::DeleteLastSessionFiles() {
+  // Delete session files whose paths do not match the current
+  // or last session path.
+  for (const SessionInfo& session : GetSessionFiles()) {
+    if (session.path != current_path_ &&
+        (!last_session_info_ || session.path != last_session_info_->path)) {
+      base::DeleteFile(session.path);
+    }
+  }
+
+  // Delete legacy session files, unless they are being used.
+  base::FilePath current_session_path =
+      GetLegacySessionPath(type_, supplied_path_, true);
+  if (last_session_info_ && current_session_path != last_session_info_->path &&
+      base::PathExists(current_session_path))
+    base::DeleteFile(current_session_path);
+
+  base::FilePath last_session_path =
+      GetLegacySessionPath(type_, supplied_path_, false);
+  if (base::PathExists(last_session_path))
+    base::DeleteFile(last_session_path);
+}
+
+std::vector<CommandStorageBackend::SessionInfo>
+CommandStorageBackend::GetSessionFiles() {
+  std::vector<SessionInfo> sessions;
+  base::FileEnumerator file_enum(
+      supplied_path_.Append(kSessionsDirectory), false,
+      base::FileEnumerator::FILES,
+      GetSessionFilename(type_, FILE_PATH_LITERAL("*")));
+  for (base::FilePath name = file_enum.Next(); !name.empty();
+       name = file_enum.Next()) {
+    base::Time file_time;
+    if (TimestampFromPath(name, file_time))
+      sessions.push_back(SessionInfo{name, file_time});
+  }
+  return sessions;
+}
+
 }  // namespace sessions

components/sessions/core/command_storage_backend.h

@@ -12,6 +12,8 @@
 
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted_delete_on_sequence.h"
+#include "base/optional.h"
+#include "components/sessions/core/command_storage_manager.h"
 #include "components/sessions/core/session_command.h"
 #include "components/sessions/core/sessions_export.h"
 
@@ -42,14 +44,21 @@ class SESSIONS_EXPORT CommandStorageBackend
   // Number of bytes encryption adds.
   static const size_type kEncryptionOverheadInBytes;
 
+  // Represents data for a session. Public for tests.
+  struct SessionInfo {
+    base::FilePath path;
+    base::Time timestamp;
+  };
+
   // Creates a CommandStorageBackend. This method is invoked on the MAIN thread,
   // and does no IO. The real work is done from Init, which is invoked on
   // a background task runer.
   //
-  // |path| is the path the file is written to.
+  // See `CommandStorageManager` for details on `type` and `path`.
   CommandStorageBackend(
       scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
-      const base::FilePath& path);
+      const base::FilePath& path,
+      CommandStorageManager::SessionType type);
   CommandStorageBackend(const CommandStorageBackend&) = delete;
   CommandStorageBackend& operator=(const CommandStorageBackend&) = delete;
 
@@ -76,26 +85,45 @@ class SESSIONS_EXPORT CommandStorageBackend
 
   bool inited() const { return inited_; }
 
- protected:
-  virtual ~CommandStorageBackend();
+  // Parses out the timestamp from a path pointing to a session file.
+  static bool TimestampFromPath(const base::FilePath& path, base::Time& result);
+
+  // Generates the path to a session file with the given timestamp.
+  static base::FilePath FilePathFromTime(
+      CommandStorageManager::SessionType type,
+      const base::FilePath& base_path,
+      base::Time time);
+
+  // Returns the commands from the last session file.
+  std::vector<std::unique_ptr<SessionCommand>> ReadLastSessionCommands();
+
+  // Deletes the file containing the commands for the last session.
+  void DeleteLastSession();
+
+  // Moves the current session file to the last session file. This is typically
+  // called during startup or if the user launches the app and no tabbed
+  // browsers are running.
+  void MoveCurrentSessionToLastSession();
+
+ private:
+  friend class base::RefCountedDeleteOnSequence<CommandStorageBackend>;
+  friend class base::DeleteHelper<CommandStorageBackend>;
+  friend class CommandStorageBackendTest;
+
+  ~CommandStorageBackend();
 
   // Performs initialization on the background task run, calling DoInit() if
   // necessary.
   void InitIfNecessary();
 
-  // Called the first time InitIfNecessary() is called.
-  virtual void DoInit() {}
-
-  const base::FilePath& path() const { return path_; }
-
   // Change the file path used to save the session. Must be called after closing
   // the file first (CloseFile())
   void SetPath(const base::FilePath& path);
 
   // Reads the commands from the specified file.  If |crypto_key| is non-empty,
   // it is used to decrypt the file.
-  std::vector<std::unique_ptr<sessions::SessionCommand>> ReadCommandsFromFile(
-      const base::FilePath& path,
+  static std::vector<std::unique_ptr<sessions::SessionCommand>>
+  ReadCommandsFromFile(const base::FilePath& path,
                        const std::vector<uint8_t>& crypto_key);
 
   // Closes the file. The next time AppendCommands() is called the file will
@@ -110,10 +138,6 @@ class SESSIONS_EXPORT CommandStorageBackend
   // the header couldn't be written.
   void TruncateFile();
 
- private:
-  friend class base::RefCountedDeleteOnSequence<CommandStorageBackend>;
-  friend class base::DeleteHelper<CommandStorageBackend>;
-
   // Opens the current file and writes the header. On success a handle to
   // the file is returned.
   std::unique_ptr<base::File> OpenAndWriteHeader(const base::FilePath& path);
@@ -136,8 +160,24 @@ class SESSIONS_EXPORT CommandStorageBackend
   // Returns true if commands are encrypted.
   bool IsEncrypted() const { return !crypto_key_.empty(); }
 
-  // Path commands are saved to.
-  base::FilePath path_;
+  // Gets data for the last session file.
+  void DetermineLastSessionFile();
+
+  // Attempt to delete all sessions besides the current and last. This is a
+  // best effort operation.
+  void DeleteLastSessionFiles();
+
+  // Gets all sessions files.
+  std::vector<SessionInfo> GetSessionFiles();
+
+  const CommandStorageManager::SessionType type_;
+
+  // This is the path supplied to the constructor. See CommandStorageManager
+  // constructor for details.
+  const base::FilePath supplied_path_;
+
+  // Path commands are currently being saved to.
+  base::FilePath current_path_;
 
   // This may be null, created as necessary.
   std::unique_ptr<base::File> file_;
@@ -151,6 +191,12 @@ class SESSIONS_EXPORT CommandStorageBackend
 
   // Incremented every time a command is written.
   int commands_written_ = 0;
+
+  // Timestamp when this session was started.
+  base::Time timestamp_;
+
+  // Data for the last session. If unset, fallback to legacy session data.
+  base::Optional<SessionInfo> last_session_info_;
 };
 
 }  // namespace sessions

components/sessions/core/command_storage_manager.cc

@@ -28,15 +28,17 @@ constexpr base::TimeDelta kSaveDelay = base::TimeDelta::FromMilliseconds(2500);
 }  // namespace
 
 CommandStorageManager::CommandStorageManager(
+    SessionType type,
     const base::FilePath& path,
     CommandStorageManagerDelegate* delegate,
     bool enable_crypto)
-    : CommandStorageManager(base::MakeRefCounted<CommandStorageBackend>(
+    : backend_(base::MakeRefCounted<CommandStorageBackend>(
           CreateDefaultBackendTaskRunner(),
-                                path),
-                            delegate) {
-  use_crypto_ = enable_crypto;
-}
+          path,
+          type)),
+      use_crypto_(enable_crypto),
+      delegate_(delegate),
+      backend_task_runner_(backend_->owning_task_runner()) {}
 
 CommandStorageManager::~CommandStorageManager() = default;
 
@@ -145,12 +147,28 @@ void CommandStorageManager::GetCurrentSessionCommands(
       std::move(callback));
 }
 
-CommandStorageManager::CommandStorageManager(
-    scoped_refptr<CommandStorageBackend> backend,
-    CommandStorageManagerDelegate* delegate)
-    : backend_(std::move(backend)),
-      delegate_(delegate),
-      backend_task_runner_(backend_->owning_task_runner()) {}
+void CommandStorageManager::MoveCurrentSessionToLastSession() {
+  Save();
+  backend_task_runner()->PostNonNestableTask(
+      FROM_HERE,
+      base::BindOnce(&CommandStorageBackend::MoveCurrentSessionToLastSession,
+                     backend()));
+}
+
+void CommandStorageManager::DeleteLastSession() {
+  backend_task_runner()->PostNonNestableTask(
+      FROM_HERE,
+      base::BindOnce(&CommandStorageBackend::DeleteLastSession, backend()));
+}
+
+void CommandStorageManager::GetLastSessionCommands(
+    GetCommandsCallback callback) {
+  backend_task_runner()->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&CommandStorageBackend::ReadLastSessionCommands,
+                     backend()),
+      std::move(callback));
+}
 
 // static
 scoped_refptr<base::SequencedTaskRunner>

components/sessions/core/command_storage_manager.h

@@ -36,11 +36,25 @@ class SESSIONS_EXPORT CommandStorageManager {
   using GetCommandsCallback =
       base::OnceCallback<void(std::vector<std::unique_ptr<SessionCommand>>)>;
 
+  // Identifies the type of session service this is. This is used by the
+  // backend to determine the name of the files.
+  // TODO(sky): this enum is purely for legacy reasons, and should be replaced
+  // with consumers building the path (similar to weblayer). Remove in
+  // approximately a year (1/2022), when we shouldn't need to worry too much
+  // about migrating older data.
+  enum SessionType { kSessionRestore, kTabRestore, kOther };
+
   // Creates a new CommandStorageManager. After creation you need to invoke
-  // Init. |delegate| will remain owned by the creator and it is guaranteed
-  // that its lifetime surpasses this class. |path| is the path to save files
-  // to. If |enable_crypto| is true, the contents of the file are encrypted.
-  CommandStorageManager(const base::FilePath& path,
+  // Init(). `delegate` is not owned by this and must outlive this. If
+  // `enable_crypto` is true, the contents of the file are encrypted.
+  //
+  // The meaning of `path` depends upon the type. If `type` is `kOther`, then
+  // the path is a file name to which `_TIMESTAMP` is added. If `type` is not
+  // `kOther`, then it is a path to a directory. The actual file name used
+  // depends upon the type. Once SessionType can be removed, this logic can
+  // standardize on that of `kOther`.
+  CommandStorageManager(SessionType type,
+                        const base::FilePath& path,
                         CommandStorageManagerDelegate* delegate,
                         bool enable_crypto = false);
   CommandStorageManager(const CommandStorageManager&) = delete;
@@ -104,10 +118,19 @@ class SESSIONS_EXPORT CommandStorageManager {
   void GetCurrentSessionCommands(GetCommandsCallback callback,
                                  const std::vector<uint8_t>& decryption_key);
 
- protected:
-  // Provided for subclasses.
-  CommandStorageManager(scoped_refptr<CommandStorageBackend> backend,
-                        CommandStorageManagerDelegate* delegate);
+  // Moves the current session to the last session.
+  void MoveCurrentSessionToLastSession();
+
+  // Deletes the last session.
+  void DeleteLastSession();
+
+  // Uses the backend to load the last session commands from disk. |callback|
+  // is called once the data has arrived, and may be called after this is
+  // deleted.
+  void GetLastSessionCommands(GetCommandsCallback callback);
+
+ private:
+  friend class CommandStorageManagerTestHelper;
 
   // Creates a SequencedTaskRunner suitable for the backend.
   static scoped_refptr<base::SequencedTaskRunner>
@@ -119,14 +142,11 @@ class SESSIONS_EXPORT CommandStorageManager {
 
   CommandStorageBackend* backend() { return backend_.get(); }
 
- private:
-  friend class CommandStorageManagerTestHelper;
-
   // The backend object which reads and saves commands.
   scoped_refptr<CommandStorageBackend> backend_;
 
   // If true, all commands are encrypted.
-  bool use_crypto_ = false;
+  const bool use_crypto_;
 
   // Commands we need to send over to the backend.
   std::vector<std::unique_ptr<SessionCommand>> pending_commands_;

components/sessions/core/command_storage_manager_test_helper.cc

@@ -7,7 +7,6 @@
 #include "base/bind.h"
 #include "components/sessions/core/command_storage_backend.h"
 #include "components/sessions/core/command_storage_manager.h"
-#include "components/sessions/core/snapshotting_command_storage_backend.h"
 
 namespace sessions {
 
@@ -31,9 +30,7 @@ bool CommandStorageManagerTestHelper::ProcessedAnyCommands() {
 
 std::vector<std::unique_ptr<SessionCommand>>
 CommandStorageManagerTestHelper::ReadLastSessionCommands() {
-  return static_cast<SnapshottingCommandStorageBackend*>(
-             command_storage_manager_->backend_.get())
-      ->ReadLastSessionCommands();
+  return command_storage_manager_->backend_.get()->ReadLastSessionCommands();
 }
 
 scoped_refptr<base::SequencedTaskRunner>

components/sessions/core/snapshotting_command_storage_backend.cc

-// Copyright 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 "components/sessions/core/snapshotting_command_storage_backend.h"
-
-#include "base/bind.h"
-#include "base/files/file_enumerator.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/strings/string_split.h"
-#include "base/strings/string_util.h"
-#include "build/build_config.h"
-#include "components/sessions/core/session_constants.h"
-
-namespace sessions {
-namespace {
-
-base::FilePath::StringType TimestampToString(const base::Time time) {
-#if defined(OS_POSIX) || defined(OS_FUCHSIA)
-  return base::NumberToString(time.ToDeltaSinceWindowsEpoch().InMicroseconds());
-#elif defined(OS_WIN)
-  return base::NumberToString16(
-      time.ToDeltaSinceWindowsEpoch().InMicroseconds());
-#endif
-}
-
-base::FilePath::StringType GetSessionFilename(
-    const SnapshottingCommandStorageManager::SessionType type,
-    const base::FilePath::StringType& timestamp_str) {
-  if (type == SnapshottingCommandStorageManager::TAB_RESTORE)
-    return base::JoinString({kTabSessionFileNamePrefix, timestamp_str},
-                            FILE_PATH_LITERAL("_"));
-
-  return base::JoinString({kSessionFileNamePrefix, timestamp_str},
-                          FILE_PATH_LITERAL("_"));
-}
-
-base::FilePath GetLegacySessionPath(
-    SnapshottingCommandStorageManager::SessionType type,
-    const base::FilePath& base_path,
-    bool current) {
-  base::FilePath session_path = base_path;
-
-  if (type == SnapshottingCommandStorageManager::TAB_RESTORE) {
-    return session_path.Append(current ? kLegacyCurrentTabSessionFileName
-                                       : kLegacyLastTabSessionFileName);
-  }
-  return session_path.Append(current ? kLegacyCurrentSessionFileName
-                                     : kLegacyLastSessionFileName);
-}
-
-}  // namespace
-
-SnapshottingCommandStorageBackend::SnapshottingCommandStorageBackend(
-    scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
-    SnapshottingCommandStorageManager::SessionType type,
-    const base::FilePath& path_to_dir)
-    : CommandStorageBackend(
-          std::move(owning_task_runner),
-          FilePathFromTime(type, path_to_dir, base::Time::Now())),
-      base_dir_(path_to_dir),
-      type_(type) {
-  // NOTE: this is invoked on the main thread, don't do file access here.
-
-  // We need to retrieve the timestamp after initializing the superclass.
-  if (!TimestampFromPath(path(), timestamp_))
-    NOTREACHED();
-}
-
-// static
-bool SnapshottingCommandStorageBackend::TimestampFromPath(
-    const base::FilePath& path,
-    base::Time& timestamp_result) {
-  auto parts =
-      base::SplitString(path.BaseName().value(), FILE_PATH_LITERAL("_"),
-                        base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  if (parts.size() != 2u)
-    return false;
-
-  int64_t result = 0u;
-  if (!base::StringToInt64(parts[1], &result))
-    return false;
-
-  timestamp_result = base::Time::FromDeltaSinceWindowsEpoch(
-      base::TimeDelta::FromMicroseconds(result));
-  return true;
-}
-
-// static
-base::FilePath SnapshottingCommandStorageBackend::FilePathFromTime(
-    const SnapshottingCommandStorageManager::SessionType type,
-    const base::FilePath& base_path,
-    base::Time time) {
-  return base_path.Append(kSessionsDirectory)
-      .Append(GetSessionFilename(type, TimestampToString(time)));
-}
-
-std::vector<std::unique_ptr<SessionCommand>>
-SnapshottingCommandStorageBackend::ReadLastSessionCommands() {
-  InitIfNecessary();
-
-  if (last_session_info_)
-    return ReadCommandsFromFile(last_session_info_->path,
-                                std::vector<uint8_t>());
-  else
-    return {};
-}
-
-void SnapshottingCommandStorageBackend::DeleteLastSession() {
-  InitIfNecessary();
-  if (last_session_info_) {
-    base::DeleteFile(last_session_info_->path);
-    last_session_info_.reset();
-  }
-}
-
-void SnapshottingCommandStorageBackend::MoveCurrentSessionToLastSession() {
-  InitIfNecessary();
-  CloseFile();
-  DeleteLastSession();
-
-  // Move current session to last.
-  if (base::PathExists(path()))
-    last_session_info_ = SessionInfo{path(), timestamp_};
-  else
-    last_session_info_.reset();
-
-  // Create new file, ensuring the timestamp is after the previous.
-  // Due to clock changes, there might be an existing session with a later
-  // time. This is especially true on Windows, which uses the local time as
-  // the system clock.
-  base::Time new_timestamp = base::Time::Now();
-  if (!last_session_info_ || last_session_info_->timestamp < new_timestamp) {
-    timestamp_ = new_timestamp;
-  } else {
-    timestamp_ =
-        last_session_info_->timestamp + base::TimeDelta::FromMicroseconds(1);
-  }
-  SetPath(FilePathFromTime(type_, base_dir_, timestamp_));
-
-  // Create and open the file for the current session.
-  DCHECK(!base::PathExists(path()));
-  TruncateFile();
-}
-
-void SnapshottingCommandStorageBackend::DoInit() {
-  // Find the last session.
-  DetermineLastSessionFile();
-  if (last_session_info_) {
-    // Check that the last session's timestamp is before the current file's.
-    // This might not be true if the system clock has changed.
-    if (last_session_info_->timestamp > timestamp_) {
-      timestamp_ =
-          last_session_info_->timestamp + base::TimeDelta::FromMicroseconds(1);
-      SetPath(FilePathFromTime(type_, base_dir_, timestamp_));
-    }
-  }
-
-  // Best effort delete all sessions except the current & last.
-  DeleteLastSessionFiles();
-
-  // Create and open the file for the current session.
-  DCHECK(!base::PathExists(path()));
-  TruncateFile();
-}
-
-void SnapshottingCommandStorageBackend::DetermineLastSessionFile() {
-  last_session_info_.reset();
-
-  // Determine the session with the most recent timestamp that
-  // does not match the current session path.
-  for (const SnapshottingCommandStorageBackend::SessionInfo& session :
-       GetSessionFiles()) {
-    if (session.path != path()) {
-      if (!last_session_info_ ||
-          session.timestamp > last_session_info_->timestamp)
-        last_session_info_ = session;
-    }
-  }
-
-  // If no last session was found, use the legacy session if present.
-  // The legacy session is considered to have a timestamp of 0, before any
-  // new session.
-  if (!last_session_info_) {
-    base::FilePath legacy_session =
-        GetLegacySessionPath(type_, base_dir_, true);
-    if (base::PathExists(legacy_session))
-      last_session_info_ = SessionInfo{legacy_session, base::Time()};
-  }
-}
-
-void SnapshottingCommandStorageBackend::DeleteLastSessionFiles() {
-  // Delete session files whose paths do not match the current
-  // or last session path.
-  for (const SnapshottingCommandStorageBackend::SessionInfo& session :
-       GetSessionFiles()) {
-    if (session.path != path() &&
-        (!last_session_info_ || session.path != last_session_info_->path)) {
-      base::DeleteFile(session.path);
-    }
-  }
-
-  // Delete legacy session files, unless they are being used.
-  base::FilePath current_session_path =
-      GetLegacySessionPath(type_, base_dir_, true);
-  if (last_session_info_ && current_session_path != last_session_info_->path &&
-      base::PathExists(current_session_path))
-    base::DeleteFile(current_session_path);
-
-  base::FilePath last_session_path =
-      GetLegacySessionPath(type_, base_dir_, false);
-  if (base::PathExists(last_session_path))
-    base::DeleteFile(last_session_path);
-}
-
-std::vector<SnapshottingCommandStorageBackend::SessionInfo>
-SnapshottingCommandStorageBackend::GetSessionFiles() {
-  std::vector<SnapshottingCommandStorageBackend::SessionInfo> sessions;
-  base::FileEnumerator file_enum(
-      base_dir_.Append(kSessionsDirectory), false, base::FileEnumerator::FILES,
-      GetSessionFilename(type_, FILE_PATH_LITERAL("*")));
-  for (base::FilePath name = file_enum.Next(); !name.empty();
-       name = file_enum.Next()) {
-    base::Time file_time;
-    if (TimestampFromPath(name, file_time))
-      sessions.push_back(SessionInfo{name, file_time});
-  }
-  return sessions;
-}
-
-SnapshottingCommandStorageBackend::~SnapshottingCommandStorageBackend() =
-    default;
-
-}  // namespace sessions

components/sessions/core/snapshotting_command_storage_backend.h

-// Copyright 2020 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 COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_BACKEND_H_
-#define COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_BACKEND_H_
-
-#include <stddef.h>
-
-#include <memory>
-#include <vector>
-
-#include "components/sessions/core/command_storage_backend.h"
-#include "components/sessions/core/sessions_export.h"
-#include "components/sessions/core/snapshotting_command_storage_manager.h"
-
-namespace base {
-class FilePath;
-}
-
-namespace sessions {
-
-// Adds the ability to snapshot the current session file. The snapshot is
-// referred to as 'last'
-class SESSIONS_EXPORT SnapshottingCommandStorageBackend
-    : public CommandStorageBackend {
- public:
-  // Creates a CommandStorageBackend. This method is invoked on the MAIN thread,
-  // and does no IO. The real work is done from Init, which is invoked on
-  // a background task runer.
-  //
-  // |path_to_dir| gives the path the files are written two, and |type|
-  // indicates which service is using this backend. |type| is used to determine
-  // the name of the files to use as well as for logging.
-  SnapshottingCommandStorageBackend(
-      scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
-      SnapshottingCommandStorageManager::SessionType type,
-      const base::FilePath& path_to_dir);
-
-  SnapshottingCommandStorageBackend(const SnapshottingCommandStorageBackend&) =
-      delete;
-  SnapshottingCommandStorageBackend& operator=(
-      const SnapshottingCommandStorageBackend&) = delete;
-
-  // Parses out the timestamp from a path pointing to a session file.
-  static bool TimestampFromPath(const base::FilePath& path, base::Time& result);
-
-  // Generates the path to a session file with the given timestamp.
-  static base::FilePath FilePathFromTime(
-      const SnapshottingCommandStorageManager::SessionType type,
-      const base::FilePath& base_path,
-      base::Time time);
-
-  // Returns the commands from the last session file.
-  std::vector<std::unique_ptr<SessionCommand>> ReadLastSessionCommands();
-
-  // Deletes the file containing the commands for the last session.
-  void DeleteLastSession();
-
-  // Moves the current session file to the last session file. This is typically
-  // called during startup or if the user launches the app and no tabbed
-  // browsers are running.
-  void MoveCurrentSessionToLastSession();
-
- protected:
-  void DoInit() override;
-
- private:
-  friend class base::RefCountedDeleteOnSequence<
-      SnapshottingCommandStorageBackend>;
-  friend class base::DeleteHelper<SnapshottingCommandStorageBackend>;
-
-  FRIEND_TEST_ALL_PREFIXES(SnapshottingCommandStorageBackendTest,
-                           ReadLegacySession);
-  FRIEND_TEST_ALL_PREFIXES(SnapshottingCommandStorageBackendTest,
-                           DeterminePreviousSessionEmpty);
-  FRIEND_TEST_ALL_PREFIXES(SnapshottingCommandStorageBackendTest,
-                           DeterminePreviousSessionSingle);
-  FRIEND_TEST_ALL_PREFIXES(SnapshottingCommandStorageBackendTest,
-                           DeterminePreviousSessionMultiple);
-  FRIEND_TEST_ALL_PREFIXES(SnapshottingCommandStorageBackendTest,
-                           DeterminePreviousSessionInvalid);
-
-  ~SnapshottingCommandStorageBackend() override;
-
-  // Represents data for a session.
-  struct SessionInfo {
-    base::FilePath path;
-    base::Time timestamp;
-  };
-
-  // Gets data for the last session file.
-  void DetermineLastSessionFile();
-
-  // Attempt to delete all sessions besides the current and last. This is a
-  // best effort operation.
-  void DeleteLastSessionFiles();
-
-  // Gets all sessions files.
-  std::vector<SessionInfo> GetSessionFiles();
-
-  // Timestamp when this session was started.
-  base::Time timestamp_;
-
-  // Directory files are relative to.
-  const base::FilePath base_dir_;
-
-  // Data for the last session. If unset, fallback to legacy session data.
-  base::Optional<SessionInfo> last_session_info_;
-
-  const SnapshottingCommandStorageManager::SessionType type_;
-};
-
-}  // namespace sessions
-
-#endif  // COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_BACKEND_H_

components/sessions/core/snapshotting_command_storage_manager.cc

-// Copyright 2020 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 "components/sessions/core/snapshotting_command_storage_manager.h"
-
-#include <utility>
-
-#include "base/bind.h"
-#include "base/location.h"
-#include "base/memory/scoped_refptr.h"
-#include "base/sequenced_task_runner.h"
-#include "base/task/post_task.h"
-#include "components/sessions/core/snapshotting_command_storage_backend.h"
-
-namespace sessions {
-
-SnapshottingCommandStorageManager::SnapshottingCommandStorageManager(
-    SessionType type,
-    const base::FilePath& path,
-    CommandStorageManagerDelegate* delegate)
-    : CommandStorageManager(
-          base::MakeRefCounted<SnapshottingCommandStorageBackend>(
-              CreateDefaultBackendTaskRunner(),
-              type,
-              path),
-          delegate) {}
-
-SnapshottingCommandStorageManager::~SnapshottingCommandStorageManager() =
-    default;
-
-void SnapshottingCommandStorageManager::MoveCurrentSessionToLastSession() {
-  Save();
-  backend_task_runner()->PostNonNestableTask(
-      FROM_HERE,
-      base::BindOnce(
-          &SnapshottingCommandStorageBackend::MoveCurrentSessionToLastSession,
-          GetSnapshottingBackend()));
-}
-
-void SnapshottingCommandStorageManager::DeleteLastSession() {
-  backend_task_runner()->PostNonNestableTask(
-      FROM_HERE,
-      base::BindOnce(&SnapshottingCommandStorageBackend::DeleteLastSession,
-                     GetSnapshottingBackend()));
-}
-
-void SnapshottingCommandStorageManager::GetLastSessionCommands(
-    GetCommandsCallback callback) {
-  backend_task_runner()->PostTaskAndReplyWithResult(
-      FROM_HERE,
-      base::BindOnce(
-          &SnapshottingCommandStorageBackend::ReadLastSessionCommands,
-          GetSnapshottingBackend()),
-      std::move(callback));
-}
-
-SnapshottingCommandStorageBackend*
-SnapshottingCommandStorageManager::GetSnapshottingBackend() {
-  return static_cast<SnapshottingCommandStorageBackend*>(backend());
-}
-
-}  // namespace sessions

components/sessions/core/snapshotting_command_storage_manager.h

-// Copyright 2020 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 COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_MANAGER_H_
-#define COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_MANAGER_H_
-
-#include <memory>
-
-#include "components/sessions/core/command_storage_manager.h"
-#include "components/sessions/core/sessions_export.h"
-
-namespace base {
-class FilePath;
-}  // namespace base
-
-namespace sessions {
-class CommandStorageManagerDelegate;
-class SnapshottingCommandStorageBackend;
-
-// Adds snapshotting to CommandStorageManager. In this context a snapshot refers
-// to a copy of the current session file (in code, the snapshot is referred to
-// as the last session file). A snapshot is made at startup, but may also occur
-// at other points. For example, when Chrome transitions from no tabbed browsers
-// to having tabbed browsers a snapshot is made.
-class SESSIONS_EXPORT SnapshottingCommandStorageManager
-    : public CommandStorageManager {
- public:
-  // Identifies the type of session service this is. This is used by the
-  // backend to determine the name of the files.
-  enum SessionType { SESSION_RESTORE, TAB_RESTORE };
-
-  SnapshottingCommandStorageManager(SessionType type,
-                                    const base::FilePath& path,
-                                    CommandStorageManagerDelegate* delegate);
-
-  SnapshottingCommandStorageManager(const SnapshottingCommandStorageManager&) =
-      delete;
-  SnapshottingCommandStorageManager& operator=(
-      const SnapshottingCommandStorageManager&) = delete;
-
-  ~SnapshottingCommandStorageManager() override;
-
-  // Moves the current session to the last session.
-  void MoveCurrentSessionToLastSession();
-
-  // Deletes the last session.
-  void DeleteLastSession();
-
-  // Uses the backend to load the last session commands from disk. |callback|
-  // is called once the data has arrived, and may be called after this is
-  // deleted.
-  void GetLastSessionCommands(GetCommandsCallback callback);
-
- private:
-  SnapshottingCommandStorageBackend* GetSnapshottingBackend();
-};
-
-}  // namespace sessions
-
-#endif  // COMPONENTS_SESSIONS_CORE_SNAPSHOTTING_COMMAND_STORAGE_MANAGER_H_

components/sessions/core/tab_restore_service_impl.cc

@@ -24,11 +24,11 @@
 #include "components/history/core/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/sessions/core/base_session_service_commands.h"
+#include "components/sessions/core/command_storage_manager.h"
 #include "components/sessions/core/command_storage_manager_delegate.h"
 #include "components/sessions/core/session_command.h"
 #include "components/sessions/core/session_constants.h"
 #include "components/sessions/core/session_id.h"
-#include "components/sessions/core/snapshotting_command_storage_manager.h"
 #include "components/tab_groups/tab_group_color.h"
 #include "components/tab_groups/tab_group_id.h"
 #include "components/tab_groups/tab_group_visual_data.h"
@@ -461,7 +461,7 @@ class TabRestoreServiceImpl::PersistenceDelegate
   // The associated client.
   TabRestoreServiceClient* client_;
 
-  std::unique_ptr<SnapshottingCommandStorageManager> command_storage_manager_;
+  std::unique_ptr<CommandStorageManager> command_storage_manager_;
 
   TabRestoreServiceHelper* tab_restore_service_helper_;
 
@@ -488,9 +488,8 @@ class TabRestoreServiceImpl::PersistenceDelegate
 TabRestoreServiceImpl::PersistenceDelegate::PersistenceDelegate(
     TabRestoreServiceClient* client)
     : client_(client),
-      command_storage_manager_(
-          std::make_unique<SnapshottingCommandStorageManager>(
-              SnapshottingCommandStorageManager::TAB_RESTORE,
+      command_storage_manager_(std::make_unique<CommandStorageManager>(
+          CommandStorageManager::kTabRestore,
           client_->GetPathToSaveTo(),
           this)),
       tab_restore_service_helper_(nullptr),

weblayer/browser/persistence/browser_persister.cc

@@ -58,6 +58,7 @@ BrowserPersister::BrowserPersister(const base::FilePath& path,
       browser_session_id_(SessionID::NewUnique()),
       command_storage_manager_(
           std::make_unique<sessions::CommandStorageManager>(
+              sessions::CommandStorageManager::kOther,
               path,
               this,
               browser->profile()->GetBrowserContext()->IsOffTheRecord())),