Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use legacy base::Value serializers for ipc (7-1-x) #21922

Merged
merged 8 commits into from Jan 31, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions build/args/goma.gn
@@ -0,0 +1,2 @@
goma_dir = rebase_path("//electron/external_binaries/goma")
use_goma = true
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -92,3 +92,4 @@ accessible_pane_view.patch
only_apply_transform_when_outermost_outer_webcontents.patch
never_let_a_non-zero-size_pixel_snap_to_zero_size.patch
fix_hi-dpi_transitions_on_catalina.patch
typemap.patch
21 changes: 21 additions & 0 deletions patches/chromium/typemap.patch
@@ -0,0 +1,21 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp <nornagon@nornagon.net>
Date: Mon, 27 Jan 2020 16:02:41 -0800
Subject: typemap

this adds electron's typemap to the global registry of typemaps. Only
needed for temporarily supporting the legacy-ipc base::ListValue
serializers until we switch to using SCA.

diff --git a/mojo/public/tools/bindings/chromium_bindings_configuration.gni b/mojo/public/tools/bindings/chromium_bindings_configuration.gni
index 6e8bc9e2c4f0edca9f22eac5017b44000e477ea2..a482fdca501ed94ec135c18cd35bc2fe3cf6ed88 100644
--- a/mojo/public/tools/bindings/chromium_bindings_configuration.gni
+++ b/mojo/public/tools/bindings/chromium_bindings_configuration.gni
@@ -22,6 +22,7 @@ _typemap_imports = [
"//device/bluetooth/public/mojom/typemaps.gni",
"//device/bluetooth/public/mojom/test/typemaps.gni",
"//device/gamepad/public/cpp/typemaps.gni",
+ "//electron/shell/common/api/typemaps.gni",
"//fuchsia/mojom/test_typemaps.gni",
"//gpu/ipc/common/typemaps.gni",
"//ipc/typemaps.gni",
27 changes: 14 additions & 13 deletions shell/browser/api/atom_api_web_contents.cc
Expand Up @@ -1023,15 +1023,15 @@ void WebContents::OnElectronBrowserConnectionError() {

void WebContents::Message(bool internal,
const std::string& channel,
base::Value arguments) {
base::ListValue arguments) {
// webContents.emit('-ipc-message', new Event(), internal, channel,
// arguments);
EmitWithSender("-ipc-message", bindings_.dispatch_context(), base::nullopt,
internal, channel, std::move(arguments));
}

void WebContents::Invoke(const std::string& channel,
base::Value arguments,
base::ListValue arguments,
InvokeCallback callback) {
// webContents.emit('-ipc-invoke', new Event(), channel, arguments);
EmitWithSender("-ipc-invoke", bindings_.dispatch_context(),
Expand All @@ -1040,7 +1040,7 @@ void WebContents::Invoke(const std::string& channel,

void WebContents::MessageSync(bool internal,
const std::string& channel,
base::Value arguments,
base::ListValue arguments,
MessageSyncCallback callback) {
// webContents.emit('-ipc-message-sync', new Event(sender, message), internal,
// channel, arguments);
Expand All @@ -1052,19 +1052,18 @@ void WebContents::MessageTo(bool internal,
bool send_to_all,
int32_t web_contents_id,
const std::string& channel,
base::Value arguments) {
base::ListValue arguments) {
auto* web_contents = mate::TrackableObject<WebContents>::FromWeakMapID(
isolate(), web_contents_id);

if (web_contents) {
web_contents->SendIPCMessageWithSender(internal, send_to_all, channel,
base::ListValue(arguments.GetList()),
ID());
std::move(arguments), ID());
}
}

void WebContents::MessageHost(const std::string& channel,
base::Value arguments) {
base::ListValue arguments) {
// webContents.emit('ipc-message-host', new Event(), channel, args);
EmitWithSender("ipc-message-host", bindings_.dispatch_context(),
base::nullopt, channel, std::move(arguments));
Expand Down Expand Up @@ -1943,14 +1942,15 @@ void WebContents::TabTraverse(bool reverse) {
bool WebContents::SendIPCMessage(bool internal,
bool send_to_all,
const std::string& channel,
const base::ListValue& args) {
return SendIPCMessageWithSender(internal, send_to_all, channel, args);
base::ListValue args) {
return SendIPCMessageWithSender(internal, send_to_all, channel,
std::move(args));
}

bool WebContents::SendIPCMessageWithSender(bool internal,
bool send_to_all,
const std::string& channel,
const base::ListValue& args,
base::ListValue args,
int32_t sender_id) {
std::vector<content::RenderFrameHost*> target_hosts;
if (!send_to_all) {
Expand All @@ -1966,7 +1966,8 @@ bool WebContents::SendIPCMessageWithSender(bool internal,
mojom::ElectronRendererAssociatedPtr electron_ptr;
frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
mojo::MakeRequest(&electron_ptr));
electron_ptr->Message(internal, false, channel, args.Clone(), sender_id);
electron_ptr->Message(internal, false, channel,
base::ListValue(args.Clone().GetList()), sender_id);
}
return true;
}
Expand All @@ -1975,7 +1976,7 @@ bool WebContents::SendIPCMessageToFrame(bool internal,
bool send_to_all,
int32_t frame_id,
const std::string& channel,
const base::ListValue& args) {
base::ListValue args) {
auto frames = web_contents()->GetAllFrames();
auto iter = std::find_if(frames.begin(), frames.end(), [frame_id](auto* f) {
return f->GetRoutingID() == frame_id;
Expand All @@ -1988,7 +1989,7 @@ bool WebContents::SendIPCMessageToFrame(bool internal,
mojom::ElectronRendererAssociatedPtr electron_ptr;
(*iter)->GetRemoteAssociatedInterfaces()->GetInterface(
mojo::MakeRequest(&electron_ptr));
electron_ptr->Message(internal, send_to_all, channel, args.Clone(),
electron_ptr->Message(internal, send_to_all, channel, std::move(args),
0 /* sender_id */);
return true;
}
Expand Down
17 changes: 9 additions & 8 deletions shell/browser/api/atom_api_web_contents.h
Expand Up @@ -217,19 +217,19 @@ class WebContents : public mate::TrackableObject<WebContents>,
bool SendIPCMessage(bool internal,
bool send_to_all,
const std::string& channel,
const base::ListValue& args);
base::ListValue args);

bool SendIPCMessageWithSender(bool internal,
bool send_to_all,
const std::string& channel,
const base::ListValue& args,
base::ListValue args,
int32_t sender_id = 0);

bool SendIPCMessageToFrame(bool internal,
bool send_to_all,
int32_t frame_id,
const std::string& channel,
const base::ListValue& args);
base::ListValue args);

// Send WebInputEvent to the page.
void SendInputEvent(v8::Isolate* isolate, v8::Local<v8::Value> input_event);
Expand Down Expand Up @@ -509,20 +509,21 @@ class WebContents : public mate::TrackableObject<WebContents>,
// mojom::ElectronBrowser
void Message(bool internal,
const std::string& channel,
base::Value arguments) override;
base::ListValue arguments) override;
void Invoke(const std::string& channel,
base::Value arguments,
base::ListValue arguments,
InvokeCallback callback) override;
void MessageSync(bool internal,
const std::string& channel,
base::Value arguments,
base::ListValue arguments,
MessageSyncCallback callback) override;
void MessageTo(bool internal,
bool send_to_all,
int32_t web_contents_id,
const std::string& channel,
base::Value arguments) override;
void MessageHost(const std::string& channel, base::Value arguments) override;
base::ListValue arguments) override;
void MessageHost(const std::string& channel,
base::ListValue arguments) override;
void UpdateDraggableRegions(
std::vector<mojom::DraggableRegionPtr> regions) override;
void SetTemporaryZoomLevel(double level) override;
Expand Down
5 changes: 4 additions & 1 deletion shell/browser/api/event.cc
Expand Up @@ -61,7 +61,10 @@ bool Event::SendReply(const base::Value& result) {
if (!callback_ || sender_ == nullptr)
return false;

std::move(*callback_).Run(result.Clone());
base::ListValue ret;
ret.GetList().push_back(result.Clone());

std::move(*callback_).Run(std::move(ret));
callback_.reset();
return true;
}
Expand Down
1 change: 1 addition & 0 deletions shell/common/api/BUILD.gn
Expand Up @@ -3,6 +3,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
mojom("mojo") {
sources = [
"api.mojom",
"native_types.mojom",
]

public_deps = [
Expand Down
15 changes: 8 additions & 7 deletions shell/common/api/api.mojom
@@ -1,15 +1,16 @@
module electron.mojom;

import "mojo/public/mojom/base/values.mojom";
import "mojo/public/mojom/base/string16.mojom";
import "ui/gfx/geometry/mojom/geometry.mojom";
import "electron/shell/common/api/native_types.mojom";
import "mojo/public/mojom/base/values.mojom";

interface ElectronRenderer {
Message(
bool internal,
bool send_to_all,
string channel,
mojo_base.mojom.ListValue arguments,
LegacyListValue arguments,
int32 sender_id);

UpdateCrashpadPipeName(string pipe_name);
Expand All @@ -32,21 +33,21 @@ interface ElectronBrowser {
Message(
bool internal,
string channel,
mojo_base.mojom.ListValue arguments);
LegacyListValue arguments);

// Emits an event on |channel| from the ipcMain JavaScript object in the main
// process, and returns the response.
Invoke(
string channel,
mojo_base.mojom.ListValue arguments) => (mojo_base.mojom.Value result);
LegacyListValue arguments) => (LegacyListValue result);

// Emits an event on |channel| from the ipcMain JavaScript object in the main
// process, and waits synchronously for a response.
[Sync]
MessageSync(
bool internal,
string channel,
mojo_base.mojom.ListValue arguments) => (mojo_base.mojom.Value result);
LegacyListValue arguments) => (LegacyListValue result);

// Emits an event from the |ipcRenderer| JavaScript object in the target
// WebContents's main frame, specified by |web_contents_id|.
Expand All @@ -55,11 +56,11 @@ interface ElectronBrowser {
bool send_to_all,
int32 web_contents_id,
string channel,
mojo_base.mojom.ListValue arguments);
LegacyListValue arguments);

MessageHost(
string channel,
mojo_base.mojom.ListValue arguments);
LegacyListValue arguments);

UpdateDraggableRegions(
array<DraggableRegion> regions);
Expand Down
4 changes: 4 additions & 0 deletions shell/common/api/native_types.mojom
@@ -0,0 +1,4 @@
module electron.mojom;

[Native]
struct LegacyListValue;
2 changes: 1 addition & 1 deletion shell/common/api/remote_callback_freer.cc
Expand Up @@ -55,7 +55,7 @@ void RemoteCallbackFreer::RunDestructor() {
(*iter)->GetRemoteAssociatedInterfaces()->GetInterface(
mojo::MakeRequest(&electron_ptr));
electron_ptr->Message(true /* internal */, false /* send_to_all */, channel,
args.Clone(), sender_id);
base::ListValue(args.Clone().GetList()), sender_id);
}

Observe(nullptr);
Expand Down
4 changes: 3 additions & 1 deletion shell/common/api/remote_object_freer.cc
Expand Up @@ -4,6 +4,8 @@

#include "shell/common/api/remote_object_freer.h"

#include <utility>

#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "content/public/renderer/render_frame.h"
Expand Down Expand Up @@ -79,7 +81,7 @@ void RemoteObjectFreer::RunDestructor() {
mojom::ElectronBrowserPtr electron_ptr;
render_frame->GetRemoteInterfaces()->GetInterface(
mojo::MakeRequest(&electron_ptr));
electron_ptr->Message(true, channel, args.Clone());
electron_ptr->Message(true, channel, std::move(args));
}

} // namespace electron
1 change: 1 addition & 0 deletions shell/common/api/typemaps.gni
@@ -0,0 +1 @@
typemaps = [ "//electron/shell/common/native_types.typemap" ]
9 changes: 9 additions & 0 deletions shell/common/native_types.typemap
@@ -0,0 +1,9 @@
mojom = "//electron/shell/common/api/native_types.mojom"
public_headers = []
traits_headers = [ "//ipc/ipc_message_utils.h" ]
deps = [
"//ipc"
]
type_mappings = [
"electron.mojom.LegacyListValue=::base::ListValue[move_only]",
]
34 changes: 17 additions & 17 deletions shell/renderer/api/atom_api_renderer_ipc.cc
Expand Up @@ -61,21 +61,23 @@ class IPCRenderer : public mate::Wrappable<IPCRenderer> {

void Send(bool internal,
const std::string& channel,
const base::ListValue& arguments) {
electron_browser_ptr_->Message(internal, channel, arguments.Clone());
base::ListValue arguments) {
electron_browser_ptr_->Message(internal, channel, std::move(arguments));
}

v8::Local<v8::Promise> Invoke(mate::Arguments* args,
const std::string& channel,
const base::Value& arguments) {
base::ListValue arguments) {
electron::util::Promise p(args->isolate());
auto handle = p.GetHandle();

electron_browser_ptr_->Invoke(
channel, arguments.Clone(),
base::BindOnce([](electron::util::Promise p,
base::Value result) { p.Resolve(result); },
std::move(p)));
channel, std::move(arguments),
base::BindOnce(
[](electron::util::Promise p, base::ListValue result) {
p.Resolve(result.GetList().at(0));
},
std::move(p)));

return handle;
}
Expand All @@ -84,25 +86,23 @@ class IPCRenderer : public mate::Wrappable<IPCRenderer> {
bool send_to_all,
int32_t web_contents_id,
const std::string& channel,
const base::ListValue& arguments) {
base::ListValue arguments) {
electron_browser_ptr_->MessageTo(internal, send_to_all, web_contents_id,
channel, arguments.Clone());
channel, std::move(arguments));
}

void SendToHost(const std::string& channel,
const base::ListValue& arguments) {
electron_browser_ptr_->MessageHost(channel, arguments.Clone());
void SendToHost(const std::string& channel, base::ListValue arguments) {
electron_browser_ptr_->MessageHost(channel, std::move(arguments));
}

base::Value SendSync(bool internal,
const std::string& channel,
const base::ListValue& arguments) {
base::Value result;
base::ListValue arguments) {
base::ListValue result;

electron_browser_ptr_->MessageSync(internal, channel, arguments.Clone(),
electron_browser_ptr_->MessageSync(internal, channel, std::move(arguments),
&result);

return result;
return std::move(result.GetList().at(0));
}

private:
Expand Down
2 changes: 1 addition & 1 deletion shell/renderer/electron_api_service_impl.cc
Expand Up @@ -131,7 +131,7 @@ void ElectronApiServiceImpl::OnConnectionError() {
void ElectronApiServiceImpl::Message(bool internal,
bool send_to_all,
const std::string& channel,
base::Value arguments,
base::ListValue arguments,
int32_t sender_id) {
// Don't handle browser messages before document element is created.
//
Expand Down
2 changes: 1 addition & 1 deletion shell/renderer/electron_api_service_impl.h
Expand Up @@ -28,7 +28,7 @@ class ElectronApiServiceImpl : public mojom::ElectronRenderer,
void Message(bool internal,
bool send_to_all,
const std::string& channel,
base::Value arguments,
base::ListValue arguments,
int32_t sender_id) override;
void UpdateCrashpadPipeName(const std::string& pipe_name) override;
void TakeHeapSnapshot(mojo::ScopedHandle file,
Expand Down