IndexedDB: Fixed support for empty blobs.

storage/browser/fileapi/file_writer_delegate.cc

Index: storage/browser/fileapi/file_writer_delegate.cc
diff --git a/storage/browser/fileapi/file_writer_delegate.cc b/storage/browser/fileapi/file_writer_delegate.cc
index abc9976e6d5be4ece24ee745ea86ab69464fb74d..99fbe9c9bbb4d8d4ddad67fb39ed8ef8c3a475e2 100644
--- a/storage/browser/fileapi/file_writer_delegate.cc
+++ b/storage/browser/fileapi/file_writer_delegate.cc
@@ -31,6 +31,7 @@ FileWriterDelegate::FileWriterDelegate(
       bytes_written_backlog_(0),
       bytes_written_(0),
       bytes_read_(0),
+      total_bytes_read_(0),
       io_buffer_(new net::IOBufferWithSize(kReadBufSize)),
       weak_factory_(this) {
 }
@@ -135,15 +136,23 @@ void FileWriterDelegate::Read() {
 
 void FileWriterDelegate::OnDataReceived(int bytes_read) {
   bytes_read_ = bytes_read;
-  if (!bytes_read_) {  // We're done.
-    OnProgress(0, true);
-  } else {
+  total_bytes_read_ += bytes_read;
+
+  if (bytes_read_ || !total_bytes_read_) {
+    // If we received bytes then write them. Zero bytes signals EOF, but if no

[jsbell, 2015/02/25 19:53:27]

Excellent comment, thanks!

+    // bytes were ever read then still call Write(). This is necessary because
+    // the file is created when the first chunk of data is received, but we need
+    // to support zero length files so explicitly write zero bytes to force the
+    // file creation.

[michaeln, 2015/02/25 20:35:43]

Should we fix this problem directly in the FileStreamWriter class? It fails to 
create a file when kCreateFlagsForWrite is used but then 0 bytes are actually
written, that seems like a bug in FileStreamWriter.

[cmumford, 2015/02/25 22:37:29]

Was reading the comment above net::FileStream::Writer, and just saw this: "Zero
byte writes are not allowed". I think I'm going to have to take a different
approach where I don't call FileStreamWriter::Write() with zero bytes, and
instead use base::File to create an empty file.

[cmumford, 2015/02/25 23:52:49]

On second thought, I'm back to the idea of fixing FileStreamWriter. Do you think
that we might create problems elsewhere where a FileStreamWriter is created, but
Write is never called?

[michaeln, 2015/02/26 02:05:37]

Maybe fix that class w/o altering behavior for existing callers.

  enum OpenOrCreate { OPEN_EXISTING_FILE, CREATE_NEW_FILE, CREATE_NEW_EMPTY_FILE
};

Where the new 3rd enum means

  delete CreateForLocalFile(task_runner, ..., CREATE_NEW_EMPTY_FILE);

should result in a task having been posted to the task_runner to create the
file.

+    //
     // This could easily be optimized to rotate between a pool of buffers, so
     // that we could read and write at the same time.  It's not yet clear that
     // it's necessary.
     cursor_ = new net::DrainableIOBuffer(io_buffer_.get(), bytes_read_);
     Write();
   }
+  if (!bytes_read_)  // We're done.
+    OnProgress(0, true);
 }
 
 void FileWriterDelegate::Write() {
@@ -234,9 +243,12 @@ void FileWriterDelegate::MaybeFlushForCompletion(
   // DCHECK_EQ on enum classes is not supported.
   DCHECK(flush_policy_ == FlushPolicy::FLUSH_ON_COMPLETION);
 
-  int flush_error = file_stream_writer_->Flush(
-      base::Bind(&FileWriterDelegate::OnFlushed, weak_factory_.GetWeakPtr(),
-                 error, bytes_written, progress_status));
+  int flush_error = 0;
+  if (bytes_written) {
+    flush_error = file_stream_writer_->Flush(
+        base::Bind(&FileWriterDelegate::OnFlushed, weak_factory_.GetWeakPtr(),
+                   error, bytes_written, progress_status));
+  }
   if (flush_error != net::ERR_IO_PENDING)
     OnFlushed(error, bytes_written, progress_status, flush_error);
 }