Skip to content

Commit

Permalink
Use BigBuffer for MessagePort message contents
Browse files Browse the repository at this point in the history
Switches ArrayBuffer and raw encoded message data mojom representations
to use BigBuffer instead of array<uint8>. This avoids large message data
exceeding hard size limitations.

Also introduces BigBufferView as a traits helper to avoid redundant
copies when a deserialized BigBuffer is using inline array storage.

Bug: 829659
Change-Id: I8b62d4bcd7458db1d6aff9e22dcb7c13999f0bf0
Reviewed-on: https://chromium-review.googlesource.com/1004594
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550021}
  • Loading branch information
krockot authored and Commit Bot committed Apr 12, 2018
1 parent 866b53c commit 1ca3931
Show file tree
Hide file tree
Showing 23 changed files with 339 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,34 @@ public void testTransferEmptyPortsArray() throws Throwable {
Assert.assertEquals("12", channelContainer.getMessage());
}

// Make sure very large messages can be sent and received.
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testVeryLargeMessage() throws Throwable {
mWebServer.setResponse(IFRAME_URL, ECHO_PAGE, null);
mActivityTestRule.triggerPopup(mAwContents, mContentsClient, mWebServer,
MAIN_PAGE_FOR_POPUP_TEST, POPUP_PAGE_WITH_IFRAME, POPUP_URL, "createPopup()");
mActivityTestRule.connectPendingPopup(mAwContents);
final ChannelContainer channelContainer = new ChannelContainer();

final StringBuilder longMessageBuilder = new StringBuilder();
for (int i = 0; i < 100000; ++i) longMessageBuilder.append(HELLO);
final String longMessage = longMessageBuilder.toString();

InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
AppWebMessagePort[] channel = mAwContents.createMessageChannel();
channelContainer.set(channel);
channel[0].setMessageCallback(
(message, sentPorts) -> channelContainer.setMessage(message), null);
mAwContents.postMessageToFrame(null, WEBVIEW_MESSAGE, mWebServer.getBaseUrl(),
new AppWebMessagePort[] {channel[1]});
channel[0].postMessage(longMessage, null);
});
channelContainer.waitForMessage();
Assert.assertEquals(longMessage + JS_MESSAGE, channelContainer.getMessage());
}

// Make sure messages are dispatched on the correct looper.
@Test
@SmallTest
Expand Down
1 change: 1 addition & 0 deletions content/public/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ android_library("content_java") {
"//media/mojo/interfaces:interfaces_java",
"//mojo/android:system_java",
"//mojo/common:common_custom_types_java",
"//mojo/public/java:base_java",
"//mojo/public/java:bindings_java",
"//mojo/public/java:system_java",
"//net/android:net_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.chromium.mojo.system.MessagePipeHandle;
import org.chromium.mojo.system.Pair;
import org.chromium.mojo.system.impl.CoreImpl;
import org.chromium.mojo_base.BigBufferUtil;
import org.chromium.skia.mojom.Bitmap;

/**
Expand Down Expand Up @@ -128,7 +129,8 @@ public boolean accept(Message mojoMessage) {
ports[i] = new AppWebMessagePort(msg.ports[i]);
}
MessagePortMessage portMsg = new MessagePortMessage();
portMsg.encodedMessage = msg.message.encodedMessage;
portMsg.encodedMessage =
BigBufferUtil.getBytesFromBigBuffer(msg.message.encodedMessage);
portMsg.ports = ports;
sendMessage(obtainMessage(MESSAGE_RECEIVED, portMsg));
} catch (DeserializationException e) {
Expand Down Expand Up @@ -248,7 +250,8 @@ public void postMessage(String message, MessagePort[] sentPorts) throws IllegalS

TransferableMessage msg = new TransferableMessage();
msg.message = new CloneableMessage();
msg.message.encodedMessage = nativeEncodeStringMessage(message);
msg.message.encodedMessage =
BigBufferUtil.createBigBufferFromBytes(nativeEncodeStringMessage(message));
msg.message.blobs = new SerializedBlob[0];
msg.arrayBufferContentsArray = new SerializedArrayBufferContents[0];
msg.imageBitmapContentsArray = new Bitmap[0];
Expand Down
83 changes: 66 additions & 17 deletions mojo/public/cpp/base/big_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,7 @@ BigBuffer::BigBuffer() : storage_type_(StorageType::kBytes) {}
BigBuffer::BigBuffer(BigBuffer&& other) = default;

BigBuffer::BigBuffer(base::span<const uint8_t> data) {
if (data.size() > kMaxInlineBytes) {
auto buffer = mojo::SharedBufferHandle::Create(data.size());
if (buffer.is_valid()) {
storage_type_ = StorageType::kSharedMemory;
shared_memory_.emplace(std::move(buffer), data.size());
std::copy(data.begin(), data.end(),
static_cast<uint8_t*>(shared_memory_->buffer_mapping_.get()));
return;
}
}

// Either the data is small enough that we're happy to carry it in an array,
// or shared buffer allocation failed and we fall back on this as a best-
// effort alternative.
storage_type_ = StorageType::kBytes;
bytes_.resize(data.size());
std::copy(data.begin(), data.end(), bytes_.begin());
*this = BigBufferView::ToBigBuffer(BigBufferView(data));
}

BigBuffer::BigBuffer(const std::vector<uint8_t>& data)
Expand Down Expand Up @@ -103,4 +87,69 @@ size_t BigBuffer::size() const {
}
}

BigBufferView::BigBufferView() = default;

BigBufferView::BigBufferView(BigBufferView&& other) = default;

BigBufferView::BigBufferView(base::span<const uint8_t> bytes) {
if (bytes.size() > BigBuffer::kMaxInlineBytes) {
auto buffer = mojo::SharedBufferHandle::Create(bytes.size());
if (buffer.is_valid()) {
storage_type_ = BigBuffer::StorageType::kSharedMemory;
shared_memory_.emplace(std::move(buffer), bytes.size());
std::copy(bytes.begin(), bytes.end(),
static_cast<uint8_t*>(shared_memory_->buffer_mapping_.get()));
return;
}
}

// Either the data is small enough or shared memory allocation failed. Either
// way we fall back to directly referencing the input bytes.
storage_type_ = BigBuffer::StorageType::kBytes;
bytes_ = bytes;
}

BigBufferView::~BigBufferView() = default;

BigBufferView& BigBufferView::operator=(BigBufferView&& other) = default;

void BigBufferView::SetBytes(base::span<const uint8_t> bytes) {
DCHECK(bytes_.empty());
DCHECK(!shared_memory_);
storage_type_ = BigBuffer::StorageType::kBytes;
bytes_ = bytes;
}

void BigBufferView::SetSharedMemory(
internal::BigBufferSharedMemoryRegion shared_memory) {
DCHECK(bytes_.empty());
DCHECK(!shared_memory_);
storage_type_ = BigBuffer::StorageType::kSharedMemory;
shared_memory_ = std::move(shared_memory);
}

base::span<const uint8_t> BigBufferView::data() const {
if (storage_type_ == BigBuffer::StorageType::kBytes) {
return bytes_;
} else {
DCHECK(shared_memory_.has_value());
return base::make_span(static_cast<const uint8_t*>(const_cast<const void*>(
shared_memory_->memory())),
shared_memory_->size());
}
}

// static
BigBuffer BigBufferView::ToBigBuffer(BigBufferView view) {
BigBuffer buffer;
buffer.storage_type_ = view.storage_type_;
if (view.storage_type_ == BigBuffer::StorageType::kBytes) {
std::copy(view.bytes_.begin(), view.bytes_.end(),
std::back_inserter(buffer.bytes_));
} else {
buffer.shared_memory_ = std::move(*view.shared_memory_);
}
return buffer;
}

} // namespace mojo_base
56 changes: 56 additions & 0 deletions mojo/public/cpp/base/big_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
namespace mojo_base {

class BigBuffer;
class BigBufferView;

namespace internal {

Expand All @@ -39,6 +40,7 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBufferSharedMemoryRegion {

private:
friend class mojo_base::BigBuffer;
friend class mojo_base::BigBufferView;

size_t size_;
mojo::ScopedSharedBufferHandle buffer_handle_;
Expand Down Expand Up @@ -111,13 +113,67 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBuffer {
}

private:
friend class BigBufferView;

StorageType storage_type_;
std::vector<uint8_t> bytes_;
base::Optional<internal::BigBufferSharedMemoryRegion> shared_memory_;

DISALLOW_COPY_AND_ASSIGN(BigBuffer);
};

// Similar to BigBuffer, but doesn't *necessarily* own the buffer storage.
// Namely, if constructed over a small enough span of memory, it will simply
// retain a reference to that memory. This is generally only safe to use for
// serialization and deserialization.
class COMPONENT_EXPORT(MOJO_BASE) BigBufferView {
public:
BigBufferView();
BigBufferView(BigBufferView&& other);

// Constructs a BigBufferView over |bytes|. If |bytes| is large enough, this
// will allocate shared memory and copy the contents there. Otherwise this
// will retain an unsafe reference to |bytes| and must therefore not outlive
// |bytes|.
explicit BigBufferView(base::span<const uint8_t> bytes);
~BigBufferView();

BigBufferView& operator=(BigBufferView&& other);

base::span<const uint8_t> data() const;

// Explicitly retains a reference to |bytes| as the backing storage for this
// view. Does not copy and therefore |bytes| must remain valid throughout the
// view's lifetime. Used for deserialization.
void SetBytes(base::span<const uint8_t> bytes);

// Explictly adopts |shared_memory| as the backing storage for this view. Used
// for deserialization.
void SetSharedMemory(internal::BigBufferSharedMemoryRegion shared_memory);

// Converts to a BigBuffer which owns the viewed data. May have to copy data.
static BigBuffer ToBigBuffer(BigBufferView view) WARN_UNUSED_RESULT;

BigBuffer::StorageType storage_type() const { return storage_type_; }

base::span<const uint8_t> bytes() const {
DCHECK_EQ(storage_type_, BigBuffer::StorageType::kBytes);
return bytes_;
}

internal::BigBufferSharedMemoryRegion& shared_memory() {
DCHECK_EQ(storage_type_, BigBuffer::StorageType::kSharedMemory);
return shared_memory_.value();
}

private:
BigBuffer::StorageType storage_type_;
base::span<const uint8_t> bytes_;
base::Optional<internal::BigBufferSharedMemoryRegion> shared_memory_;

DISALLOW_COPY_AND_ASSIGN(BigBufferView);
};

} // namespace mojo_base

#endif // MOJO_PUBLIC_CPP_BASE_BIG_BUFFER_H_
54 changes: 54 additions & 0 deletions mojo/public/cpp/base/big_buffer_mojom_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,58 @@ bool UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>::
return false;
}

// static
mojo_base::mojom::BigBufferDataView::Tag UnionTraits<
mojo_base::mojom::BigBufferDataView,
mojo_base::BigBufferView>::GetTag(const mojo_base::BigBufferView& view) {
switch (view.storage_type()) {
case mojo_base::BigBuffer::StorageType::kBytes:
return mojo_base::mojom::BigBufferDataView::Tag::BYTES;
case mojo_base::BigBuffer::StorageType::kSharedMemory:
return mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY;
}

NOTREACHED();
return mojo_base::mojom::BigBufferDataView::Tag::BYTES;
}

// static
base::span<const uint8_t> UnionTraits<
mojo_base::mojom::BigBufferDataView,
mojo_base::BigBufferView>::bytes(const mojo_base::BigBufferView& view) {
return view.bytes();
}

// static
mojo_base::internal::BigBufferSharedMemoryRegion& UnionTraits<
mojo_base::mojom::BigBufferDataView,
mojo_base::BigBufferView>::shared_memory(mojo_base::BigBufferView& view) {
return view.shared_memory();
}

// static
bool UnionTraits<
mojo_base::mojom::BigBufferDataView,
mojo_base::BigBufferView>::Read(mojo_base::mojom::BigBufferDataView data,
mojo_base::BigBufferView* out) {
switch (data.tag()) {
case mojo_base::mojom::BigBufferDataView::Tag::BYTES: {
mojo::ArrayDataView<uint8_t> bytes_view;
data.GetBytesDataView(&bytes_view);
out->SetBytes(bytes_view);
return true;
}

case mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY: {
mojo_base::internal::BigBufferSharedMemoryRegion shared_memory;
if (!data.ReadSharedMemory(&shared_memory))
return false;
out->SetSharedMemory(std::move(shared_memory));
return true;
}
}

return false;
}

} // namespace mojo
16 changes: 16 additions & 0 deletions mojo/public/cpp/base/big_buffer_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <vector>

#include "base/component_export.h"
#include "base/containers/span.h"
#include "base/macros.h"
#include "mojo/public/cpp/base/big_buffer.h"
#include "mojo/public/cpp/bindings/union_traits.h"
#include "mojo/public/cpp/system/buffer.h"
Expand Down Expand Up @@ -43,6 +45,20 @@ struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
mojo_base::BigBuffer* out);
};

template <>
struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBufferView> {
static mojo_base::mojom::BigBufferDataView::Tag GetTag(
const mojo_base::BigBufferView& view);

static base::span<const uint8_t> bytes(const mojo_base::BigBufferView& view);
static mojo_base::internal::BigBufferSharedMemoryRegion& shared_memory(
mojo_base::BigBufferView& view);

static bool Read(mojo_base::mojom::BigBufferDataView data,
mojo_base::BigBufferView* out);
};

} // namespace mojo

#endif // MOJO_PUBLIC_CPP_BASE_BIG_BUFFER_MOJOM_TRAITS_H_
10 changes: 10 additions & 0 deletions mojo/public/java/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,13 @@ android_library("bindings_java") {

srcjar_deps = [ "../interfaces/bindings:bindings_java_sources" ]
}

android_library("base_java") {
java_files = [ "base/src/org/chromium/mojo_base/BigBufferUtil.java" ]

deps = [
":system_java",
"//mojo/android:system_java",
"//mojo/public/mojom/base:base_java",
]
}
Loading

0 comments on commit 1ca3931

Please sign in to comment.