sessions: return read commands on read error

Prior to this patch if there was an error in reading the session
file nothing would be returned. This changes the code to return
what was parsed. The hope is that we are at least able to restore
some portion of the previous session rather than nothing.

BUG=648266
TEST=none

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

components/sessions/core/command_storage_backend.cc

@@ -132,9 +132,7 @@ class SessionFileReader {
   bool ReadToMarker();
 
   // Reads a single command, returning it. A return value of null indicates
-  // either there are no commands, or there was an error. Use errored_ to
-  // distinguish the two. If null is returned, and there is no error, it means
-  // the end of file was successfully reached.
+  // either there are no commands, or there was an error.
   std::unique_ptr<sessions::SessionCommand> ReadCommand();
 
   // Decrypts a previously encrypted command. Returns the new command on
@@ -149,15 +147,11 @@ class SessionFileReader {
 
   // Shifts the unused portion of buffer_ to the beginning and fills the
   // remaining portion with data from the file. Returns false if the buffer
-  // couldn't be filled. A return value of false only signals an error if
-  // errored_ is set to true.
+  // couldn't be filled or there was an error reading the file.
   bool FillBuffer();
 
   bool is_header_valid_ = false;
 
-  // Whether an error condition has been detected.
-  bool errored_ = false;
-
   // As we read from the file, data goes here.
   std::string buffer_;
 
@@ -193,14 +187,12 @@ SessionFileReader::Read() {
 
   std::vector<std::unique_ptr<sessions::SessionCommand>> read_commands;
   for (std::unique_ptr<sessions::SessionCommand> command = ReadCommand();
-       command && !errored_; command = ReadCommand()) {
+       command; command = ReadCommand()) {
     if (command->id() != kInitialStateMarkerCommandId)
       read_commands.push_back(std::move(command));
   }
-  // TODO(sky): remove `errored_`. It basically means there is an error reading,
-  // but I think we should continue in this case.
-  if (errored_)
-    return {};
+  // Even if there was an error the commands are returned. The hope is at least
+  // some portion of the previous session is restored.
   return read_commands;
 }
 
@@ -228,7 +220,7 @@ bool SessionFileReader::ReadToMarker() {
   // It's expected this is only called if the marker is supported.
   DCHECK(IsHeaderValid() && SupportsMarker());
   for (std::unique_ptr<sessions::SessionCommand> command = ReadCommand();
-       command && !errored_; command = ReadCommand()) {
+       command; command = ReadCommand()) {
     if (command->id() == kInitialStateMarkerCommandId)
       return true;
   }
@@ -337,7 +329,7 @@ bool SessionFileReader::FillBuffer() {
   int read_count =
       file_->ReadAtCurrentPos(&(buffer_[available_count_]), to_read);
   if (read_count < 0) {
-    errored_ = true;
+    // TODO(sky): communicate/log an error here.
     return false;
   }
   if (read_count == 0)