[Storage] Blob Storage Refactoring pt 1

content/browser/service_worker/service_worker_url_request_job.cc

Index: content/browser/service_worker/service_worker_url_request_job.cc
diff --git a/content/browser/service_worker/service_worker_url_request_job.cc b/content/browser/service_worker/service_worker_url_request_job.cc
index b2d836b947d3d99e65539ed5fb1ac42d556b6f75..cdb506cac3f96aab0ac7778b47432afceb7567ca 100644
--- a/content/browser/service_worker/service_worker_url_request_job.cc
+++ b/content/browser/service_worker/service_worker_url_request_job.cc
@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/guid.h"
+#include "base/memory/scoped_vector.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
@@ -409,27 +410,35 @@ bool ServiceWorkerURLRequestJob::CreateRequestBodyBlob(std::string* blob_uuid,
   if (!body_.get() || !blob_storage_context_)
     return false;
 
+  // To ensure the blobs stick around until the end of the reading.
+  ScopedVector<storage::BlobDataHandle> handles;
+  ScopedVector<storage::BlobDataSnapshot> snapshots;
+  // TODO(dmurph): Allow blobs to be added below, so that the context can
+  // efficiently re-use blob items for the new blob.
   std::vector<const ResourceRequestBody::Element*> resolved_elements;
-  for (size_t i = 0; i < body_->elements()->size(); ++i) {
-    const ResourceRequestBody::Element& element = (*body_->elements())[i];
+  for (const ResourceRequestBody::Element& element : (*body_->elements())) {
     if (element.type() != ResourceRequestBody::Element::TYPE_BLOB) {
       resolved_elements.push_back(&element);
       continue;
     }
     scoped_ptr<storage::BlobDataHandle> handle =
         blob_storage_context_->GetBlobDataFromUUID(element.blob_uuid());
-    if (handle->data()->items().empty())
+    scoped_ptr<storage::BlobDataSnapshot> snapshot = handle->CreateSnapshot();
+    if (snapshot->items().empty())
       continue;
-    for (size_t i = 0; i < handle->data()->items().size(); ++i) {
-      const storage::BlobData::Item& item = handle->data()->items().at(i);
-      DCHECK_NE(storage::BlobData::Item::TYPE_BLOB, item.type());
-      resolved_elements.push_back(&item);
+    const auto& items = snapshot->items();
+    for (const auto& item : items) {
+      DCHECK_NE(storage::DataElement::TYPE_BLOB, item->type());
+      resolved_elements.push_back(item->data_element_ptr());
     }
+    handles.push_back(handle.release());
+    snapshots.push_back(snapshot.release());
   }
 
   const std::string uuid(base::GenerateGUID());
   uint64 total_size = 0;
-  scoped_refptr<storage::BlobData> blob_data = new storage::BlobData(uuid);
+  scoped_ptr<storage::BlobDataBuilder> blob_data(

[michaeln, 2015/01/21 01:46:41]

i think BlobDataBuilder can be on the stack here too and
might be nice to use blob_builder as a name for locals like this

[dmurph, 2015/01/21 22:40:11]

Done.

+      new storage::BlobDataBuilder(uuid));
   for (size_t i = 0; i < resolved_elements.size(); ++i) {
     const ResourceRequestBody::Element& element = *resolved_elements[i];
     if (total_size != kuint64max && element.length() != kuint64max)
@@ -441,8 +450,7 @@ bool ServiceWorkerURLRequestJob::CreateRequestBodyBlob(std::string* blob_uuid,
         blob_data->AppendData(element.bytes(), element.length());
         break;
       case ResourceRequestBody::Element::TYPE_FILE:
-        blob_data->AppendFile(element.path(),
-                              element.offset(),
+        blob_data->AppendFile(element.path(), element.offset(),
                               element.length(),
                               element.expected_modification_time());
         break;
@@ -462,7 +470,7 @@ bool ServiceWorkerURLRequestJob::CreateRequestBodyBlob(std::string* blob_uuid,
   }
 
   request_body_blob_data_handle_ =
-      blob_storage_context_->AddFinishedBlob(blob_data.get());
+      blob_storage_context_->AddFinishedBlob(*blob_data.get());
   *blob_uuid = uuid;
   *blob_size = total_size;
   return true;