New blobstoragecontext for use in the main browser process, not plugged in yet.

webkit/blob/blob_storage_context.cc

Index: webkit/blob/blob_storage_context.cc
===================================================================
--- webkit/blob/blob_storage_context.cc	(revision 189105)
+++ webkit/blob/blob_storage_context.cc	(working copy)
@@ -1,10 +1,14 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 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 "webkit/blob/blob_storage_controller.h"
+#include "webkit/blob/blob_storage_context.h"
 
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/logging.h"
+#include "base/message_loop_proxy.h"
+#include "base/sys_info.h"
 #include "googleurl/src/gurl.h"
 #include "webkit/blob/blob_data.h"
 
@@ -28,33 +32,217 @@
   return GURL(url.spec().substr(0, hash_pos));
 }
 
-static const int64 kMaxMemoryUsage = 1024 * 1024 * 1024;  // 1G
+// TODO(michaeln): use base::SysInfo::AmountOfPhysicalMemoryMB() in some
+// way to come up with a better limit.
+static const int64 kMaxMemoryUsage = 500 * 1024 * 1024;  // Half a gig.
 
 }  // namespace
 
-BlobStorageController::BlobStorageController()
+//-----------------------------------------------------------------------
+// BlobDataHandle
+//-----------------------------------------------------------------------
+
+BlobDataHandle::BlobDataHandle(BlobData* blob_data, BlobStorageContext* context,
+                               base::SequencedTaskRunner* task_runner)
+    : blob_data_(blob_data),
+      context_(context->AsWeakPtr()),
+      io_task_runner_(task_runner) {
+  // Ensures the uuid remains registered and the underlying data is not deleted.
+  DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
+  context_->IncrementBlobRefCount(blob_data->uuid());
+  blob_data_->AddRef();
+}
+
+BlobDataHandle::~BlobDataHandle() {
+  if (io_task_runner_->RunsTasksOnCurrentThread()) {
+    // Note: Do not test context_ on the wrong thread.
+    if (context_.get())
+      context_->DecrementBlobRefCount(blob_data_->uuid());
+    blob_data_->Release();

[ericu, 2013/04/24 23:54:43]

Wouldn't this be a bit cleaner with a scoped_refptr for blob_data_, and only
handling it specially if we're on the wrong thread?  This manually dealing with
refcounts feels awkward.  What about ReleaseSoon?

[michaeln, 2013/04/25 01:21:37]

It would not be cleaner because there is no api to tell a scoped_refptr to
fuggedaboutit. There is no way to use ReleaseSoon w/o altering the refcount on
the 'wrong thread'. Here's the recipe for ReleaseSoon.

    DatabaseTracker* tracker = db_tracker_;
    tracker->AddRef();   <<<< addref here on the wrong thread
    db_tracker_ = NULL;  <<<< deref there on the wrong thread
    if (!db_tracker_thread_->ReleaseSoon(FROM_HERE, tracker))
      tracker->Release();

I could have a scoped_ptr to a refptr which is Passed to the other thread, but
thats not really cleaner or clearer.

[ericu, 2013/04/26 22:17:37]

Arg.  OK, that's clearly not better.  Nevermind.

+    return;
+  }
+
+  io_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&DeleteHelper, context_, base::Unretained(blob_data_)));
+}
+
+BlobData* BlobDataHandle::data() const {
+  DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
+  return blob_data_;
+}
+
+// static
+void BlobDataHandle::DeleteHelper(
+    base::WeakPtr<BlobStorageContext> context,
+    BlobData* blob_data) {
+  if (context.get())
+    context->DecrementBlobRefCount(blob_data->uuid());
+  blob_data->Release();
+}
+
+//-----------------------------------------------------------------------
+// BlobStorageHost
+//-----------------------------------------------------------------------
+
+BlobStorageHost::BlobStorageHost(BlobStorageContext* context)
+    : context_(context->AsWeakPtr()) {
+}
+
+BlobStorageHost::~BlobStorageHost() {
+  if (!context_)
+    return;
+  for (std::set<GURL>::iterator iter = public_blob_urls_.begin();
+       iter != public_blob_urls_.end(); ++iter) {
+    context_->RevokePublicBlobURL(*iter);
+  }
+  for (BlobReferenceMap::iterator iter = blobs_inuse_map_.begin();
+       iter != blobs_inuse_map_.end(); ++iter) {
+    for (int i = 0; i < iter->second; ++i)
+      context_->DecrementBlobRefCount(iter->first);
+  }
+}
+
+bool BlobStorageHost::StartBuildingBlob(const std::string& uuid) {
+  if (!context_ || context_->IsInUse(uuid))
+    return false;
+  context_->StartBuildingBlob(uuid);
+  blobs_inuse_map_[uuid] = 1;
+  return true;
+}
+
+bool BlobStorageHost::AppendBlobDataItem(
+    const std::string& uuid, const BlobData::Item& data_item) {
+  if (!context_ || !HasUseCountOfOne(uuid))
+    return false;
+  context_->AppendBlobDataItem(uuid, data_item);
+  return true;
+}
+
+bool BlobStorageHost::CancelBuildingBlob(const std::string& uuid) {
+  if (!context_ || !HasUseCountOfOne(uuid))
+    return false;
+  blobs_inuse_map_.erase(uuid);
+  context_->CancelBuildingBlob(uuid);
+  return true;
+}
+
+bool BlobStorageHost::FinishBuildingBlob(
+    const std::string& uuid, const std::string& content_type) {
+  if (!context_ || !HasUseCountOfOne(uuid))
+    return false;
+  context_->FinishBuildingBlob(uuid, content_type);
+  return true;
+}
+
+bool BlobStorageHost::IncrementBlobRefCount(const std::string& uuid) {
+  if (!context_ || !context_->IsInUse(uuid))
+    return false;
+  context_->IncrementBlobRefCount(uuid);
+  blobs_inuse_map_[uuid] += 1;
+  return true;
+}
+
+bool BlobStorageHost::DecrementBlobRefCount(const std::string& uuid) {
+  if (!context_ || !IsInUse(uuid))
+    return false;
+  context_->DecrementBlobRefCount(uuid);
+  blobs_inuse_map_[uuid] -= 1;
+  if (blobs_inuse_map_[uuid] == 0)
+    blobs_inuse_map_.erase(uuid);
+  return true;
+}
+
+bool BlobStorageHost::RegisterPublicBlobURL(
+    const GURL& blob_url, const std::string& uuid) {
+  if (!context_ || !IsInUse(uuid) || context_->IsUrlRegistered(blob_url))
+    return false;
+  context_->RegisterPublicBlobURL(blob_url, uuid);
+  public_blob_urls_.insert(blob_url);
+  return true;
+}
+
+bool BlobStorageHost::RevokePublicBlobURL(const GURL& blob_url) {
+  if (!context_ || !IsUrlRegistered(blob_url))
+    return false;
+  context_->RevokePublicBlobURL(blob_url);
+  public_blob_urls_.erase(blob_url);
+  return true;
+}
+
+bool BlobStorageHost::IsInUse(const std::string& uuid) {
+  return blobs_inuse_map_.find(uuid) != blobs_inuse_map_.end();
+}
+
+bool BlobStorageHost::HasUseCountOfOne(const std::string& uuid) {
+  return IsInUse(uuid) && (blobs_inuse_map_[uuid] == 1);

[ericu, 2013/04/24 23:54:43]

Given how often this is called, you might want to optimize it to make only a
single lookup in the map, rather than using IsInUse here.

[michaeln, 2013/04/25 01:21:37]

will do when splitting files

[ericu, 2013/04/26 22:17:37]

OK; if that's not going to be in the next upload, please add a TODO.  That way I
can be lazy and not have to remember to check for it ;'>.

[michaeln, 2013/04/29 18:32:48]

this is done elsewhere, see later comment about where

+}
+
+bool BlobStorageHost::IsUrlRegistered(const GURL& blob_url) {
+  return public_blob_urls_.find(blob_url) != public_blob_urls_.end();
+}
+
+//-----------------------------------------------------------------------
+// BlobStorageContext
+//-----------------------------------------------------------------------
+
+BlobStorageContext::BlobStorageContext()
     : memory_usage_(0) {
 }
 
-BlobStorageController::~BlobStorageController() {
+BlobStorageContext::~BlobStorageContext() {
 }
 
-void BlobStorageController::StartBuildingBlob(const GURL& url) {
-  DCHECK(url.SchemeIs("blob"));
-  DCHECK(!BlobUrlHasRef(url));
-  BlobData* blob_data = new BlobData;
-  unfinalized_blob_map_[url.spec()] = blob_data;
-  IncrementBlobDataUsage(blob_data);
+scoped_ptr<BlobDataHandle> BlobStorageContext::GetBlobDataFromUUID(
+    const std::string& uuid) {
+  scoped_ptr<BlobDataHandle> result;
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end())
+    return result.Pass();
+  if (found->second.flags & EXCEEDED_MEMORY)
+    return result.Pass();
+  DCHECK(!(found->second.flags & BEING_BUILT));
+  result.reset(new BlobDataHandle(found->second.data, this,
+                                  base::MessageLoopProxy::current()));
+  return result.Pass();
 }
 
-void BlobStorageController::AppendBlobDataItem(
-    const GURL& url, const BlobData::Item& item) {
-  DCHECK(url.SchemeIs("blob"));
-  DCHECK(!BlobUrlHasRef(url));
-  BlobMap::iterator found = unfinalized_blob_map_.find(url.spec());
-  if (found == unfinalized_blob_map_.end())
+scoped_ptr<BlobDataHandle> BlobStorageContext::GetBlobDataFromPublicURL(
+    const GURL& url) {
+  BlobURLMap::iterator found = public_blob_urls_.find(
+      BlobUrlHasRef(url) ? ClearBlobUrlRef(url) : url);
+  if (found == public_blob_urls_.end())
+    return scoped_ptr<BlobDataHandle>();
+  return GetBlobDataFromUUID(found->second);
+}
+
+scoped_ptr<BlobDataHandle> BlobStorageContext::AddFinishedBlob(
+    const BlobData* data) {
+  StartBuildingBlob(data->uuid());
+  for (std::vector<BlobData::Item>::const_iterator iter =
+           data->items().begin();
+       iter != data->items().end(); ++iter) {
+    AppendBlobDataItem(data->uuid(), *iter);
+  }
+  FinishBuildingBlob(data->uuid(), data->content_type());
+  scoped_ptr<BlobDataHandle> handle = GetBlobDataFromUUID(data->uuid());
+  DecrementBlobRefCount(data->uuid());
+  return handle.Pass();
+}
+
+void BlobStorageContext::StartBuildingBlob(const std::string& uuid) {
+  DCHECK(!IsInUse(uuid));
+  blob_map_[uuid] = BlobMapEntry(1, BEING_BUILT, new BlobData(uuid));
+}
+
+void BlobStorageContext::AppendBlobDataItem(
+    const std::string& uuid, const BlobData::Item& item) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end())
     return;
-  BlobData* target_blob_data = found->second;
+  if (found->second.flags & EXCEEDED_MEMORY)

[ericu, 2013/04/24 23:54:43]

Check for BEING_BUILT too?

[michaeln, 2013/04/25 01:21:37]

i'll add a DCHECK here, and an IsBeingBuilt() for use in the Host.

[michaeln, 2013/04/25 21:22:46]

I've massaged the rel and dbg runtime checks a little. My goal with those checks
are...

1) Prevent the blob storage data structs from getting out of whack when given
untrusted renderer inputs.

2) Notice when a renderer is providing bad inputs and return false to the
caller. This will be used in later CLs to terminate the bad renderer.

3) Dbg only checks to let developers know when they're providing unexpected
inputs.

I'm not defending against the consequences of buggy code with that last one in
rel, just helping to identify the buggy code at dbg time (so it never makes it
to rel). Nothing a renderer can say should lead to a crash or trip a DCHECK in
the context.

+    return;
+  BlobData* target_blob_data = found->second.data;
   DCHECK(target_blob_data);
 
   memory_usage_ -= target_blob_data->GetMemoryUsage();
@@ -67,7 +255,7 @@
   // 3) The FileSystem File item is denoted by the FileSystem URL, the range
   //    and the expected modification time.
   // All the Blob items in the passing blob data are resolved and expanded into
-  // a set of Data and File items.
+  // a set of Data, File, and FileSystemFile items.
 
   DCHECK(item.length() > 0);
   switch (item.type()) {
@@ -84,17 +272,17 @@
       break;
     case BlobData::Item::TYPE_FILE_FILESYSTEM:
       AppendFileSystemFileItem(target_blob_data,
-                               item.url(),
+                               item.filesystem_url(),
                                item.offset(),
                                item.length(),
                                item.expected_modification_time());
       break;
     case BlobData::Item::TYPE_BLOB: {
-      BlobData* src_blob_data = GetBlobDataFromUrl(item.url());
-      DCHECK(src_blob_data);
-      if (src_blob_data)
+      scoped_ptr<BlobDataHandle> src = GetBlobDataFromUUID(item.blob_uuid());
+      DCHECK(src.get());
+      if (src.get())
         AppendStorageItems(target_blob_data,
-                           src_blob_data,
+                           src->data(),
                            item.offset(),
                            item.length());
       break;
@@ -108,75 +296,66 @@
 
   // If we're using too much memory, drop this blob.
   // TODO(michaeln): Blob memory storage does not yet spill over to disk,
-  // until it does, we'll prevent memory usage over a max amount.
-  if (memory_usage_ > kMaxMemoryUsage)
-    RemoveBlob(url);
+  // as a stop gap, we'll prevent memory usage over a max amount.
+  if (memory_usage_ > kMaxMemoryUsage) {
+    // TODO(michaeln): Hoist this test up to before we go over.
+    memory_usage_ -= target_blob_data->GetMemoryUsage();
+    found->second.flags |= EXCEEDED_MEMORY;
+    found->second.data = new BlobData(uuid);
+  }
 }
 
-void BlobStorageController::FinishBuildingBlob(
-    const GURL& url, const std::string& content_type) {
-  DCHECK(url.SchemeIs("blob"));
-  DCHECK(!BlobUrlHasRef(url));
-  BlobMap::iterator found = unfinalized_blob_map_.find(url.spec());
-  if (found == unfinalized_blob_map_.end())
+void BlobStorageContext::FinishBuildingBlob(
+    const std::string& uuid, const std::string& content_type) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end())
     return;
-  found->second->set_content_type(content_type);
-  blob_map_[url.spec()] = found->second;
-  unfinalized_blob_map_.erase(found);
+  found->second.data->set_content_type(content_type);
+  found->second.flags &= ~BEING_BUILT;

[ericu, 2013/04/24 23:54:43]

DCHECK before clearing it?

 }
 
-void BlobStorageController::AddFinishedBlob(const GURL& url,
-                                            const BlobData* data) {
-  StartBuildingBlob(url);
-  for (std::vector<BlobData::Item>::const_iterator iter =
-           data->items().begin();
-       iter != data->items().end(); ++iter) {
-    AppendBlobDataItem(url, *iter);
-  }
-  FinishBuildingBlob(url, data->content_type());
+void BlobStorageContext::CancelBuildingBlob(const std::string& uuid) {
+  DecrementBlobRefCount(uuid);
 }
 
-void BlobStorageController::CloneBlob(
-    const GURL& url, const GURL& src_url) {
-  DCHECK(url.SchemeIs("blob"));
-  DCHECK(!BlobUrlHasRef(url));
-
-  BlobData* blob_data = GetBlobDataFromUrl(src_url);
-  DCHECK(blob_data);
-  if (!blob_data)
+void BlobStorageContext::IncrementBlobRefCount(const std::string& uuid) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end()) {
+    DCHECK(false);
     return;
-
-  blob_map_[url.spec()] = blob_data;
-  IncrementBlobDataUsage(blob_data);
+  }
+  ++(found->second.refcount);
 }
 
-void BlobStorageController::RemoveBlob(const GURL& url) {
-  DCHECK(url.SchemeIs("blob"));
-  DCHECK(!BlobUrlHasRef(url));
-
-  if (!RemoveFromMapHelper(&unfinalized_blob_map_, url))
-    RemoveFromMapHelper(&blob_map_, url);
+void BlobStorageContext::DecrementBlobRefCount(const std::string& uuid) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end())
+    return;
+  DCHECK_EQ(found->second.data->uuid(), uuid);
+  if (--(found->second.refcount) == 0) {
+    memory_usage_ -= found->second.data->GetMemoryUsage();
+    blob_map_.erase(found);
+  }
 }
 
-bool BlobStorageController::RemoveFromMapHelper(
-    BlobMap* map, const GURL& url) {
-  BlobMap::iterator found = map->find(url.spec());
-  if (found == map->end())
-    return false;
-  if (DecrementBlobDataUsage(found->second))
-    memory_usage_ -= found->second->GetMemoryUsage();
-  map->erase(found);
-  return true;
+void BlobStorageContext::RegisterPublicBlobURL(
+    const GURL& blob_url, const std::string& uuid) {
+  DCHECK(!BlobUrlHasRef(blob_url));
+  DCHECK(IsInUse(uuid));
+  DCHECK(!IsUrlRegistered(blob_url));
+  IncrementBlobRefCount(uuid);
+  public_blob_urls_[blob_url] = uuid;
 }
 
-
-BlobData* BlobStorageController::GetBlobDataFromUrl(const GURL& url) {
-  BlobMap::iterator found = blob_map_.find(
-      BlobUrlHasRef(url) ? ClearBlobUrlRef(url).spec() : url.spec());
-  return (found != blob_map_.end()) ? found->second : NULL;
+void BlobStorageContext::RevokePublicBlobURL(const GURL& blob_url) {
+  DCHECK(!BlobUrlHasRef(blob_url));
+  if (!IsUrlRegistered(blob_url))
+    return;
+  DecrementBlobRefCount(public_blob_urls_[blob_url]);
+  public_blob_urls_.erase(blob_url);
 }
 
-void BlobStorageController::AppendStorageItems(
+void BlobStorageContext::AppendStorageItems(
     BlobData* target_blob_data, BlobData* src_blob_data,
     uint64 offset, uint64 length) {
   DCHECK(target_blob_data && src_blob_data &&
@@ -209,7 +388,7 @@
     } else {
       DCHECK(iter->type() == BlobData::Item::TYPE_FILE_FILESYSTEM);
       AppendFileSystemFileItem(target_blob_data,
-                               iter->url(),
+                               iter->filesystem_url(),
                                iter->offset() + offset,
                                new_length,
                                iter->expected_modification_time());
@@ -219,7 +398,7 @@
   }
 }
 
-void BlobStorageController::AppendFileItem(
+void BlobStorageContext::AppendFileItem(
     BlobData* target_blob_data,
     const base::FilePath& file_path, uint64 offset, uint64 length,
     const base::Time& expected_modification_time) {
@@ -233,25 +412,24 @@
     target_blob_data->AttachShareableFileReference(shareable_file);
 }
 
-void BlobStorageController::AppendFileSystemFileItem(
+void BlobStorageContext::AppendFileSystemFileItem(
     BlobData* target_blob_data,
-    const GURL& url, uint64 offset, uint64 length,
+    const GURL& filesystem_url, uint64 offset, uint64 length,
     const base::Time& expected_modification_time) {
-  target_blob_data->AppendFileSystemFile(url, offset, length,
+  target_blob_data->AppendFileSystemFile(filesystem_url, offset, length,
                                          expected_modification_time);
 }
 
-void BlobStorageController::IncrementBlobDataUsage(BlobData* blob_data) {
-  blob_data_usage_count_[blob_data] += 1;
+bool BlobStorageContext::IsInUse(const std::string& uuid) {
+  return blob_map_.find(uuid) != blob_map_.end();
 }
 
-bool BlobStorageController::DecrementBlobDataUsage(BlobData* blob_data) {
-  BlobDataUsageMap::iterator found = blob_data_usage_count_.find(blob_data);
-  DCHECK(found != blob_data_usage_count_.end());
-  if (--(found->second))
-    return false;  // Still in use
-  blob_data_usage_count_.erase(found);
-  return true;
+bool BlobStorageContext::HasUseCountOfOne(const std::string& uuid) {
+  return IsInUse(uuid) && (blob_map_[uuid].refcount == 1);
 }
 
+bool BlobStorageContext::IsUrlRegistered(const GURL& blob_url) {
+  return public_blob_urls_.find(blob_url) != public_blob_urls_.end();
+}
+
 }  // namespace webkit_blob