IndexedDB: Fixed support for empty blobs.

content/browser/loader/upload_data_stream_builder_unittest.cc

Index: content/browser/loader/upload_data_stream_builder_unittest.cc
diff --git a/content/browser/loader/upload_data_stream_builder_unittest.cc b/content/browser/loader/upload_data_stream_builder_unittest.cc
index a946e651e1f9700d673107635f070c2c6bf959be..5ff3b7bc9f25a3678ac76460cc80708b34faaaa3 100644
--- a/content/browser/loader/upload_data_stream_builder_unittest.cc
+++ b/content/browser/loader/upload_data_stream_builder_unittest.cc
@@ -5,6 +5,7 @@
 #include "content/browser/loader/upload_data_stream_builder.h"
 
 #include <algorithm>
+#include <string>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -13,6 +14,9 @@
 #include "base/run_loop.h"
 #include "base/time/time.h"
 #include "content/common/resource_request_body.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_data_stream.h"
 #include "net/base/upload_file_element_reader.h"
@@ -310,4 +314,79 @@ TEST(UploadDataStreamBuilderTest, ResolveBlobAndCreateUploadDataStream) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST(UploadDataStreamBuilderTest,
+     WriteUploadDataStreamWithEmptyFileBackedBlob) {
+  base::MessageLoop message_loop;
+  {
+    base::FilePath test_blob_path;
+    ASSERT_TRUE(base::CreateTemporaryFile(&test_blob_path));
+
+    const uint64_t kZeroLength = 0;
+    base::Time blob_time;
+    base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &blob_time);
+    ASSERT_TRUE(base::TouchFile(test_blob_path, blob_time, blob_time));
+
+    BlobStorageContext blob_storage_context;
+
+    // First an empty blob
+    const std::string blob_id0("id-0");
+    scoped_ptr<BlobDataBuilder> blob_data_builder(
+        new BlobDataBuilder(blob_id0));
+    scoped_ptr<BlobDataHandle> handle1 =

[michaeln, 2015/03/09 22:34:06]

for clarity, i'd make the id and handle vars use the same naming scheme so id0
and handle0 would pair up.

wait... better yet... i don't see where handle1 is used, so maybe just delete
this block and drop the numbering

[cmumford, 2015/03/09 23:00:10]

Done. I "cargo culted" that from the test above
(ResolveBlobAndCreateUploadDataStream). 

[michaeln, 2015/03/11 00:10:25]

But the block about "// First an empty blob" is still in the cl and blob_id1 is
still paired up with handle2? I think you can delete lines 331 thru 336 because
blob_id0 and handle are not used below.

+        blob_storage_context.AddFinishedBlob(blob_data_builder.get());
+
+    // Then a blob created from an empty file added several times.
+    const std::string blob_id1("id-1");
+    blob_data_builder.reset(new BlobDataBuilder(blob_id1));
+    blob_data_builder->AppendFile(test_blob_path, 0, kZeroLength, blob_time);
+    scoped_ptr<BlobDataHandle> handle2 =
+        blob_storage_context.AddFinishedBlob(blob_data_builder.get());
+
+    ResourceRequestBody::Element blob_element;
+    blob_element.SetToFilePathRange(test_blob_path, 0, kZeroLength, blob_time);
+
+    scoped_refptr<ResourceRequestBody> request_body(new ResourceRequestBody());
+    scoped_ptr<net::UploadDataStream> upload(UploadDataStreamBuilder::Build(
+        request_body.get(), &blob_storage_context, NULL,
+        base::MessageLoopProxy::current().get()));
+
+    request_body = new ResourceRequestBody();
+    request_body->AppendBlob(blob_id1);
+    request_body->AppendBlob(blob_id1);
+    request_body->AppendBlob(blob_id1);
+
+    upload = UploadDataStreamBuilder::Build(
+        request_body.get(), &blob_storage_context, NULL,
+        base::MessageLoopProxy::current().get());
+    ASSERT_TRUE(upload->GetElementReaders());
+    const auto& readers = *upload->GetElementReaders();
+    ASSERT_EQ(3U, readers.size());
+    EXPECT_TRUE(AreElementsEqual(*readers[0], blob_element));
+    EXPECT_TRUE(AreElementsEqual(*readers[1], blob_element));
+    EXPECT_TRUE(AreElementsEqual(*readers[2], blob_element));
+
+    net::TestCompletionCallback init_callback;
+    ASSERT_EQ(net::ERR_IO_PENDING, upload->Init(init_callback.callback()));
+    EXPECT_EQ(net::OK, init_callback.WaitForResult());
+
+    const size_t kExpectedStreamLength = 3U * kZeroLength;

[michaeln, 2015/03/09 22:34:06]

nit: maybe use the zero constant here and below

+    EXPECT_EQ(kExpectedStreamLength, upload->size());
+
+    // Purposely (try to) read more than what is in the stream. If we try to
+    // read zero bytes then UploadDataStream::Read will fail a DCHECK.
+    int kAttemptedReadLength = kExpectedStreamLength + 1;

[michaeln, 2015/03/09 22:34:06]

nit: kBufferLength or kBufferSize is more typical

+    scoped_ptr<char[]> buffer(new char[kAttemptedReadLength]);
+    scoped_refptr<net::IOBuffer> write_buff =

[michaeln, 2015/03/09 22:34:06]

nit: read_buf or io_buffer?

[cmumford, 2015/03/09 23:00:10]

Well (if I'm not mistaken). Read is reading from the UploadDataStream, and
writing to that buffer. I'll go with io_buffer.

+        new net::WrappedIOBuffer(buffer.get());
+    net::TestCompletionCallback read_callback;
+    int result = upload->Read(write_buff.get(), kAttemptedReadLength,
+                              read_callback.callback());
+    EXPECT_EQ(static_cast<int>(kExpectedStreamLength),
+              read_callback.GetResult(result));
+
+    base::DeleteFile(test_blob_path, false);
+  }
+  // Clean up for ASAN.
+  base::RunLoop().RunUntilIdle();
+}
 }  // namespace content