Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3302)

Unified Diff: blimp/engine/renderer/blob_channel_sender_proxy.cc

Issue 2033013003: Use shared memory for moving data over BlobChannel Mojo interface. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@blobchannel-master
Patch Set: dcheng feedback Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: blimp/engine/renderer/blob_channel_sender_proxy.cc
diff --git a/blimp/engine/renderer/blob_channel_sender_proxy.cc b/blimp/engine/renderer/blob_channel_sender_proxy.cc
index efdfeae668bcd02cc3e329944e7a495559337843..dfebdd44a00bfaf9d68e3c9e010dfec2d26ca5ff 100644
--- a/blimp/engine/renderer/blob_channel_sender_proxy.cc
+++ b/blimp/engine/renderer/blob_channel_sender_proxy.cc
@@ -4,6 +4,8 @@
#include "blimp/engine/renderer/blob_channel_sender_proxy.h"
+#include "blimp/common/blob_cache/id_util.h"
+#include "content/public/common/service_registry.h"
#include "content/public/renderer/render_thread.h"
#include "services/shell/public/cpp/interface_provider.h"
@@ -11,6 +13,10 @@ namespace blimp {
namespace engine {
namespace {
+// Sets a finite upper limit for blob sizes, to prevent massive memory
+// allocations from taking place.
+const size_t kMaxBlobSizeBytes = 10 * 1024 * 1024;
+
mojom::BlobChannelPtr GetConnectedBlobChannel() {
mojom::BlobChannelPtr blob_channel_ptr;
content::RenderThread::Get()->GetRemoteInterfaces()->GetInterface(
@@ -19,30 +25,98 @@ mojom::BlobChannelPtr GetConnectedBlobChannel() {
return blob_channel_ptr;
}
+// Manages the creation and lifetime of Mojo shared memory buffers for blobs.
+// Cleans up the shared memory state when deleted.
+// The SharedMemoryBlob must not be deleted until the data consumer has
+// acknowledged that it is finished using the buffer.
+class SharedMemoryBlob {
+ public:
+ explicit SharedMemoryBlob(BlobDataPtr data);
+ ~SharedMemoryBlob();
+
+ mojo::ScopedSharedBufferHandle GetRemoteHandle();
+
+ private:
+ mojo::ScopedSharedBufferHandle local_handle_;
+
+ DISALLOW_COPY_AND_ASSIGN(SharedMemoryBlob);
+};
+
+SharedMemoryBlob::SharedMemoryBlob(BlobDataPtr data) {
+ CHECK_LT(data->data.size(), kMaxBlobSizeBytes);
dcheng 2016/06/28 06:26:18 I think this should be a DCHECK(), with a browser-
Kevin M 2016/06/28 18:18:53 Done and done.
+
+ local_handle_ = mojo::SharedBufferHandle::Create(data->data.size());
+ DCHECK(local_handle_.is_valid()) << "Mojo error when creating shared buffer.";
+
+ auto mapped = local_handle_->Map(data->data.size());
+ DCHECK(mapped) << "Mojo error when memory mapping shared buffer.";
+ memcpy(mapped.get(), data->data.data(), data->data.size());
+}
+
+SharedMemoryBlob::~SharedMemoryBlob() {}
+
+mojo::ScopedSharedBufferHandle SharedMemoryBlob::GetRemoteHandle() {
+ auto remote_handle =
+ local_handle_->Clone(mojo::SharedBufferHandle::AccessMode::READ_ONLY);
+ CHECK(remote_handle.is_valid())
+ << "Mojo error when creating read-only buffer handle.";
+ return remote_handle;
+}
+
} // namespace
BlobChannelSenderProxy::BlobChannelSenderProxy()
- : blob_channel_(GetConnectedBlobChannel()) {}
+ : BlobChannelSenderProxy(GetConnectedBlobChannel()) {}
BlobChannelSenderProxy::~BlobChannelSenderProxy() {}
+BlobChannelSenderProxy::BlobChannelSenderProxy(
+ mojom::BlobChannelPtr blob_channel)
+ : blob_channel_(std::move(blob_channel)) {}
+
+// static
+std::unique_ptr<BlobChannelSenderProxy> BlobChannelSenderProxy::CreateForTest(
+ mojom::BlobChannelPtr blob_channel) {
+ return base::WrapUnique(new BlobChannelSenderProxy(std::move(blob_channel)));
+}
+
bool BlobChannelSenderProxy::IsInEngineCache(const std::string& id) const {
return replication_state_.find(id) != replication_state_.end();
}
bool BlobChannelSenderProxy::IsInClientCache(const std::string& id) const {
- return replication_state_.find(id)->second;
+ auto found = replication_state_.find(id);
+ return found != replication_state_.end() && found->second;
}
void BlobChannelSenderProxy::PutBlob(const BlobId& id, BlobDataPtr data) {
- DCHECK(!IsInEngineCache(id));
+ if (IsInEngineCache(id)) {
+ DLOG(FATAL) << "Redundant blob put attempted: " << BlobIdToString(id);
+ return;
+ }
+
+ size_t size = data->data.size();
+ if (size == 0) {
+ DLOG(FATAL) << "Zero length blob sent: " << BlobIdToString(id);
+ return;
+ }
replication_state_[id] = false;
- blob_channel_->PutBlob(id, data->data);
+ std::unique_ptr<SharedMemoryBlob> shared_mem_blob(
+ new SharedMemoryBlob(std::move(data)));
+ blob_channel_->PutBlob(id, shared_mem_blob->GetRemoteHandle(), size);
}
void BlobChannelSenderProxy::DeliverBlob(const std::string& id) {
- DCHECK(!IsInClientCache(id));
+ if (!IsInEngineCache(id)) {
+ DLOG(FATAL) << "Attempted to deliver an invalid blob: "
+ << BlobIdToString(id);
+ return;
+ }
+ if (IsInClientCache(id)) {
+ DVLOG(1) << "Blob is already in the remote cache:" << BlobIdToString(id);
+ return;
+ }
// We assume that the client will have the blob if we push it.
// TODO(kmarshall): Revisit this assumption when asynchronous blob transport

Powered by Google App Engine
This is Rietveld 408576698