Blobs: Catching browser-process created Blobs in extension code.

extensions/browser/blob_holder.cc

+// Copyright 2014 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 "extensions/browser/blob_holder.h"
+
+#include "base/logging.h"
+#include "content/public/browser/blob_handle.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+
+namespace extensions {
+
+namespace {
+
+// Address to this variable used as the user data key.
+const int kBlobHolderUserDataKey = 0;
+}
+
+// static
+BlobHolder* BlobHolder::FromRenderProcessHost(
+    content::RenderProcessHost* render_process_host) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(render_process_host);
+  BlobHolder* existing = static_cast<BlobHolder*>(
+      render_process_host->GetUserData(&kBlobHolderUserDataKey));
+
+  if (existing)
+    return existing;
+
+  BlobHolder* new_instance = new BlobHolder(render_process_host);
+  render_process_host->SetUserData(&kBlobHolderUserDataKey, new_instance);
+  return new_instance;
+}
+
+BlobHolder::~BlobHolder() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+}
+
+void BlobHolder::HoldBlobReference(scoped_ptr<content::BlobHandle> blob) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!ContainsBlobHandle(blob.get()));

[michaeln, 2014/05/23 17:31:36]

might be able to simplify

DCHECK_EQ(held_blobs_.end(),
          std::find(held_blobs_.begin(), held_blobs_.end(), handle));

[tommycli, 2014/05/23 18:48:39]

Was possible. No longer possible since I switched to the multimap.

+  held_blobs_.push_back(blob.release());
+}
+
+BlobHolder::BlobHolder(content::RenderProcessHost* render_process_host)
+    : render_process_host_(render_process_host) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+}
+
+bool BlobHolder::ContainsBlobHandle(content::BlobHandle* handle) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  for (BlobHandleVector::const_iterator it = held_blobs_.begin();
+       it != held_blobs_.end();
+       ++it) {
+    if (*it == handle)
+      return true;
+  }
+
+  return false;
+}
+
+void BlobHolder::DropBlobs(const std::vector<std::string>& blob_uuids) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  for (std::vector<std::string>::const_iterator uuid_it = blob_uuids.begin();

[michaeln, 2014/05/23 17:31:36]

this n*n loop is probably not so bad because these collections will never be
very large? another option could be to have the holder hold collection s and
identify each collection with an id

int BlobHolder::HoldBlobs(scoped_ptr<std::vector<...> > blobs_to_hold) {
  return idmapofownptrs.insert(blobs_to_hold.release());
}

BlobHolder::ReleaseBlobs(int held_collection_id) {
  idmapofownptrs.erase(held_collection_id);
}

and send just the collection_id in the ipc round trip

might be smaller more efficient code here (with a larger storage struct)

[tommycli, 2014/05/23 18:48:39]

Yeah you're right this is a little hard to understand. In my case blob_uuids is
1 or 2 in size, but i agree that this looks kind of janky.

I changed it to a multimap<string, linked_ptr> to see if I can improve without
changing the interface.

The collection id thing would also work. I can do that if yoz also thinks that's
better.

+       uuid_it != blob_uuids.end();
+       ++uuid_it) {
+    bool found = false;
+    for (BlobHandleVector::iterator blob_handle_it = held_blobs_.begin();
+         blob_handle_it != held_blobs_.end();
+         ++blob_handle_it) {
+      if ((*blob_handle_it)->GetUUID() == *uuid_it) {
+        held_blobs_.erase(blob_handle_it);
+        found = true;
+        break;
+      }
+    }
+
+    if (!found) {
+      DLOG(ERROR) << "Tried to release a Blob we don't have ownership to."
+                  << "UUID: " << *uuid_it;
+      render_process_host_->ReceivedBadMessage();
+    }
+  }
+}
+
+}  // namespace extensions