Move SetUserData of BlobDataHandles from UploadDataStreamBuilder to ResourceDispatcherHostImpl.

content/browser/loader/upload_data_stream_builder.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 "base/logging.h"
 #include "content/browser/fileapi/upload_file_system_file_element_reader.h"
 #include "content/common/resource_request_body.h"
 #include "net/base/upload_bytes_element_reader.h"
@@ skipping 46 matching lines @@
 
   virtual ~FileElementReader() {}
 
  private:
   scoped_refptr<ResourceRequestBody> resource_request_body_;
 
   DISALLOW_COPY_AND_ASSIGN(FileElementReader);
 };
 
 void ResolveBlobReference(
-    ResourceRequestBody* body,
     webkit_blob::BlobStorageContext* blob_context,
     const ResourceRequestBody::Element& element,
     std::vector<const ResourceRequestBody::Element*>* resolved_elements) {
   DCHECK(blob_context);
   scoped_ptr<webkit_blob::BlobDataHandle> handle =
       blob_context->GetBlobDataFromUUID(element.blob_uuid());

[mmenke, 2014/08/25 18:49:02]

I know this works, due to refcounting, but but should we really be creating a
new handle here?  Seems like we should be using a raw pointer instead of
creating short-lived handles.  My suggestion is just to make
UploadDataStreamBuilder::Build take in an optional BlobData*, and mention the
lifetime dependency in its documentation.

[horo, 2014/08/26 00:31:53]

If my understanding is correct, we resolve the blob into the other type (BYTES,
FILE, FILE_SYSTEM) items here to avoid code complexity.
If we don't resolve here we have to implement UploadElementReader for Blob.

So we need to get this BlobDataHandle from the uuid in order to resolve the
blob.

[mmenke, 2014/08/26 18:09:14]

Think it's at least worth a comment the caller is responsible for making sure
the BlobData outlives the returned value of UploadDataStream, maybe in the docs
for UploadDataStreamBuilder::Build?

[michaeln, 2014/08/26 20:34:08]

Thats a good point.

I asked horo to hoist the SetUserData() calls into RDH because there's a 2nd
chunk of code, also invoked by RDH, that's relying on that behavior as well.
Another option could be to have both UploadDataStreamBuilder and the other
consumer (serviceworker thing) independently use SetUserData() to take care of
themselves (so to speak).

[horo, 2014/08/27 01:38:15]

Added comments in upload_data_stream_builder.h.

   DCHECK(handle);
   if (!handle)
     return;
 
   // If there is no element in the referred blob data, just return.
   if (handle->data()->items().empty())
     return;
 
   // Append the elements in the referenced blob data.
   for (size_t i = 0; i < handle->data()->items().size(); ++i) {
     const BlobData::Item& item = handle->data()->items().at(i);
     DCHECK_NE(BlobData::Item::TYPE_BLOB, item.type());
     resolved_elements->push_back(&item);
   }
-
-  // Ensure the blob and any attached shareable files survive until
-  // upload completion. The |body| takes ownership of |handle|.
-  const void* key = handle.get();
-  body->SetUserData(key, handle.release());
 }
 
 }  // namespace
 
 scoped_ptr<net::UploadDataStream> UploadDataStreamBuilder::Build(
     ResourceRequestBody* body,
     BlobStorageContext* blob_context,
     fileapi::FileSystemContext* file_system_context,
     base::TaskRunner* file_task_runner) {
   // Resolve all blob elements.
   std::vector<const ResourceRequestBody::Element*> resolved_elements;
   for (size_t i = 0; i < body->elements()->size(); ++i) {
     const ResourceRequestBody::Element& element = (*body->elements())[i];
     if (element.type() == ResourceRequestBody::Element::TYPE_BLOB)
-      ResolveBlobReference(body, blob_context, element, &resolved_elements);
+      ResolveBlobReference(blob_context, element, &resolved_elements);
     else
       resolved_elements.push_back(&element);
   }
 
   ScopedVector<net::UploadElementReader> element_readers;
   for (size_t i = 0; i < resolved_elements.size(); ++i) {
     const ResourceRequestBody::Element& element = *resolved_elements[i];
     switch (element.type()) {
       case ResourceRequestBody::Element::TYPE_BYTES:
         element_readers.push_back(new BytesElementReader(body, element));
@@ skipping 19 matching lines @@
         NOTREACHED();
         break;
     }
   }
 
   return make_scoped_ptr(
       new net::UploadDataStream(element_readers.Pass(), body->identifier()));
 }
 
 }  // namespace content