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..1be8eccbf36fc311879f46dcd67cef42ff7a92a2 100644
--- a/content/browser/loader/upload_data_stream_builder_unittest.cc
+++ b/content/browser/loader/upload_data_stream_builder_unittest.cc
@@ -310,4 +310,60 @@ TEST(UploadDataStreamBuilderTest, ResolveBlobAndCreateUploadDataStream) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST(UploadDataStreamBuilderTest, CreateUploadDataStreamWithEmptyBlob) {
+  base::MessageLoop message_loop;
+  {
+    // Setup blob data for testing.
+    base::Time time1, time2;
+    base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &time1);
+    base::Time::FromString("Mon, 14 Nov 1994, 11:30:49 GMT", &time2);
+
+    BlobStorageContext blob_storage_context;
+
+    const std::string blob_id0("id-0");
+    scoped_ptr<BlobDataBuilder> blob_data_builder(
+        new BlobDataBuilder(blob_id0));
+    scoped_ptr<BlobDataHandle> handle1 =
+        blob_storage_context.AddFinishedBlob(blob_data_builder.get());
+
+    const std::string blob_id1("id-1");
+    const std::string kBlobData = "";
+    blob_data_builder.reset(new BlobDataBuilder(blob_id1));
+    blob_data_builder->AppendData(kBlobData);
+    blob_data_builder->AppendFile(
+        base::FilePath(FILE_PATH_LITERAL("BlobFile.txt")), 0, 20, time1);

[michaeln, 2015/03/06 22:31:38]

This isn't an empty blob?

[cmumford, 2015/03/09 20:05:48]

Acknowledged.

+    scoped_ptr<BlobDataHandle> handle2 =
+        blob_storage_context.AddFinishedBlob(blob_data_builder.get());
+
+    ResourceRequestBody::Element blob_element1, blob_element2;
+    blob_element1.SetToBytes(kBlobData.c_str(), kBlobData.size());
+    blob_element2.SetToFilePathRange(
+        base::FilePath(FILE_PATH_LITERAL("BlobFile.txt")), 0, 20, time1);

[mmenke, 2015/03/06 20:54:40]

The CL description indicates it supports empty files, too...  Should we be using
an empty file here, or a 0-length file range?

If it's correct as-is, should the test be renamed to WithEmptyBlobElements?  We
aren't actually checking a completely empty blob, right?

[cmumford, 2015/03/09 20:05:48]

Acknowledged.

+
+    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(6U, readers.size());
+    EXPECT_TRUE(AreElementsEqual(*readers[0], blob_element1));

[michaeln, 2015/03/06 22:31:38]

Not sure about this test? It's making sure that
blob_data_builder->AppendData("") actually creates a DataElement under the
covers and adds it to the list. And its further ensuring that the emtpy elements
are propagated into the UpdateDataStream. But neither of those internal details
are important.

It's an unoptimzation that those empty elements are not culled at some point in
the process and probably shouldn't be a criteria for success.

The test doesn't determine if UploadDataStream can properly functions given a
list of empty elements. By inspection, it looks like it'll be ok, but at a
glance, the existing tests for those classes only use non-empty elements.

[cmumford, 2015/03/09 20:05:48]

Yes, I see your point. I've modified the test to use UploadDataStream (i.e read
from it).

+    EXPECT_TRUE(AreElementsEqual(*readers[1], blob_element2));
+    EXPECT_TRUE(AreElementsEqual(*readers[2], blob_element1));
+    EXPECT_TRUE(AreElementsEqual(*readers[3], blob_element2));
+    EXPECT_TRUE(AreElementsEqual(*readers[4], blob_element1));
+    EXPECT_TRUE(AreElementsEqual(*readers[5], blob_element2));
+  }
+  // Clean up for ASAN.
+  base::RunLoop().RunUntilIdle();
+}
 }  // namespace content