[BlobAsync] Patch 1: BlobStorageRegistry

storage/browser/blob/blob_storage_registry.cc

+// Copyright 2015 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 "storage/browser/blob/blob_storage_registry.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
+#include "url/gurl.h"
+
+namespace storage {
+using BlobState = BlobStorageRegistry::BlobState;
+
+namespace {
+// We can't use GURL directly for these hash fragment manipulations
+// since it doesn't have specific knowlege of the BlobURL format. GURL
+// treats BlobURLs as if they were PathURLs which don't support hash
+// fragments.
+
+bool BlobUrlHasRef(const GURL& url) {
+  return url.spec().find('#') != std::string::npos;
+}
+
+GURL ClearBlobUrlRef(const GURL& url) {
+  size_t hash_pos = url.spec().find('#');
+  if (hash_pos == std::string::npos)
+    return url;
+  return GURL(url.spec().substr(0, hash_pos));
+}
+
+}  // namespace
+
+BlobStorageRegistry::Entry::Entry(int refcount,
+                                                          BlobState state)

[michaeln, 2015/10/09 02:16:14]

indent is (waaaaayyyy :) off

[dmurph, 2015/10/10 00:11:33]

Done.

+    : refcount(refcount), state(state), flags(0) {}
+
+BlobStorageRegistry::Entry::~Entry() {}
+
+BlobStorageRegistry::BlobStorageRegistry() {}
+
+BlobStorageRegistry::~BlobStorageRegistry() {
+  for (const auto& pair : blob_map_) {
+    scoped_ptr<Entry> entry(pair.second);
+    // We need to make sure we cleanup all of the callbacks so we don't leave
+    // any dangling URL requests around.

[michaeln, 2015/10/09 02:16:14]

Alternatively, just don't callback. The registry class is real long lived. It's
only destroyed at the end-of-time (very apocalyptic). Running code when half the
objects in the system are deleted opens the door to going down unexpected code
paths, and all for what? To tell some small part of the exiting browser that the
blob it was waiting for is never going to be built.

Since there aren't any consumers yet expecting/dependingon post-apocalyptic
callbacks, i'd vote to define them out of existence and to just delete the
callbacks w/o calling them. Good chance of sparing us a few shutdown bugs.

[dmurph, 2015/10/10 00:11:33]

Done. Left a comment.

+    if (!entry->construction_complete_callbacks.empty()) {
+      for (const auto& callback : entry->construction_complete_callbacks) {
+        base::MessageLoop::current()->PostTask(FROM_HERE,
+                                               base::Bind(callback, false));
+      }
+    }
+  }
+}
+
+BlobStorageRegistry::Entry* BlobStorageRegistry::CreateEntry(
+    const std::string& uuid) {
+  if (ContainsKey(blob_map_, uuid)) {
+    return nullptr;
+  }
+  auto* entry = new Entry(1, BlobState::RESERVED);

[michaeln, 2015/10/09 02:16:14]

Entry* instead of auto (slightly less magical)

[dmurph, 2015/10/10 00:11:33]

Done.

+  blob_map_[uuid] = entry;
+  return entry;
+}
+
+bool BlobStorageRegistry::DeleteEntry(const std::string& uuid) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end()) {
+    return false;
+  }
+  delete found->second;
+  blob_map_.erase(found);
+  return true;
+}
+
+bool BlobStorageRegistry::HasEntry(const std::string& uuid) const {
+  return ContainsKey(blob_map_, uuid);
+}
+
+BlobStorageRegistry::Entry* BlobStorageRegistry::GetBlobEntry(
+    const std::string& uuid) {
+  BlobMap::iterator found = blob_map_.find(uuid);
+  if (found == blob_map_.end()) {
+    return nullptr;
+  }
+  return found->second;
+}
+
+BlobState BlobStorageRegistry::GetBlobState(const std::string& uuid) const {
+  BlobMap::const_iterator found = blob_map_.find(uuid);
+  return found == blob_map_.end() ? BlobState::UNKNOWN : found->second->state;
+}
+
+bool BlobStorageRegistry::TestAndSetState(const std::string& uuid,
+                                          BlobState expected,
+                                          BlobState set) {
+  Entry* entry = GetBlobEntry(uuid);
+  if (!entry || entry->state != expected)
+    return false;
+  entry->state = set;
+  return true;
+}
+
+bool BlobStorageRegistry::CreateUrlMapping(const GURL& blob_url,
+                                             const std::string& uuid) {

[michaeln, 2015/10/09 02:16:14]

maybe DCHECK(!BlobUrlHasRef(blob_url));

[dmurph, 2015/10/10 00:11:33]

Done.

+  if (!HasEntry(uuid) || IsURLMapped(blob_url))
+    return false;
+  url_to_uuid_[blob_url] = uuid;
+  return true;
+}
+
+bool BlobStorageRegistry::DeleteURLMapping(const GURL& blob_url,
+                                           std::string* uuid) {
+  DCHECK(!BlobUrlHasRef(blob_url));
+  URLMap::iterator found = url_to_uuid_.find(blob_url);
+  if (found == url_to_uuid_.end()) {
+    return false;
+  }
+  if (uuid) {
+    uuid->assign(found->second);
+  }
+  url_to_uuid_.erase(found);
+  return true;
+}
+
+bool BlobStorageRegistry::IsURLMapped(const GURL& blob_url) const {
+  return ContainsKey(url_to_uuid_, blob_url);
+}
+
+BlobStorageRegistry::Entry*
+BlobStorageRegistry::GetBlobEntryFromURL(const GURL& url, std::string* uuid) {
+  URLMap::iterator found =
+      url_to_uuid_.find(BlobUrlHasRef(url) ? ClearBlobUrlRef(url) : url);
+  if (found == url_to_uuid_.end()) {
+    return nullptr;
+  }
+  Entry* entry = GetBlobEntry(found->second);
+  if (entry && uuid) {
+    uuid->assign(found->second);
+  }
+  return entry;
+}
+
+}  // namespace storage