[Storage] Blob Storage Refactoring pt 1

storage/browser/blob/blob_url_request_job.cc

Index: storage/browser/blob/blob_url_request_job.cc
diff --git a/storage/browser/blob/blob_url_request_job.cc b/storage/browser/blob/blob_url_request_job.cc
index 41e79429b241a6b89ecb70e76bbe4eb219daf72f..6a7356168b7df1985dc1d6551086eff0f30569eb 100644
--- a/storage/browser/blob/blob_url_request_job.cc
+++ b/storage/browser/blob/blob_url_request_job.cc
@@ -34,15 +34,16 @@
 #include "storage/browser/blob/file_stream_reader.h"
 #include "storage/browser/fileapi/file_system_context.h"
 #include "storage/browser/fileapi/file_system_url.h"
+#include "storage/common/data_element.h"
 
 namespace storage {
 
 namespace {
 
-bool IsFileType(BlobData::Item::Type type) {
+bool IsFileType(DataElement::Type type) {
   switch (type) {
-    case BlobData::Item::TYPE_FILE:
-    case BlobData::Item::TYPE_FILE_FILESYSTEM:
+    case DataElement::TYPE_FILE:
+    case DataElement::TYPE_FILE_FILESYSTEM:
       return true;
     default:
       return false;
@@ -54,11 +55,11 @@ bool IsFileType(BlobData::Item::Type type) {
 BlobURLRequestJob::BlobURLRequestJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
-    const scoped_refptr<BlobData>& blob_data,
+    scoped_ptr<BlobDataSnapshot> blob_data,
     storage::FileSystemContext* file_system_context,
     base::MessageLoopProxy* file_thread_proxy)
     : net::URLRequestJob(request, network_delegate),
-      blob_data_(blob_data),
+      blob_data_(blob_data.Pass()),
       file_system_context_(file_system_context),
       file_thread_proxy_(file_thread_proxy),
       total_size_(0),
@@ -174,7 +175,7 @@ void BlobURLRequestJob::DidStart() {
   }
 
   // If the blob data is not present, bail out.
-  if (!blob_data_.get()) {
+  if (!blob_data_) {
     NotifyFailure(net::ERR_FILE_NOT_FOUND);
     return;
   }
@@ -198,10 +199,11 @@ bool BlobURLRequestJob::AddItemLength(size_t index, int64 item_length) {
 void BlobURLRequestJob::CountSize() {
   pending_get_file_info_count_ = 0;
   total_size_ = 0;
-  item_length_list_.resize(blob_data_->items().size());
+  auto& items = blob_data_->items();

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

does const auto work here? i was wondering about the const'ness

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

Ah, yeah, sorry, that was a mistake.

+  item_length_list_.resize(items.size());
 
-  for (size_t i = 0; i < blob_data_->items().size(); ++i) {
-    const BlobData::Item& item = blob_data_->items().at(i);
+  for (size_t i = 0; i < items.size(); ++i) {
+    const BlobDataItem& item = *items.at(i);

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

it's seems a little odd to have a & ref type to a refcounted data type

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

It avoids copying a the refptr, and it's pretty clear.  Also, the refptr is
staying alive.  I could do a "const scoped_refptr<BlobDataItem>&" so there isn't
the copy, but that seems less clear.

     if (IsFileType(item.type())) {
       ++pending_get_file_info_count_;
       GetFileStreamReader(i)->GetLength(
@@ -257,8 +259,9 @@ void BlobURLRequestJob::DidGetFileItemLength(size_t index, int64 result) {
     return;
   }
 
-  DCHECK_LT(index, blob_data_->items().size());
-  const BlobData::Item& item = blob_data_->items().at(index);
+  auto& items = blob_data_->items();
+  DCHECK_LT(index, items.size());
+  const BlobDataItem& item = *items.at(index);
   DCHECK(IsFileType(item.type()));
 
   uint64 file_length = result;
@@ -290,8 +293,9 @@ void BlobURLRequestJob::DidGetFileItemLength(size_t index, int64 result) {
 
 void BlobURLRequestJob::Seek(int64 offset) {
   // Skip the initial items that are not in the range.
+  auto& items = blob_data_->items();
   for (current_item_index_ = 0;
-       current_item_index_ < blob_data_->items().size() &&
+       current_item_index_ < items.size() &&
            offset >= item_length_list_[current_item_index_];
        ++current_item_index_) {
     offset -= item_length_list_[current_item_index_];
@@ -304,7 +308,7 @@ void BlobURLRequestJob::Seek(int64 offset) {
     return;
 
   // Adjust the offset of the first stream if it is of file type.
-  const BlobData::Item& item = blob_data_->items().at(current_item_index_);
+  const BlobDataItem& item = *items.at(current_item_index_);
   if (IsFileType(item.type())) {
     DeleteCurrentFileReader();
     CreateFileStreamReader(current_item_index_, offset);
@@ -316,9 +320,10 @@ bool BlobURLRequestJob::ReadItem() {
   if (remaining_bytes_ == 0)
     return true;
 
+  auto& items = blob_data_->items();
   // If we get to the last item but still expect something to read, bail out
   // since something is wrong.
-  if (current_item_index_ >= blob_data_->items().size()) {
+  if (current_item_index_ >= items.size()) {
     NotifyFailure(net::ERR_FAILED);
     return false;
   }
@@ -333,8 +338,8 @@ bool BlobURLRequestJob::ReadItem() {
   }
 
   // Do the reading.
-  const BlobData::Item& item = blob_data_->items().at(current_item_index_);
-  if (item.type() == BlobData::Item::TYPE_BYTES)
+  const BlobDataItem& item = *items.at(current_item_index_);
+  if (item.type() == DataElement::TYPE_BYTES)
     return ReadBytesItem(item, bytes_to_read);
   if (IsFileType(item.type())) {
     return ReadFileItem(GetFileStreamReader(current_item_index_),
@@ -370,7 +375,7 @@ void BlobURLRequestJob::AdvanceBytesRead(int result) {
   DCHECK_GE(read_buf_->BytesRemaining(), 0);
 }
 
-bool BlobURLRequestJob::ReadBytesItem(const BlobData::Item& item,
+bool BlobURLRequestJob::ReadBytesItem(const BlobDataItem& item,
                                       int bytes_to_read) {
   DCHECK_GE(read_buf_->BytesRemaining(), bytes_to_read);
 
@@ -555,8 +560,9 @@ void BlobURLRequestJob::HeadersCompleted(net::HttpStatusCode status_code) {
 }
 
 FileStreamReader* BlobURLRequestJob::GetFileStreamReader(size_t index) {
-  DCHECK_LT(index, blob_data_->items().size());
-  const BlobData::Item& item = blob_data_->items().at(index);
+  auto& items = blob_data_->items();
+  DCHECK_LT(index, items.size());
+  const BlobDataItem& item = *items.at(index);
   if (!IsFileType(item.type()))
     return NULL;
   if (index_to_reader_.find(index) == index_to_reader_.end())
@@ -567,21 +573,22 @@ FileStreamReader* BlobURLRequestJob::GetFileStreamReader(size_t index) {
 
 void BlobURLRequestJob::CreateFileStreamReader(size_t index,
                                                int64 additional_offset) {
-  DCHECK_LT(index, blob_data_->items().size());
-  const BlobData::Item& item = blob_data_->items().at(index);
+  auto& items = blob_data_->items();
+  DCHECK_LT(index, items.size());
+  const BlobDataItem& item = *items.at(index);
   DCHECK(IsFileType(item.type()));
   DCHECK_EQ(0U, index_to_reader_.count(index));
 
   FileStreamReader* reader = NULL;
   switch (item.type()) {
-    case BlobData::Item::TYPE_FILE:
+    case DataElement::TYPE_FILE:
       reader = FileStreamReader::CreateForLocalFile(
           file_thread_proxy_.get(),
           item.path(),
           item.offset() + additional_offset,
           item.expected_modification_time());
       break;
-    case BlobData::Item::TYPE_FILE_FILESYSTEM:
+    case DataElement::TYPE_FILE_FILESYSTEM:
       reader = file_system_context_
                    ->CreateFileStreamReader(
                          storage::FileSystemURL(file_system_context_->CrackURL(