Use BigBuffer for legacy IPC messages
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}
CrOS-Libchrome-Original-Commit: 3e126191c1a7a022260c1072b1b9288f09bb2dce
diff --git a/ipc/ipc.mojom b/ipc/ipc.mojom
index 87aedae..9631ce8 100644
--- a/ipc/ipc.mojom
+++ b/ipc/ipc.mojom
@@ -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);
diff --git a/ipc/ipc_message_pipe_reader.cc b/ipc/ipc_message_pipe_reader.cc
index 4a8ca8f..b055d41 100644
--- a/ipc/ipc_message_pipe_reader.cc
+++ b/ipc/ipc_message_pipe_reader.cc
@@ -63,10 +63,7 @@
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;
}
@@ -84,23 +81,20 @@
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;
diff --git a/ipc/ipc_message_pipe_reader.h b/ipc/ipc_message_pipe_reader.h
index 77dd9ee..e1c3fd4 100644
--- a/ipc/ipc_message_pipe_reader.h
+++ b/ipc/ipc_message_pipe_reader.h
@@ -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 {
@@ -95,9 +94,7 @@
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;
diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
index 208080d..10d75bb 100644
--- a/ipc/ipc_message_utils.cc
+++ b/ipc/ipc_message_utils.cc
@@ -1414,31 +1414,6 @@
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));
}
diff --git a/ipc/ipc_message_utils.h b/ipc/ipc_message_utils.h
index 0dbf500..00a164a 100644
--- a/ipc/ipc_message_utils.h
+++ b/ipc/ipc_message_utils.h
@@ -1019,16 +1019,6 @@
};
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;
static void Write(base::Pickle* m, const param_type& p);
diff --git a/ipc/ipc_mojo_bootstrap_unittest.cc b/ipc/ipc_mojo_bootstrap_unittest.cc
index e5d9518..7772896 100644
--- a/ipc/ipc_mojo_bootstrap_unittest.cc
+++ b/ipc/ipc_mojo_bootstrap_unittest.cc
@@ -72,14 +72,11 @@
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());
@@ -196,7 +193,9 @@
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());
diff --git a/ipc/message_mojom_traits.cc b/ipc/message_mojom_traits.cc
new file mode 100644
index 0000000..4aab924
--- /dev/null
+++ b/ipc/message_mojom_traits.cc
@@ -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
diff --git a/ipc/message_mojom_traits.h b/ipc/message_mojom_traits.h
new file mode 100644
index 0000000..617ffbe
--- /dev/null
+++ b/ipc/message_mojom_traits.h
@@ -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_
diff --git a/ipc/message_view.cc b/ipc/message_view.cc
new file mode 100644
index 0000000..d6ff4fb
--- /dev/null
+++ b/ipc/message_view.cc
@@ -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
diff --git a/ipc/message_view.h b/ipc/message_view.h
new file mode 100644
index 0000000..23e5da6
--- /dev/null
+++ b/ipc/message_view.h
@@ -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_
diff --git a/mojo/public/cpp/base/file_info.typemap b/mojo/public/cpp/base/file_info.typemap
index f6ef16a..4faac8f 100644
--- a/mojo/public/cpp/base/file_info.typemap
+++ b/mojo/public/cpp/base/file_info.typemap
@@ -4,9 +4,10 @@
mojom = "//mojo/public/mojom/base/file_info.mojom"
public_headers = [ "//base/files/file.h" ]
-traits_headers = [ "//ipc/ipc_message_utils.h" ]
+traits_headers = [ "//mojo/public/cpp/base/file_info_mojom_traits.h" ]
public_deps = [
- "//ipc",
+ "//base",
+ "//mojo/public/cpp/base:shared_typemap_traits",
]
type_mappings = [ "mojo_base.mojom.FileInfo=base::File::Info" ]
diff --git a/mojo/public/cpp/base/file_info_mojom_traits.cc b/mojo/public/cpp/base/file_info_mojom_traits.cc
new file mode 100644
index 0000000..64a0802
--- /dev/null
+++ b/mojo/public/cpp/base/file_info_mojom_traits.cc
@@ -0,0 +1,28 @@
+// 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 "mojo/public/cpp/base/file_info_mojom_traits.h"
+
+#include "base/logging.h"
+#include "mojo/public/cpp/base/time_mojom_traits.h"
+
+namespace mojo {
+
+// static
+bool StructTraits<mojo_base::mojom::FileInfoDataView, base::File::Info>::Read(
+ mojo_base::mojom::FileInfoDataView data,
+ base::File::Info* out) {
+ if (!data.ReadLastModified(&out->last_modified))
+ return false;
+ if (!data.ReadLastAccessed(&out->last_accessed))
+ return false;
+ if (!data.ReadCreationTime(&out->creation_time))
+ return false;
+ out->size = data.size();
+ out->is_directory = data.is_directory();
+ out->is_symbolic_link = data.is_symbolic_link();
+ return true;
+}
+
+} // namespace mojo
diff --git a/mojo/public/cpp/base/file_info_mojom_traits.h b/mojo/public/cpp/base/file_info_mojom_traits.h
new file mode 100644
index 0000000..4049e03
--- /dev/null
+++ b/mojo/public/cpp/base/file_info_mojom_traits.h
@@ -0,0 +1,49 @@
+// 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 MOJO_PUBLIC_CPP_BASE_FILE_INFO_MOJOM_TRAITS_H_
+#define MOJO_PUBLIC_CPP_BASE_FILE_INFO_MOJOM_TRAITS_H_
+
+#include <cstdint>
+
+#include "base/component_export.h"
+#include "base/files/file.h"
+#include "base/macros.h"
+#include "mojo/public/cpp/bindings/struct_traits.h"
+#include "mojo/public/mojom/base/file_info.mojom-shared.h"
+
+namespace mojo {
+
+template <>
+struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
+ StructTraits<mojo_base::mojom::FileInfoDataView, base::File::Info> {
+ static int64_t size(const base::File::Info& info) { return info.size; }
+
+ static bool is_directory(const base::File::Info& info) {
+ return info.is_directory;
+ }
+
+ static bool is_symbolic_link(const base::File::Info& info) {
+ return info.is_symbolic_link;
+ }
+
+ static base::Time last_modified(const base::File::Info& info) {
+ return info.last_modified;
+ }
+
+ static base::Time last_accessed(const base::File::Info& info) {
+ return info.last_accessed;
+ }
+
+ static base::Time creation_time(const base::File::Info& info) {
+ return info.creation_time;
+ }
+
+ static bool Read(mojo_base::mojom::FileInfoDataView data,
+ base::File::Info* out);
+};
+
+} // namespace mojo
+
+#endif // MOJO_PUBLIC_CPP_BASE_FILE_INFO_MOJOM_TRAITS_H_
diff --git a/mojo/public/cpp/base/time.typemap b/mojo/public/cpp/base/time.typemap
index b3b2e8b..ecd6a27 100644
--- a/mojo/public/cpp/base/time.typemap
+++ b/mojo/public/cpp/base/time.typemap
@@ -5,12 +5,9 @@
mojom = "//mojo/public/mojom/base/time.mojom"
public_headers = [ "//base/time/time.h" ]
traits_headers = [ "//mojo/public/cpp/base/time_mojom_traits.h" ]
-sources = [
- "//mojo/public/cpp/base/time_mojom_traits.cc",
- "//mojo/public/cpp/base/time_mojom_traits.h",
-]
public_deps = [
"//base",
+ "//mojo/public/cpp/base:shared_typemap_traits",
]
type_mappings = [
diff --git a/mojo/public/cpp/base/time_mojom_traits.h b/mojo/public/cpp/base/time_mojom_traits.h
index db54476..213c6e2 100644
--- a/mojo/public/cpp/base/time_mojom_traits.h
+++ b/mojo/public/cpp/base/time_mojom_traits.h
@@ -12,7 +12,7 @@
namespace mojo {
template <>
-struct COMPONENT_EXPORT(MOJO_BASE_MOJOM)
+struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
StructTraits<mojo_base::mojom::TimeDataView, base::Time> {
static int64_t internal_value(const base::Time& time);
@@ -20,7 +20,7 @@
};
template <>
-struct COMPONENT_EXPORT(MOJO_BASE_MOJOM)
+struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
StructTraits<mojo_base::mojom::TimeDeltaDataView, base::TimeDelta> {
static int64_t microseconds(const base::TimeDelta& delta);
@@ -29,7 +29,7 @@
};
template <>
-struct COMPONENT_EXPORT(MOJO_BASE_MOJOM)
+struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
StructTraits<mojo_base::mojom::TimeTicksDataView, base::TimeTicks> {
static int64_t internal_value(const base::TimeTicks& time);
diff --git a/mojo/public/mojom/base/file_info.mojom b/mojo/public/mojom/base/file_info.mojom
index 544f291..5dff3ca 100644
--- a/mojo/public/mojom/base/file_info.mojom
+++ b/mojo/public/mojom/base/file_info.mojom
@@ -4,5 +4,14 @@
module mojo_base.mojom;
-[Native]
-struct FileInfo;
+import "mojo/public/mojom/base/time.mojom";
+
+// Corresponds to base::File::Info.
+struct FileInfo {
+ int64 size;
+ bool is_directory;
+ bool is_symbolic_link;
+ Time last_modified;
+ Time last_accessed;
+ Time creation_time;
+};
diff --git a/mojo/public/tools/bindings/chromium_bindings_configuration.gni b/mojo/public/tools/bindings/chromium_bindings_configuration.gni
index 5e1df16..e0e9e23 100644
--- a/mojo/public/tools/bindings/chromium_bindings_configuration.gni
+++ b/mojo/public/tools/bindings/chromium_bindings_configuration.gni
@@ -24,6 +24,7 @@
"//device/bluetooth/public/mojom/test/typemaps.gni",
"//device/gamepad/public/cpp/typemaps.gni",
"//gpu/ipc/common/typemaps.gni",
+ "//ipc/typemaps.gni",
"//media/capture/mojom/typemaps.gni",
"//media/mojo/interfaces/typemaps.gni",
"//mojo/public/cpp/base/typemaps.gni",