Skip to content

Commit

Permalink
Use BigBuffer for legacy IPC messages
Browse files Browse the repository at this point in the history
This replaces use of ReadOnlyBuffer with a new mojom Message type
specific to //ipc. This type maps to another new C++ MessageView type
which in turn wraps a BigBufferView.

This allows us to transparently fall back onto shared memory for large
IPC messages without increasing the number of copies during send or
receive in any (small- or large-message) cases.

In order to avoid introducing more mojo-base targets, this also removes
the remaining [Native] structs from mojo_base mojom (LOGFONT and
FileInfo) and replaces them with real mojom structures + StructTraits,
thus allowing //ipc to depend on mojo/public/*/base in its entirety.

Also fixes random missing public_deps entries for a
chrome/services/file_util typemap, because it decided to start breaking
all of my local builds. :3

Bug: 784069
Change-Id: I359b964ffc1fe44ffd6aa704405ea63156f4fbc9
Reviewed-on: https://chromium-review.googlesource.com/1131685
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573956}
  • Loading branch information
krockot authored and Commit Bot committed Jul 10, 2018
1 parent ec61662 commit 3e12619
Show file tree
Hide file tree
Showing 27 changed files with 391 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public_headers = [ "//chrome/common/safe_browsing/zip_analyzer.h" ]

traits_headers = [ "//chrome/services/file_util/public/mojom/safe_archive_analyzer_param_traits.h" ]

deps = [
public_deps = [
"//chrome/common/safe_browsing:proto",
"//components/safe_browsing:csd_proto",
]
Expand Down
3 changes: 2 additions & 1 deletion ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ component("ipc") {
":mojom",
":native_handle_type_converters",
":param_traits",
"//mojo/public/cpp/base",
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/system",
]
Expand Down Expand Up @@ -182,7 +183,7 @@ mojom_component("mojom") {
]
public_deps = [
"//mojo/public/interfaces/bindings",
"//mojo/public/mojom/base:read_only_buffer",
"//mojo/public/mojom/base",
]

# Don't generate a variant sources since we depend on generated internal
Expand Down
4 changes: 4 additions & 0 deletions ipc/OWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ per-file *_messages*.h=set noparent
per-file *_messages*.h=file://ipc/SECURITY_OWNERS
per-file *.mojom=set noparent
per-file *.mojom=file://ipc/SECURITY_OWNERS
per-file *_mojom_traits*.*=set noparent
per-file *_mojom_traits*.*=file://ipc/SECURITY_OWNERS
per-file *_param_traits*.*=set noparent
per-file *_param_traits*.*=file://ipc/SECURITY_OWNERS
per-file *.typemap=set noparent
per-file *.typemap=file://ipc/SECURITY_OWNERS
per-file *_type_converter*.*=set noparent
per-file *_type_converter*.*=file://ipc/SECURITY_OWNERS

Expand Down
12 changes: 9 additions & 3 deletions ipc/ipc.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,27 @@

module IPC.mojom;

import "mojo/public/mojom/base/read_only_buffer.mojom";
import "mojo/public/mojom/base/big_buffer.mojom";
import "mojo/public/interfaces/bindings/native_struct.mojom";

// A placeholder interface type since we don't yet support generic associated
// message pipe handles.
interface GenericInterface {};

// Typemapped such that arbitrarily large IPC::Message objects can be sent and
// received with minimal copying.
struct Message {
mojo_base.mojom.BigBuffer buffer;
array<mojo.native.SerializedHandle>? handles;
};

interface Channel {
// Informs the remote end of this client's PID. Must be called exactly once,
// before any calls to Receive() below.
SetPeerPid(int32 pid);

// Transmits a classical Chrome IPC message.
Receive(mojo_base.mojom.ReadOnlyBuffer data,
array<mojo.native.SerializedHandle>? handles);
Receive(Message message);

// Requests a Channel-associated interface.
GetAssociatedInterface(string name, associated GenericInterface& request);
Expand Down
18 changes: 6 additions & 12 deletions ipc/ipc_message_pipe_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ bool MessagePipeReader::Send(std::unique_ptr<Message> message) {
if (!sender_)
return false;

sender_->Receive(base::make_span(static_cast<const uint8_t*>(message->data()),
message->size()),
std::move(handles));

sender_->Receive(MessageView(*message, std::move(handles)));
DVLOG(4) << "Send " << message->type() << ": " << message->size();
return true;
}
Expand All @@ -84,23 +81,20 @@ void MessagePipeReader::SetPeerPid(int32_t peer_pid) {
delegate_->OnPeerPidReceived(peer_pid);
}

void MessagePipeReader::Receive(
base::span<const uint8_t> data,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles) {
if (data.empty()) {
void MessagePipeReader::Receive(MessageView message_view) {
if (!message_view.size()) {
delegate_->OnBrokenDataReceived();
return;
}
Message message(reinterpret_cast<const char*>(data.data()),
static_cast<uint32_t>(data.size()));
Message message(message_view.data(), message_view.size());
if (!message.IsValid()) {
delegate_->OnBrokenDataReceived();
return;
}

DVLOG(4) << "Receive " << message.type() << ": " << message.size();
MojoResult write_result =
ChannelMojo::WriteToMessageAttachmentSet(std::move(handles), &message);
MojoResult write_result = ChannelMojo::WriteToMessageAttachmentSet(
message_view.TakeHandles(), &message);
if (write_result != MOJO_RESULT_OK) {
OnPipeError(write_result);
return;
Expand Down
5 changes: 1 addition & 4 deletions ipc/ipc_message_pipe_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
#include "mojo/public/cpp/system/core.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "mojo/public/interfaces/bindings/native_struct.mojom.h"

namespace IPC {
namespace internal {
Expand Down Expand Up @@ -95,9 +94,7 @@ class COMPONENT_EXPORT(IPC) MessagePipeReader : public mojom::Channel {
private:
// mojom::Channel:
void SetPeerPid(int32_t peer_pid) override;
void Receive(base::span<const uint8_t> data,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>>
handles) override;
void Receive(MessageView message_view) override;
void GetAssociatedInterface(
const std::string& name,
mojom::GenericInterfaceAssociatedRequest request) override;
Expand Down
25 changes: 0 additions & 25 deletions ipc/ipc_message_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1414,31 +1414,6 @@ void ParamTraits<HANDLE>::Log(const param_type& p, std::string* l) {
l->append(base::StringPrintf("0x%p", p));
}

void ParamTraits<LOGFONT>::Write(base::Pickle* m, const param_type& p) {
m->WriteData(reinterpret_cast<const char*>(&p), sizeof(LOGFONT));
}

bool ParamTraits<LOGFONT>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* r) {
const char *data;
int data_size = 0;
if (iter->ReadData(&data, &data_size) && data_size == sizeof(LOGFONT)) {
const LOGFONT *font = reinterpret_cast<LOGFONT*>(const_cast<char*>(data));
if (_tcsnlen(font->lfFaceName, LF_FACESIZE) < LF_FACESIZE) {
memcpy(r, data, sizeof(LOGFONT));
return true;
}
}

NOTREACHED();
return false;
}

void ParamTraits<LOGFONT>::Log(const param_type& p, std::string* l) {
l->append(base::StringPrintf("<LOGFONT>"));
}

void ParamTraits<MSG>::Write(base::Pickle* m, const param_type& p) {
m->WriteData(reinterpret_cast<const char*>(&p), sizeof(MSG));
}
Expand Down
10 changes: 0 additions & 10 deletions ipc/ipc_message_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1018,16 +1018,6 @@ struct COMPONENT_EXPORT(IPC) ParamTraits<HANDLE> {
static void Log(const param_type& p, std::string* l);
};

template <>
struct COMPONENT_EXPORT(IPC) ParamTraits<LOGFONT> {
typedef LOGFONT param_type;
static void Write(base::Pickle* m, const param_type& p);
static bool Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* r);
static void Log(const param_type& p, std::string* l);
};

template <>
struct COMPONENT_EXPORT(IPC) ParamTraits<MSG> {
typedef MSG param_type;
Expand Down
11 changes: 5 additions & 6 deletions ipc/ipc_mojo_bootstrap_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,11 @@ class PeerPidReceiver : public IPC::mojom::Channel {
on_peer_pid_set_.Run();
}

void Receive(base::span<const uint8_t> data,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>>
handles) override {
void Receive(IPC::MessageView message_view) override {
ASSERT_NE(MessageExpectation::kNotExpected, message_expectation_);
received_message_ = true;

IPC::Message message(reinterpret_cast<const char*>(data.data()),
static_cast<uint32_t>(data.size()));
IPC::Message message(message_view.data(), message_view.size());
bool expected_valid =
message_expectation_ == MessageExpectation::kExpectedValid;
EXPECT_EQ(expected_valid, message.IsValid());
Expand Down Expand Up @@ -196,7 +193,9 @@ MULTIPROCESS_TEST_MAIN_WITH_SETUP(
auto& sender = connection.GetSender();

uint8_t data = 0;
sender->Receive(base::make_span(&data, 0), {});
sender->Receive(
IPC::MessageView(mojo_base::BigBufferView(base::make_span(&data, 0)),
base::nullopt /* handles */));

base::RunLoop run_loop;
PeerPidReceiver impl(std::move(receiver), run_loop.QuitClosure());
Expand Down
18 changes: 18 additions & 0 deletions ipc/message.typemap
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright 2018 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.

mojom = "//ipc/ipc.mojom"
public_headers = [ "//ipc/message_view.h" ]
traits_headers = [ "//ipc/message_mojom_traits.h" ]
sources = [
"//ipc/message_mojom_traits.cc",
"//ipc/message_mojom_traits.h",
"//ipc/message_view.cc",
"//ipc/message_view.h",
]
public_deps = [
"//ipc:message_support",
"//mojo/public/cpp/base:shared_typemap_traits",
]
type_mappings = [ "IPC.mojom.Message=IPC::MessageView[move_only]" ]
40 changes: 40 additions & 0 deletions ipc/message_mojom_traits.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2018 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 "ipc/message_mojom_traits.h"

#include "mojo/public/cpp/base/big_buffer_mojom_traits.h"

namespace mojo {

// static
mojo_base::BigBufferView
StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::buffer(
IPC::MessageView& view) {
return view.TakeBufferView();
}

// static
base::Optional<std::vector<mojo::native::SerializedHandlePtr>>
StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::handles(
IPC::MessageView& view) {
return view.TakeHandles();
}

// static
bool StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::Read(
IPC::mojom::MessageDataView data,
IPC::MessageView* out) {
mojo_base::BigBufferView buffer_view;
if (!data.ReadBuffer(&buffer_view))
return false;
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles;
if (!data.ReadHandles(&handles))
return false;

*out = IPC::MessageView(std::move(buffer_view), std::move(handles));
return true;
}

} // namespace mojo
31 changes: 31 additions & 0 deletions ipc/message_mojom_traits.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2018 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.

#ifndef IPC_MESSAGE_MOJOM_TRAITS_H_
#define IPC_MESSAGE_MOJOM_TRAITS_H_

#include <vector>

#include "base/optional.h"
#include "ipc/ipc.mojom-shared.h"
#include "ipc/message_view.h"
#include "mojo/public/cpp/base/big_buffer.h"
#include "mojo/public/cpp/bindings/struct_traits.h"
#include "mojo/public/interfaces/bindings/native_struct.mojom.h"

namespace mojo {

template <>
class StructTraits<IPC::mojom::MessageDataView, IPC::MessageView> {
public:
static mojo_base::BigBufferView buffer(IPC::MessageView& view);
static base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles(
IPC::MessageView& view);

static bool Read(IPC::mojom::MessageDataView data, IPC::MessageView* out);
};

} // namespace mojo

#endif // IPC_MESSAGE_MOJOM_TRAITS_H_
30 changes: 30 additions & 0 deletions ipc/message_view.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2018 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 "ipc/message_view.h"

namespace IPC {

MessageView::MessageView() = default;

MessageView::MessageView(
const Message& message,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
: buffer_view_(base::make_span<const uint8_t>(
static_cast<const uint8_t*>(message.data()),
message.size())),
handles_(std::move(handles)) {}

MessageView::MessageView(
mojo_base::BigBufferView buffer_view,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
: buffer_view_(std::move(buffer_view)), handles_(std::move(handles)) {}

MessageView::MessageView(MessageView&&) = default;

MessageView::~MessageView() = default;

MessageView& MessageView::operator=(MessageView&&) = default;

} // namespace IPC
56 changes: 56 additions & 0 deletions ipc/message_view.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2018 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.

#ifndef IPC_MESSAGE_VIEW_H_
#define IPC_MESSAGE_VIEW_H_

#include <vector>

#include "base/component_export.h"
#include "base/containers/span.h"
#include "base/macros.h"
#include "ipc/ipc_message.h"
#include "mojo/public/cpp/base/big_buffer.h"
#include "mojo/public/interfaces/bindings/native_struct.mojom.h"

namespace IPC {

class COMPONENT_EXPORT(IPC_MOJOM) MessageView {
public:
MessageView();
MessageView(
const Message& message,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
MessageView(
mojo_base::BigBufferView buffer_view,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
MessageView(MessageView&&);
~MessageView();

MessageView& operator=(MessageView&&);

const char* data() const {
return reinterpret_cast<const char*>(buffer_view_.data().data());
}

uint32_t size() const {
return static_cast<uint32_t>(buffer_view_.data().size());
}

mojo_base::BigBufferView TakeBufferView() { return std::move(buffer_view_); }

base::Optional<std::vector<mojo::native::SerializedHandlePtr>> TakeHandles() {
return std::move(handles_);
}

private:
mojo_base::BigBufferView buffer_view_;
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles_;

DISALLOW_COPY_AND_ASSIGN(MessageView);
};

} // namespace IPC

#endif // IPC_MESSAGE_VIEW_H_
5 changes: 5 additions & 0 deletions ipc/typemaps.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright 2018 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.

typemaps = [ "//ipc/message.typemap" ]
Loading

0 comments on commit 3e12619

Please sign in to comment.