Browse Source

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

Jeremy Apthorp 5 years ago
parent
commit
e7576019e9

+ 1 - 0
patches/chromium/.patches

@@ -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 - 0
patches/chromium/typemap.patch

@@ -0,0 +1,21 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <[email protected]>
+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",

+ 14 - 13
shell/browser/api/atom_api_web_contents.cc

@@ -1023,7 +1023,7 @@ 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,
@@ -1031,7 +1031,7 @@ void WebContents::Message(bool internal,
 }
 
 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(),
@@ -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);
@@ -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));
@@ -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) {
@@ -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;
 }
@@ -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;
@@ -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;
 }

+ 9 - 8
shell/browser/api/atom_api_web_contents.h

@@ -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);
@@ -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;

+ 4 - 1
shell/browser/api/event.cc

@@ -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;
 }

+ 1 - 0
shell/common/api/BUILD.gn

@@ -3,6 +3,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
 mojom("mojo") {
   sources = [
     "api.mojom",
+    "native_types.mojom",
   ]
 
   public_deps = [

+ 8 - 7
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);
@@ -32,13 +33,13 @@ 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.
@@ -46,7 +47,7 @@ interface ElectronBrowser {
   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|.
@@ -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);

+ 4 - 0
shell/common/api/native_types.mojom

@@ -0,0 +1,4 @@
+module electron.mojom;
+
+[Native]
+struct LegacyListValue;

+ 1 - 1
shell/common/api/remote_callback_freer.cc

@@ -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);

+ 3 - 1
shell/common/api/remote_object_freer.cc

@@ -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"
@@ -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 - 0
shell/common/api/typemaps.gni

@@ -0,0 +1 @@
+typemaps = [ "//electron/shell/common/native_types.typemap" ]

+ 9 - 0
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]",
+]

+ 17 - 17
shell/renderer/api/atom_api_renderer_ipc.cc

@@ -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;
   }
@@ -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:

+ 1 - 1
shell/renderer/electron_api_service_impl.cc

@@ -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.
   //

+ 1 - 1
shell/renderer/electron_api_service_impl.h

@@ -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,