IndexedDB: Fixed support for empty blobs.

content/browser/loader/upload_data_stream_builder_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/loader/upload_data_stream_builder.h"
 
 #include <algorithm>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ skipping 292 matching lines @@
         *(*upload->GetElementReaders())[5], blob_element1));
     EXPECT_TRUE(AreElementsEqual(
         *(*upload->GetElementReaders())[6], blob_element2));
     EXPECT_TRUE(AreElementsEqual(
         *(*upload->GetElementReaders())[7], upload_element2));
   }
   // Clean up for ASAN.
   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