Make sure new file is actually created.

Bug: b:165851833
Change-Id: Id4d65e2426663783d510a72a8a1f298462e3d2a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378655
Commit-Queue: Leonid Baraz <lbaraz@chromium.org>
Auto-Submit: Leonid Baraz <lbaraz@chromium.org>
Reviewed-by: Zach Trudo <zatrudo@google.com>
Cr-Commit-Position: refs/heads/master@{#802085}

chrome/browser/policy/messaging_layer/storage/storage_queue.cc

@@ -307,24 +307,29 @@ StatusOr<scoped_refptr<StorageQueue::SingleFile>> StorageQueue::AssignLastFile(
     // The last file will become too large, asynchronously close it and add
     // new.
     last_file->Close();
-    auto insert_result = files_.emplace(
-        next_seq_number_,
-        base::MakeRefCounted<SingleFile>(
-            options_.directory()
-                .Append(options_.file_prefix())
-                .AddExtensionASCII(base::NumberToString(next_seq_number_)),
-            /*size=*/0));
-    if (!insert_result.second) {
-      return Status(
-          error::ALREADY_EXISTS,
-          base::StrCat({"Sequence number already assigned: '",
-                        base::NumberToString(next_seq_number_), "'"}));
-    }
-    last_file = insert_result.first->second;
+    ASSIGN_OR_RETURN(last_file, OpenNewWriteableFile());
   }
   return last_file;
 }
 
+StatusOr<scoped_refptr<StorageQueue::SingleFile>>
+StorageQueue::OpenNewWriteableFile() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(storage_queue_sequence_checker_);
+  auto new_file = base::MakeRefCounted<SingleFile>(
+      options_.directory()
+          .Append(options_.file_prefix())
+          .AddExtensionASCII(base::NumberToString(next_seq_number_)),
+      /*size=*/0);
+  RETURN_IF_ERROR(new_file->Open(/*read_only=*/false));
+  auto insert_result = files_.emplace(next_seq_number_, new_file);
+  if (!insert_result.second) {
+    return Status(error::ALREADY_EXISTS,
+                  base::StrCat({"Sequence number already assigned: '",
+                                base::NumberToString(next_seq_number_), "'"}));
+  }
+  return new_file;
+}
+
 Status StorageQueue::WriteHeaderAndBlock(
     base::span<const uint8_t> data,
     scoped_refptr<StorageQueue::SingleFile> file) {
@@ -676,18 +681,7 @@ Status StorageQueue::SwitchLastFileIfNotEmpty() {
     return Status::StatusOK();  // Already empty.
   }
   files_.rbegin()->second->Close();
-  auto insert_result = files_.emplace(
-      next_seq_number_,
-      base::MakeRefCounted<SingleFile>(
-          options_.directory()
-              .Append(options_.file_prefix())
-              .AddExtensionASCII(base::NumberToString(next_seq_number_)),
-          /*size=*/0));
-  if (!insert_result.second) {
-    return Status(error::ALREADY_EXISTS,
-                  base::StrCat({"Sequence number already assigned: '",
-                                base::NumberToString(next_seq_number_), "'"}));
-  }
+  ASSIGN_OR_RETURN(scoped_refptr<SingleFile> last_file, OpenNewWriteableFile());
   return Status::StatusOK();
 }
 

chrome/browser/policy/messaging_layer/storage/storage_queue.h

@@ -262,6 +262,10 @@ class StorageQueue : public base::RefCountedThreadSafe<StorageQueue> {
   // file will be created and assigned to be the last one.
   StatusOr<scoped_refptr<SingleFile>> AssignLastFile(size_t size);
 
+  // Helper method for Write() and Read(): creates and opens a new empty
+  // writeable file, adding it to |files_|.
+  StatusOr<scoped_refptr<SingleFile>> OpenNewWriteableFile();
+
   // Helper method for Write(): composes record header and writes it to the
   // file, followed by data.
   Status WriteHeaderAndBlock(base::span<const uint8_t> data,