sessions: remove support for disabling use_marker

Now that all code has been converted to use_marker, the old path
can be removed.

BUG=648266
TEST=covered by tests

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

chrome/browser/sessions/session_service_base.cc

@@ -144,7 +144,6 @@ SessionServiceBase::SessionServiceBase(Profile* profile,
 
   command_storage_manager_ = std::make_unique<sessions::CommandStorageManager>(
       backend_type, profile->GetPath(), this,
-      /* use_marker */ true,
       /* enable_crypto */ false, std::vector<uint8_t>(),
       TaskRunnerData::GetBackendTaskRunnerForProfile(profile, type));
 

components/sessions/core/command_storage_backend.cc

@@ -30,6 +30,8 @@ using SessionType = CommandStorageManager::SessionType;
 namespace {
 
 // File version number.
+// TODO(sky): remove these in ~1 year. They are no longer written as of
+// ~5/2021.
 constexpr int32_t kFileVersion1 = 1;
 constexpr int32_t kEncryptedFileVersion = 2;
 // The versions that are used if `use_marker` is true.
@@ -430,12 +432,10 @@ CommandStorageBackend::CommandStorageBackend(
     scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
     const base::FilePath& path,
     SessionType type,
-    bool use_marker,
     const std::vector<uint8_t>& decryption_key)
     : RefCountedDeleteOnSequence(owning_task_runner),
       type_(type),
       supplied_path_(path),
-      use_marker_(use_marker),
       initial_decryption_key_(decryption_key),
       callback_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
   // This is invoked on the main thread, don't do file access here.
@@ -459,22 +459,16 @@ void CommandStorageBackend::AppendCommands(
     DCHECK_NE(kInitialStateMarkerCommandId, command->id());
 #endif
 
-  // If `use_marker` is true, the consumer must call this with `truncate` set
-  // to true to indicate the initial state has been supplied. To do otherwise
-  // would mean the file never contains the marker, and would not be considered
+  // The consumer must call this with `truncate` set to true to indicate the
+  // initial state has been supplied. To do otherwise would mean the file never
+  // contains the marker, and would not be considered
   // valid. This includes first time through.
-  if (use_marker_ && !truncate && (!file_ || !file_->IsValid()))
+  if (!truncate && (!file_ || !file_->IsValid()))
     return;
 
   if (truncate) {
-    const bool was_encrypted = IsEncrypted();
+    CloseFile();
     const bool encrypt = !crypto_key.empty();
-
-    // The header is different when encrypting, so the file needs to be
-    // recreated.
-    if (use_marker_ || (was_encrypted != encrypt))
-      CloseFile();
-
     if (encrypt) {
       aead_ = std::make_unique<crypto::Aead>(crypto::Aead::AES_256_GCM);
       crypto_key_ = crypto_key;
@@ -482,10 +476,8 @@ void CommandStorageBackend::AppendCommands(
     } else {
       aead_.reset();
     }
-    if (use_marker_) {
-      commands.push_back(
-          std::make_unique<SessionCommand>(kInitialStateMarkerCommandId, 0));
-    }
+    commands.push_back(
+        std::make_unique<SessionCommand>(kInitialStateMarkerCommandId, 0));
   } else {
     // |crypto_key| is only used when |truncate| is true.
     DCHECK(crypto_key.empty());
@@ -502,7 +494,7 @@ void CommandStorageBackend::AppendCommands(
     CloseFile();
   }
 
-  if (use_marker_ && truncate && file_ && file_->IsValid()) {
+  if (truncate && file_ && file_->IsValid()) {
     did_write_marker_ = true;
     if (last_file_with_valid_marker_) {
       DCHECK_NE(*last_file_with_valid_marker_, current_path_);
@@ -573,14 +565,10 @@ void CommandStorageBackend::MoveCurrentSessionToLastSession() {
 
   // Move current session to last.
   absl::optional<SessionInfo> new_last_session_info;
-  if (use_marker_) {
-    if (last_file_with_valid_marker_) {
-      new_last_session_info =
-          SessionInfo{*last_file_with_valid_marker_, timestamp_};
-      last_file_with_valid_marker_.reset();
-    }
-  } else if (base::PathExists(current_path_)) {
-    new_last_session_info = SessionInfo{current_path_, timestamp_};
+  if (last_file_with_valid_marker_) {
+    new_last_session_info =
+        SessionInfo{*last_file_with_valid_marker_, timestamp_};
+    last_file_with_valid_marker_.reset();
   }
   last_session_info_ = new_last_session_info;
 
@@ -624,7 +612,7 @@ void CommandStorageBackend::InitIfNecessary() {
   inited_ = true;
   base::CreateDirectory(GetSessionDirName(type_, supplied_path_));
 
-  // TODO(sky): with `use_marker_` this is expensive. See if it can be delayed.
+  // TODO(sky): this is expensive. See if it can be delayed.
   last_session_info_ = FindLastSessionFile();
 
   // Best effort delete all sessions except the current & last.
@@ -652,47 +640,30 @@ void CommandStorageBackend::CloseFile() {
   file_.reset();
 
   // If a marker wasn't written, no need to keep the current file.
-  if (use_marker_ && !did_write_marker_ && !current_path_.empty())
+  if (!did_write_marker_ && !current_path_.empty())
     base::DeleteFile(current_path_);
 }
 
 void CommandStorageBackend::TruncateOrOpenFile() {
   DCHECK(inited_);
-  if (use_marker_)
-    CloseFile();
-  if (use_marker_ || current_path_.empty()) {
-    DCHECK(!file_);
-    base::Time new_timestamp = base::Time::Now();
-    // Ensure we don't reuse the current file (this is extremely unlikely to
-    // ever be true).
-    if (new_timestamp == timestamp_)
-      new_timestamp += base::TimeDelta::FromMicroseconds(1);
-    if (last_session_info_) {
-      // Ensure 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 > new_timestamp) {
-        new_timestamp = last_session_info_->timestamp +
-                        base::TimeDelta::FromMicroseconds(1);
-      }
+  CloseFile();
+  DCHECK(!file_);
+  base::Time new_timestamp = base::Time::Now();
+  // Ensure we don't reuse the current file (this is extremely unlikely to
+  // ever be true).
+  if (new_timestamp == timestamp_)
+    new_timestamp += base::TimeDelta::FromMicroseconds(1);
+  if (last_session_info_) {
+    // Ensure 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 > new_timestamp) {
+      new_timestamp =
+          last_session_info_->timestamp + base::TimeDelta::FromMicroseconds(1);
     }
-    timestamp_ = new_timestamp;
-    current_path_ = FilePathFromTime(type_, supplied_path_, timestamp_);
   }
-  if (file_) {
-    // If `use_marker_` is true, the file is always closed before being
-    // truncated.
-    DCHECK(!use_marker_);
-    // File is already open, truncate it. We truncate instead of closing and
-    // reopening to avoid the possibility of scanners locking the file out
-    // from under us once we close it. If truncation fails, we'll try to
-    // recreate.
-    const int header_size = static_cast<int>(sizeof(FileHeader));
-    if (file_->Seek(base::File::FROM_BEGIN, header_size) != header_size ||
-        !file_->SetLength(header_size))
-      file_.reset();
-  }
-  if (!file_)
-    file_ = OpenAndWriteHeader(current_path_);
+  timestamp_ = new_timestamp;
+  current_path_ = FilePathFromTime(type_, supplied_path_, timestamp_);
+  file_ = OpenAndWriteHeader(current_path_);
   commands_written_ = 0;
   did_write_marker_ = false;
 }
@@ -708,12 +679,8 @@ std::unique_ptr<base::File> CommandStorageBackend::OpenAndWriteHeader(
     return nullptr;
   FileHeader header;
   header.signature = kFileSignature;
-  if (use_marker_) {
-    header.version = IsEncrypted() ? kEncryptedFileVersionWithMarker
-                                   : kFileVersionWithMarker;
-  } else {
-    header.version = IsEncrypted() ? kEncryptedFileVersion : kFileVersion1;
-  }
+  header.version =
+      IsEncrypted() ? kEncryptedFileVersionWithMarker : kFileVersionWithMarker;
   if (file->WriteAtCurrentPos(reinterpret_cast<char*>(&header),
                               sizeof(header)) != sizeof(header)) {
     return nullptr;
@@ -866,9 +833,6 @@ CommandStorageBackend::GetSessionFilesSortedByReverseTimestamp(
 
 bool CommandStorageBackend::CanUseFileForLastSession(
     const base::FilePath& path) const {
-  if (!use_marker_)
-    return true;
-
   const SessionFileReader::MarkerStatus status =
       SessionFileReader::GetMarkerStatus(path, initial_decryption_key_);
   return !status.supports_marker || status.has_marker;

components/sessions/core/command_storage_backend.h

@@ -34,8 +34,6 @@ namespace sessions {
 // CommandStorageBackend (mostly) does not interpret the commands in any way, it
 // simply reads/writes them.
 //
-// The following comment applies when `use_marker` is true, which will
-// eventually be the default (and there will not be an option to disable it).
 // CommandStorageBackend writes to a file with a suffix that indicates the
 // time the file was opened. The time stamp allows this code to determine the
 // most recently written file. When AppendCommands() is supplied a value of true
@@ -89,7 +87,6 @@ class SESSIONS_EXPORT CommandStorageBackend
       scoped_refptr<base::SequencedTaskRunner> owning_task_runner,
       const base::FilePath& path,
       CommandStorageManager::SessionType type,
-      bool use_marker,
       const std::vector<uint8_t>& decryption_key = {});
   CommandStorageBackend(const CommandStorageBackend&) = delete;
   CommandStorageBackend& operator=(const CommandStorageBackend&) = delete;
@@ -235,8 +232,6 @@ class SESSIONS_EXPORT CommandStorageBackend
   // constructor for details.
   const base::FilePath supplied_path_;
 
-  const bool use_marker_;
-
   // Used to decode the initial last session file.
   // TODO(sky): this is currently required because InitIfNecessary() determines
   // the last file. If that can be delayed, then this can be supplied to

components/sessions/core/command_storage_backend_unittest.cc

@@ -50,7 +50,7 @@ std::unique_ptr<SessionCommand> CreateCommandFromData(const TestData& data) {
 
 }  // namespace
 
-class CommandStorageBackendTest : public testing::TestWithParam<bool> {
+class CommandStorageBackendTest : public testing::Test {
  protected:
   // testing::TestWithParam:
   void SetUp() override {
@@ -83,16 +83,14 @@ class CommandStorageBackendTest : public testing::TestWithParam<bool> {
       const std::vector<uint8_t>& decryption_key = {}) {
     return MakeRefCounted<CommandStorageBackend>(
         task_environment_.GetMainThreadTaskRunner(), file_path_,
-        CommandStorageManager::SessionType::kOther, IsUsingMarker(),
-        decryption_key);
+        CommandStorageManager::SessionType::kOther, decryption_key);
   }
 
   scoped_refptr<CommandStorageBackend> CreateBackendWithRestoreType() {
     const CommandStorageManager::SessionType type =
         CommandStorageManager::SessionType::kSessionRestore;
     return MakeRefCounted<CommandStorageBackend>(
-        task_environment_.GetMainThreadTaskRunner(), restore_path_, type,
-        IsUsingMarker());
+        task_environment_.GetMainThreadTaskRunner(), restore_path_, type);
   }
 
   // Functions that call into private members of CommandStorageBackend.
@@ -112,8 +110,6 @@ class CommandStorageBackendTest : public testing::TestWithParam<bool> {
     return result;
   }
 
-  bool IsUsingMarker() const { return GetParam(); }
-
   static base::FilePath FilePathFromTime(
       CommandStorageManager::SessionType type,
       const base::FilePath& path,
@@ -134,7 +130,7 @@ class CommandStorageBackendTest : public testing::TestWithParam<bool> {
   base::ScopedTempDir temp_dir_;
 };
 
-TEST_P(CommandStorageBackendTest, MigrateOther) {
+TEST_F(CommandStorageBackendTest, MigrateOther) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   struct TestData data = {1, "a"};
   SessionCommands commands;
@@ -173,7 +169,7 @@ TEST_P(CommandStorageBackendTest, MigrateOther) {
   AssertCommandEqualsData(data2, commands[0].get());
 }
 
-TEST_P(CommandStorageBackendTest, SimpleReadWriteEncrypted) {
+TEST_F(CommandStorageBackendTest, SimpleReadWriteEncrypted) {
   std::vector<uint8_t> key = CommandStorageManager::CreateCryptoKey();
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   struct TestData data = {1, "a"};
@@ -197,7 +193,7 @@ TEST_P(CommandStorageBackendTest, SimpleReadWriteEncrypted) {
   EXPECT_TRUE(commands.empty());
 }
 
-TEST_P(CommandStorageBackendTest, RandomDataEncrypted) {
+TEST_F(CommandStorageBackendTest, RandomDataEncrypted) {
   struct TestData data[] = {
       {1, "a"},
       {2, "ab"},
@@ -235,7 +231,7 @@ TEST_P(CommandStorageBackendTest, RandomDataEncrypted) {
   }
 }
 
-TEST_P(CommandStorageBackendTest, BigDataEncrypted) {
+TEST_F(CommandStorageBackendTest, BigDataEncrypted) {
   struct TestData data[] = {
       {1, "a"},
       {2, "ab"},
@@ -272,7 +268,7 @@ TEST_P(CommandStorageBackendTest, BigDataEncrypted) {
             reinterpret_cast<char*>(commands[1]->contents())[big_size - 1]);
 }
 
-TEST_P(CommandStorageBackendTest, MarkerOnlyEncrypted) {
+TEST_F(CommandStorageBackendTest, MarkerOnlyEncrypted) {
   std::vector<uint8_t> key = CommandStorageManager::CreateCryptoKey();
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   SessionCommands commands;
@@ -288,7 +284,7 @@ TEST_P(CommandStorageBackendTest, MarkerOnlyEncrypted) {
 
 // Writes a command, appends another command with reset to true, then reads
 // making sure we only get back the second command.
-TEST_P(CommandStorageBackendTest, TruncateEncrypted) {
+TEST_F(CommandStorageBackendTest, TruncateEncrypted) {
   std::vector<uint8_t> key = CommandStorageManager::CreateCryptoKey();
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   struct TestData first_data = {1, "a"};
@@ -323,7 +319,7 @@ std::unique_ptr<SessionCommand> CreateCommandWithMaxSize() {
   return command;
 }
 
-TEST_P(CommandStorageBackendTest, MaxSizeTypeEncrypted) {
+TEST_F(CommandStorageBackendTest, MaxSizeTypeEncrypted) {
   std::vector<uint8_t> key = CommandStorageManager::CreateCryptoKey();
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
 
@@ -349,7 +345,7 @@ TEST_P(CommandStorageBackendTest, MaxSizeTypeEncrypted) {
                      expected_size) == 0);
 }
 
-TEST_P(CommandStorageBackendTest, MaxSizeType) {
+TEST_F(CommandStorageBackendTest, MaxSizeType) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
 
   SessionCommands commands;
@@ -371,7 +367,7 @@ TEST_P(CommandStorageBackendTest, MaxSizeType) {
                      expected_size) == 0);
 }
 
-TEST_P(CommandStorageBackendTest, IsValidFileWithInvalidFiles) {
+TEST_F(CommandStorageBackendTest, IsValidFileWithInvalidFiles) {
   base::WriteFile(file_path(), "z");
   EXPECT_FALSE(CommandStorageBackend::IsValidFile(file_path()));
 
@@ -379,11 +375,7 @@ TEST_P(CommandStorageBackendTest, IsValidFileWithInvalidFiles) {
   EXPECT_FALSE(CommandStorageBackend::IsValidFile(file_path()));
 }
 
-TEST_P(CommandStorageBackendTest, IsNotValidFileWithoutMarker) {
-  // This test is only applicable when the marker is used.
-  if (!IsUsingMarker())
-    return;
-
+TEST_F(CommandStorageBackendTest, IsNotValidFileWithoutMarker) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   const auto path = backend->current_path();
   backend->AppendCommands({}, true, base::DoNothing());
@@ -392,20 +384,7 @@ TEST_P(CommandStorageBackendTest, IsNotValidFileWithoutMarker) {
   EXPECT_FALSE(CommandStorageBackend::IsValidFile(path));
 }
 
-TEST_P(CommandStorageBackendTest, IsValidFileWithValidFile) {
-  // This test is only applicable when the marker is not used.
-  if (IsUsingMarker())
-    return;
-
-  scoped_refptr<CommandStorageBackend> backend = CreateBackend();
-  backend->AppendCommands({}, true, base::DoNothing());
-  const auto path = backend->current_path();
-  backend = nullptr;
-
-  EXPECT_TRUE(CommandStorageBackend::IsValidFile(path));
-}
-
-TEST_P(CommandStorageBackendTest, SimpleReadWriteWithRestoreType) {
+TEST_F(CommandStorageBackendTest, SimpleReadWriteWithRestoreType) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackendWithRestoreType();
   struct TestData data = {1, "a"};
   SessionCommands commands;
@@ -434,7 +413,7 @@ TEST_P(CommandStorageBackendTest, SimpleReadWriteWithRestoreType) {
   ASSERT_EQ(0U, commands.size());
 }
 
-TEST_P(CommandStorageBackendTest, RandomDataWithRestoreType) {
+TEST_F(CommandStorageBackendTest, RandomDataWithRestoreType) {
   struct TestData data[] = {
       {1, "a"},
       {2, "ab"},
@@ -471,7 +450,7 @@ TEST_P(CommandStorageBackendTest, RandomDataWithRestoreType) {
   }
 }
 
-TEST_P(CommandStorageBackendTest, BigDataWithRestoreType) {
+TEST_F(CommandStorageBackendTest, BigDataWithRestoreType) {
   struct TestData data[] = {
       {1, "a"},
       {2, "ab"},
@@ -507,7 +486,7 @@ TEST_P(CommandStorageBackendTest, BigDataWithRestoreType) {
             reinterpret_cast<char*>(commands[1]->contents())[big_size - 1]);
 }
 
-TEST_P(CommandStorageBackendTest, CommandWithRestoreType) {
+TEST_F(CommandStorageBackendTest, CommandWithRestoreType) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackendWithRestoreType();
   SessionCommands commands;
   backend->AppendCommands(std::move(commands), true, base::DoNothing());
@@ -519,7 +498,7 @@ TEST_P(CommandStorageBackendTest, CommandWithRestoreType) {
 
 // Writes a command, appends another command with reset to true, then reads
 // making sure we only get back the second command.
-TEST_P(CommandStorageBackendTest, TruncateWithRestoreType) {
+TEST_F(CommandStorageBackendTest, TruncateWithRestoreType) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackendWithRestoreType();
   struct TestData first_data = {1, "a"};
   SessionCommands commands;
@@ -543,7 +522,7 @@ TEST_P(CommandStorageBackendTest, TruncateWithRestoreType) {
 }
 
 // Test parsing the timestamp of a session from the path.
-TEST_P(CommandStorageBackendTest, TimestampFromPathWithRestoreType) {
+TEST_F(CommandStorageBackendTest, TimestampFromPathWithRestoreType) {
   const auto base_dir = base::FilePath(kSessionsDirectory);
 
   // Test parsing the timestamp from a valid session.
@@ -576,7 +555,7 @@ TEST_P(CommandStorageBackendTest, TimestampFromPathWithRestoreType) {
 }
 
 // Test serializing a timestamp to string.
-TEST_P(CommandStorageBackendTest, FilePathFromTimeWithRestoreType) {
+TEST_F(CommandStorageBackendTest, FilePathFromTimeWithRestoreType) {
   const auto base_dir = base::FilePath(kSessionsDirectory);
   const auto test_time_1 = base::Time();
   const auto result_path_1 =
@@ -594,14 +573,14 @@ TEST_P(CommandStorageBackendTest, FilePathFromTimeWithRestoreType) {
 }
 
 // Test that the previous session is empty if no session files exist.
-TEST_P(CommandStorageBackendTest,
+TEST_F(CommandStorageBackendTest,
        DeterminePreviousSessionEmptyWithRestoreType) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackendWithRestoreType();
   ASSERT_FALSE(GetLastSessionInfo(backend.get()));
 }
 
 // Test that the previous session is selected correctly when a file is present.
-TEST_P(CommandStorageBackendTest,
+TEST_F(CommandStorageBackendTest,
        DeterminePreviousSessionSingleWithRestoreType) {
   const auto prev_path = restore_path().Append(
       base::FilePath(kSessionsDirectory)
@@ -617,7 +596,7 @@ TEST_P(CommandStorageBackendTest,
 
 // Test that the previous session is selected correctly when multiple session
 // files are present.
-TEST_P(CommandStorageBackendTest,
+TEST_F(CommandStorageBackendTest,
        DeterminePreviousSessionMultipleWithRestoreType) {
   const auto sessions_dir =
       restore_path().Append(base::FilePath(kSessionsDirectory));
@@ -638,7 +617,7 @@ TEST_P(CommandStorageBackendTest,
 }
 
 // Test that the a file with an invalid name won't be used.
-TEST_P(CommandStorageBackendTest,
+TEST_F(CommandStorageBackendTest,
        DeterminePreviousSessionInvalidWithRestoreType) {
   const auto prev_path =
       restore_path().Append(base::FilePath(kSessionsDirectory)
@@ -652,7 +631,7 @@ TEST_P(CommandStorageBackendTest,
 }
 
 // Tests that MoveCurrentSessionToLastSession deletes the last session file.
-TEST_P(CommandStorageBackendTest,
+TEST_F(CommandStorageBackendTest,
        MoveCurrentSessionToLastDeletesLastSessionWithRestoreType) {
   const auto sessions_dir =
       restore_path().Append(base::FilePath(kSessionsDirectory));
@@ -668,7 +647,7 @@ TEST_P(CommandStorageBackendTest,
   ASSERT_EQ(-1, base::ReadFile(last_session, buffer, 0));
 }
 
-TEST_P(CommandStorageBackendTest, GetSessionFiles) {
+TEST_F(CommandStorageBackendTest, GetSessionFiles) {
   EXPECT_TRUE(CommandStorageBackend::GetSessionFilePaths(
                   file_path(), CommandStorageManager::kOther)
                   .empty());
@@ -688,12 +667,12 @@ TEST_P(CommandStorageBackendTest, GetSessionFiles) {
   EXPECT_EQ("Session_124", paths.begin()->BaseName().MaybeAsASCII());
 }
 
-TEST_P(CommandStorageBackendTest, TimestampSeparatorIsAscii) {
+TEST_F(CommandStorageBackendTest, TimestampSeparatorIsAscii) {
   // Code in WebLayer relies on the timestamp separator being ascii.
   ASSERT_TRUE(!base::FilePath(kTimestampSeparator).MaybeAsASCII().empty());
 }
 
-TEST_P(CommandStorageBackendTest, GetSessionFilesAreSortedByReverseTimestamp) {
+TEST_F(CommandStorageBackendTest, GetSessionFilesAreSortedByReverseTimestamp) {
   ASSERT_EQ(0, base::WriteFile(file_path().DirName().AppendASCII("Session_130"),
                                "", 0));
   ASSERT_EQ(0, base::WriteFile(file_path().DirName().AppendASCII("Session_120"),
@@ -710,11 +689,7 @@ TEST_P(CommandStorageBackendTest, GetSessionFilesAreSortedByReverseTimestamp) {
   EXPECT_EQ("Session_120", paths[3].BaseName().MaybeAsASCII());
 }
 
-TEST_P(CommandStorageBackendTest, UseMarkerWithoutValidMarker) {
-  // This test is only interesting when the marker is used.
-  if (!IsUsingMarker())
-    return;
-
+TEST_F(CommandStorageBackendTest, UseMarkerWithoutValidMarker) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   struct TestData data = {1, "a"};
   SessionCommands commands;
@@ -735,7 +710,7 @@ TEST_P(CommandStorageBackendTest, UseMarkerWithoutValidMarker) {
 // This test moves a previously written file into the expected location and
 // ensures it's read. This is to verify reading hasn't changed in an
 // incompatible manner.
-TEST_P(CommandStorageBackendTest, ReadPreviouslyWrittenData) {
+TEST_F(CommandStorageBackendTest, ReadPreviouslyWrittenData) {
   base::FilePath test_data_path;
   ASSERT_TRUE(base::PathService::Get(base::DIR_SOURCE_ROOT, &test_data_path));
   test_data_path = test_data_path.AppendASCII("components")
@@ -766,11 +741,7 @@ TEST_P(CommandStorageBackendTest, ReadPreviouslyWrittenData) {
                            backend->ReadLastSessionCommands().commands);
 }
 
-TEST_P(CommandStorageBackendTest, NewFileOnTruncate) {
-  // This test is only applicable with marker.
-  if (!IsUsingMarker())
-    return;
-
+TEST_F(CommandStorageBackendTest, NewFileOnTruncate) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackendWithRestoreType();
   struct TestData data = {1, "a"};
   SessionCommands commands;
@@ -793,7 +764,7 @@ TEST_P(CommandStorageBackendTest, NewFileOnTruncate) {
   EXPECT_FALSE(base::PathExists(path1));
 }
 
-TEST_P(CommandStorageBackendTest, AppendCommandsCallbackRunOnError) {
+TEST_F(CommandStorageBackendTest, AppendCommandsCallbackRunOnError) {
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   backend->ForceAppendCommandsToFailForTesting();
   base::RunLoop run_loop;
@@ -801,11 +772,7 @@ TEST_P(CommandStorageBackendTest, AppendCommandsCallbackRunOnError) {
   run_loop.Run();
 }
 
-TEST_P(CommandStorageBackendTest, RestoresFileWithMarkerAfterFailure) {
-  // This test is only applicable with marker.
-  if (!IsUsingMarker())
-    return;
-
+TEST_F(CommandStorageBackendTest, RestoresFileWithMarkerAfterFailure) {
   // Write `data` and a marker.
   scoped_refptr<CommandStorageBackend> backend = CreateBackend();
   struct TestData data = {11, "X"};
@@ -835,8 +802,4 @@ TEST_P(CommandStorageBackendTest, RestoresFileWithMarkerAfterFailure) {
   AssertCommandEqualsData(data, commands[0].get());
 }
 
-INSTANTIATE_TEST_SUITE_P(,
-                         CommandStorageBackendTest,
-                         testing::Values(false, true));
-
 }  // namespace sessions

components/sessions/core/command_storage_manager.cc

@@ -36,7 +36,6 @@ CommandStorageManager::CommandStorageManager(
     SessionType type,
     const base::FilePath& path,
     CommandStorageManagerDelegate* delegate,
-    bool use_marker,
     bool enable_crypto,
     const std::vector<uint8_t>& decryption_key,
     scoped_refptr<base::SequencedTaskRunner> backend_task_runner)
@@ -45,10 +44,8 @@ CommandStorageManager::CommandStorageManager(
                               : CreateDefaultBackendTaskRunner(),
           path,
           type,
-          use_marker,
           decryption_key)),
       use_crypto_(enable_crypto),
-      pending_reset_(use_marker),
       delegate_(delegate),
       backend_task_runner_(backend_->owning_task_runner()) {}
 

components/sessions/core/command_storage_manager.h

@@ -54,13 +54,10 @@ class SESSIONS_EXPORT CommandStorageManager {
   // `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`.
-  //
-  // See CommandStorageBackend for details on `use_marker`.
   CommandStorageManager(
       SessionType type,
       const base::FilePath& path,
       CommandStorageManagerDelegate* delegate,
-      bool use_marker = false,
       bool enable_crypto = false,
       const std::vector<uint8_t>& decryption_key = {},
       scoped_refptr<base::SequencedTaskRunner> backend_task_runner = nullptr);
@@ -151,7 +148,7 @@ class SESSIONS_EXPORT CommandStorageManager {
 
   // Whether the backend file should be recreated the next time we send
   // over the commands.
-  bool pending_reset_;
+  bool pending_reset_ = true;
 
   // The number of commands sent to the backend before doing a reset.
   int commands_since_reset_ = 0;

components/sessions/core/command_storage_manager_unittest.cc

@@ -47,8 +47,8 @@ class TestCommandStorageManagerDelegate : public CommandStorageManagerDelegate {
 
 TEST_F(CommandStorageManagerTest, OnErrorWritingSessionCommands) {
   TestCommandStorageManagerDelegate delegate;
-  CommandStorageManager manager(CommandStorageManager::kOther, path_, &delegate,
-                                /* use_marker */ true);
+  CommandStorageManager manager(CommandStorageManager::kOther, path_,
+                                &delegate);
   CommandStorageManagerTestHelper test_helper(&manager);
   manager.set_pending_reset(true);
   // Write a command, the delegate should not be notified of an error.

components/sessions/core/tab_restore_service_impl.cc

@@ -512,8 +512,7 @@ TabRestoreServiceImpl::PersistenceDelegate::PersistenceDelegate(
       command_storage_manager_(std::make_unique<CommandStorageManager>(
           CommandStorageManager::kTabRestore,
           client_->GetPathToSaveTo(),
-          this,
-          /* use_marker */ true)),
+          this)),
       tab_restore_service_helper_(nullptr),
       entries_to_write_(0),
       entries_written_(0),

weblayer/browser/persistence/browser_persister.cc

@@ -61,7 +61,6 @@ BrowserPersister::BrowserPersister(const base::FilePath& path,
               sessions::CommandStorageManager::kOther,
               path,
               this,
-              /* use_marker */ true,
               browser->profile()->GetBrowserContext()->IsOffTheRecord(),
               decryption_key)),
       rebuild_on_next_save_(false),