refactor: use v8 serialization for ipc

docs/api/breaking-changes.md

@@ -6,6 +6,59 @@ Breaking changes will be documented here, and deprecation warnings added to JS c
 
 The `FIXME` string is used in code comments to denote things that should be fixed for future releases. See https://github.com/electron/electron/search?q=fixme
 
+## Planned Breaking API Changes (8.0)
+
+### Values sent over IPC are now serialized with Structured Clone Algorithm
+
+The algorithm used to serialize objects sent over IPC (through
+`ipcRenderer.send`, `ipcRenderer.sendSync`, `WebContents.send` and related
+methods) has been switched from a custom algorithm to V8's built-in [Structured
+Clone Algorithm][SCA], the same algorithm used to serialize messages for
+`postMessage`. This brings about a 2x performance improvement for large
+messages, but also brings some breaking changes in behavior.
+
+- Sending Functions, Promises, WeakMaps, WeakSets, or objects containing any
+  such values, over IPC will now throw an exception, instead of silently
+  converting the functions to `undefined`.
+```js
+// Previously:
+ipcRenderer.send('channel', { value: 3, someFunction: () => {} })
+// => results in { value: 3 } arriving in the main process
+
+// From Electron 8:
+ipcRenderer.send('channel', { value: 3, someFunction: () => {} })
+// => throws Error("() => {} could not be cloned.")
+```
+- `NaN`, `Infinity` and `-Infinity` will now be correctly serialized, instead
+  of being converted to `null`.
+- Objects containing cyclic references will now be correctly serialized,
+  instead of being converted to `null`.
+- `Set`, `Map`, `Error` and `RegExp` values will be correctly serialized,
+  instead of being converted to `{}`.
+- `BigInt` values will be correctly serialized, instead of being converted to
+  `null`.
+- Sparse arrays will be serialized as such, instead of being converted to dense
+  arrays with `null`s.
+- `Date` objects will be transferred as `Date` objects, instead of being
+  converted to their ISO string representation.
+- Typed Arrays (such as `Uint8Array`, `Uint16Array`, `Uint32Array` and so on)
+  will be transferred as such, instead of being converted to Node.js `Buffer`.
+- Node.js `Buffer` objects will be transferred as `Uint8Array`s. You can
+  convert a `Uint8Array` back to a Node.js `Buffer` by wrapping the underlying
+  `ArrayBuffer`:
+```js
+Buffer.from(someUint8Array.buffer)
+```
+
+Sending any objects that aren't native JS types, such as DOM objects (e.g.
+`Element`, `Location`, `DOMMatrix`), Node.js objects (e.g. `process.env`,
+`Stream`), or Electron objects (e.g. `WebContents`, `BrowserWindow`,
+`WebFrame`) is deprecated. In Electron 8, these objects will be serialized as
+before with a DeprecationWarning message, but starting in Electron 9, sending
+these kinds of objects will throw a 'could not be cloned' error.
+
+[SCA]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
+
 ## Planned Breaking API Changes (7.0)
 
 ### Node Headers URL

filenames.gni

@@ -483,6 +483,7 @@ filenames = {
     "shell/common/gin_converters/net_converter.cc",
     "shell/common/gin_converters/net_converter.h",
     "shell/common/gin_converters/std_converter.h",
+    "shell/common/gin_converters/blink_converter_gin_adapter.h",
     "shell/common/gin_converters/value_converter_gin_adapter.h",
     "shell/common/gin_helper/callback.cc",
     "shell/common/gin_helper/callback.h",

lib/browser/chrome-extension.js

@@ -240,7 +240,7 @@ const getMessagesPath = (extensionId) => {
 
 ipcMainUtils.handleSync('CHROME_GET_MESSAGES', async function (event, extensionId) {
   const messagesPath = getMessagesPath(extensionId)
-  return fs.promises.readFile(messagesPath)
+  return fs.promises.readFile(messagesPath).then(b => b.toString('utf8'))
 })
 
 const validStorageTypes = new Set(['sync', 'local'])

lib/browser/guest-view-manager.js

@@ -23,7 +23,6 @@ const supportedWebViewEvents = [
   'devtools-opened',
   'devtools-closed',
   'devtools-focused',
-  'new-window',
   'will-navigate',
   'did-start-navigation',
   'did-navigate',
@@ -48,6 +47,13 @@ const supportedWebViewEvents = [
 const guestInstances = {}
 const embedderElementsMap = {}
 
+function sanitizeOptionsForGuest (options) {
+  const ret = { ...options }
+  // WebContents values can't be sent over IPC.
+  delete ret.webContents
+  return ret
+}
+
 // Create a new guest instance.
 const createGuest = function (embedder, params) {
   if (webViewManager == null) {
@@ -114,6 +120,12 @@ const createGuest = function (embedder, params) {
     fn(event)
   }
 
+  guest.on('new-window', function (event, url, frameName, disposition, options, additionalFeatures, referrer) {
+    sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT', 'new-window', url,
+      frameName, disposition, sanitizeOptionsForGuest(options),
+      additionalFeatures, referrer)
+  })
+
   // Dispatch guest's IPC messages to embedder.
   guest.on('ipc-message-host', function (_, channel, args) {
     sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE', channel, ...args)

lib/browser/guest-window-manager.js

@@ -250,8 +250,7 @@ ipcMainInternal.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', (event, url, fra
 
 // Routed window.open messages with fully parsed options
 ipcMainInternal.on('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', function (event, url, referrer,
-  frameName, disposition, options,
-  additionalFeatures, postData) {
+  frameName, disposition, options, additionalFeatures, postData) {
   options = mergeBrowserWindowOptions(event.sender, options)
   event.sender.emit('new-window', event, url, frameName, disposition, options, additionalFeatures, referrer)
   const { newGuest } = event

lib/browser/rpc-server.js

@@ -114,7 +114,7 @@ ipcMainUtils.handleSync('ELECTRON_BROWSER_SANDBOX_LOAD', async function (event)
     process: {
       arch: process.arch,
       platform: process.platform,
-      env: process.env,
+      env: { ...process.env },
       version: process.version,
       versions: process.versions,
       execPath: process.helperExecPath

patches/chromium/.patches

@@ -79,4 +79,5 @@ expose_setuseragent_on_networkcontext.patch
 feat_add_set_theme_source_to_allow_apps_to.patch
 revert_cleanup_remove_menu_subtitles_sublabels.patch
 ui_views_fix_jumbo_build.patch
+export_fetchapi_mojo_traits_to_fix_component_build.patch
 fix_windows_build.patch

patches/chromium/export_fetchapi_mojo_traits_to_fix_component_build.patch

@@ -0,0 +1,36 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <jeremya@chromium.org>
+Date: Fri, 20 Sep 2019 16:44:18 -0400
+Subject: export FetchAPI mojo traits to fix component build
+
+Without these, we get link errors in the component build when using the
+blink::CloneableMessage mojo traits.
+
+diff --git a/third_party/blink/public/common/fetch/fetch_api_request_body_mojom_traits.h b/third_party/blink/public/common/fetch/fetch_api_request_body_mojom_traits.h
+index 1ddfc2108f9a0104247ea2559b597552cd20a342..f9a10b5b428e29a8824b87616f19292b38e38024 100644
+--- a/third_party/blink/public/common/fetch/fetch_api_request_body_mojom_traits.h
++++ b/third_party/blink/public/common/fetch/fetch_api_request_body_mojom_traits.h
+@@ -11,12 +11,13 @@
+ #include "mojo/public/cpp/bindings/pending_remote.h"
+ #include "services/network/public/cpp/resource_request_body.h"
+ #include "services/network/public/mojom/url_loader.mojom-forward.h"
++#include "third_party/blink/public/common/common_export.h"
+ #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-forward.h"
+
+ namespace mojo {
+
+ template <>
+-struct StructTraits<blink::mojom::FetchAPIRequestBodyDataView,
++struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::FetchAPIRequestBodyDataView,
+                     scoped_refptr<network::ResourceRequestBody>> {
+   static bool IsNull(const scoped_refptr<network::ResourceRequestBody>& r) {
+     return !r;
+@@ -46,7 +47,7 @@ struct StructTraits<blink::mojom::FetchAPIRequestBodyDataView,
+ };
+
+ template <>
+-struct StructTraits<blink::mojom::FetchAPIDataElementDataView,
++struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::FetchAPIDataElementDataView,
+                     network::DataElement> {
+   static const network::mojom::DataElementType& type(
+       const network::DataElement& element) {

patches/chromium/gin_with_namespace.patch

@@ -12,7 +12,7 @@ native_mate, and we should remove this patch once native_mate is erased
 from Electron.
 
 diff --git a/gin/arguments.h b/gin/arguments.h
-index eaded13e2991..03e1495566d1 100644
+index eaded13e29919793494dfe2f7f85fad7dcb125cf..03e1495566d1ab561dcd67517053173911288cea 100644
 --- a/gin/arguments.h
 +++ b/gin/arguments.h
 @@ -28,14 +28,14 @@ class GIN_EXPORT Arguments {
@@ -60,7 +60,7 @@ index eaded13e2991..03e1495566d1 100644
      (is_for_property_ ? info_for_property_->GetReturnValue()
                        : info_for_function_->GetReturnValue())
 diff --git a/gin/converter.h b/gin/converter.h
-index 27b4d0acd016..b19209a8534a 100644
+index 27b4d0acd016df378e4cb44ccda1a433244fe2c6..b19209a8534a497373c5a2f861b26502e96144c9 100644
 --- a/gin/converter.h
 +++ b/gin/converter.h
 @@ -250,7 +250,7 @@ std::enable_if_t<ToV8ReturnsMaybe<T>::value, bool> TryConvertToV8(

patches/node/.patches

@@ -39,3 +39,4 @@ chore_split_createenvironment_into_createenvironment_and.patch
 chore_handle_default_configuration_not_being_set_in_the_electron_env.patch
 revert_crypto_add_outputlength_option_to_crypto_createhash.patch
 add_openssl_is_boringssl_guard_to_oaep_hash_check.patch
+fix_microtasks.patch

patches/node/fix_microtasks.patch

@@ -0,0 +1,58 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <nornagon@nornagon.net>
+Date: Mon, 16 Sep 2019 17:50:28 -0400
+Subject: fix microtasks
+
+backports https://github.com/nodejs/node/pull/29581 and https://github.com/nodejs/node/pull/29434
+
+diff --git a/src/api/callback.cc b/src/api/callback.cc
+index 43ccfafd9f2c85e23a9ea6277e88e4864e287905..3c518870c9c8d92f3dfcd6c270f5e023e3b69633 100644
+--- a/src/api/callback.cc
++++ b/src/api/callback.cc
+@@ -12,6 +12,7 @@ using v8::HandleScope;
+ using v8::Isolate;
+ using v8::Local;
+ using v8::MaybeLocal;
++using v8::MicrotasksScope;
+ using v8::NewStringType;
+ using v8::Object;
+ using v8::String;
+@@ -100,7 +101,7 @@ void InternalCallbackScope::Close() {
+
+   if (!env_->can_call_into_js()) return;
+   if (!tick_info->has_tick_scheduled()) {
+-    env_->isolate()->RunMicrotasks();
++    MicrotasksScope::PerformCheckpoint(env_->isolate());
+   }
+
+ #if 0  // FIXME(codebytere): figure out why this check fails/causes crash
+diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc
+index e6b4d0b8e211cdb1fef4759457c2550e28448360..918796ba77d80cf66324164a930f8068e0622ccb 100644
+--- a/src/node_task_queue.cc
++++ b/src/node_task_queue.cc
+@@ -21,6 +21,7 @@ using v8::kPromiseRejectWithNoHandler;
+ using v8::kPromiseResolveAfterResolved;
+ using v8::Local;
+ using v8::Message;
++using v8::MicrotasksScope;
+ using v8::Number;
+ using v8::Object;
+ using v8::Promise;
+@@ -43,7 +44,7 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
+ bool RunNextTicksNative(Environment* env) {
+   TickInfo* tick_info = env->tick_info();
+   if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
+-    env->isolate()->RunMicrotasks();
++    MicrotasksScope::PerformCheckpoint(env->isolate());
+   if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
+     return true;
+
+@@ -54,7 +55,7 @@ bool RunNextTicksNative(Environment* env) {
+ }
+
+ static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
+-  args.GetIsolate()->RunMicrotasks();
++  MicrotasksScope::PerformCheckpoint(args.GetIsolate());
+ }
+
+ static void SetTickCallback(const FunctionCallbackInfo<Value>& args) {

shell/browser/api/atom_api_web_contents.cc

@@ -1000,7 +1000,7 @@ void WebContents::OnElectronBrowserConnectionError() {
 
 void WebContents::Message(bool internal,
                           const std::string& channel,
-                          base::Value arguments) {
+                          blink::CloneableMessage arguments) {
   // webContents.emit('-ipc-message', new Event(), internal, channel,
   // arguments);
   EmitWithSender("-ipc-message", bindings_.dispatch_context(), base::nullopt,
@@ -1009,7 +1009,7 @@ void WebContents::Message(bool internal,
 
 void WebContents::Invoke(bool internal,
                          const std::string& channel,
-                         base::Value arguments,
+                         blink::CloneableMessage arguments,
                          InvokeCallback callback) {
   // webContents.emit('-ipc-invoke', new Event(), internal, channel, arguments);
   EmitWithSender("-ipc-invoke", bindings_.dispatch_context(),
@@ -1018,7 +1018,7 @@ void WebContents::Invoke(bool internal,
 
 void WebContents::MessageSync(bool internal,
                               const std::string& channel,
-                              base::Value arguments,
+                              blink::CloneableMessage arguments,
                               MessageSyncCallback callback) {
   // webContents.emit('-ipc-message-sync', new Event(sender, message), internal,
   // channel, arguments);
@@ -1030,24 +1030,35 @@ void WebContents::MessageTo(bool internal,
                             bool send_to_all,
                             int32_t web_contents_id,
                             const std::string& channel,
-                            base::Value arguments) {
+                            blink::CloneableMessage 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());
+                                           arguments.ShallowClone(), ID());
   }
 }
 
 void WebContents::MessageHost(const std::string& channel,
-                              base::Value arguments) {
+                              blink::CloneableMessage arguments) {
   // webContents.emit('ipc-message-host', new Event(), channel, args);
   EmitWithSender("ipc-message-host", bindings_.dispatch_context(),
                  base::nullopt, channel, std::move(arguments));
 }
 
+void WebContents::DereferenceRemoteJSObject(const std::string& context_id,
+                                            int object_id,
+                                            int ref_count) {
+  base::ListValue args;
+  args.AppendString(context_id);
+  args.AppendInteger(object_id);
+  args.AppendInteger(ref_count);
+  EmitWithSender("-ipc-message", bindings_.dispatch_context(), base::nullopt,
+                 /* internal */ true, "ELECTRON_BROWSER_DEREFERENCE",
+                 std::move(args));
+}
+
 void WebContents::UpdateDraggableRegions(
     std::vector<mojom::DraggableRegionPtr> regions) {
   for (ExtendedWebContentsObserver& observer : observers_)
@@ -1919,14 +1930,21 @@ 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);
+                                 v8::Local<v8::Value> args) {
+  blink::CloneableMessage message;
+  if (!mate::ConvertFromV8(isolate(), args, &message)) {
+    isolate()->ThrowException(v8::Exception::Error(
+        mate::StringToV8(isolate(), "Failed to serialize arguments")));
+    return false;
+  }
+  return SendIPCMessageWithSender(internal, send_to_all, channel,
+                                  std::move(message));
 }
 
 bool WebContents::SendIPCMessageWithSender(bool internal,
                                            bool send_to_all,
                                            const std::string& channel,
-                                           const base::ListValue& args,
+                                           blink::CloneableMessage args,
                                            int32_t sender_id) {
   std::vector<content::RenderFrameHost*> target_hosts;
   if (!send_to_all) {
@@ -1942,7 +1960,7 @@ 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, std::move(args), sender_id);
   }
   return true;
 }
@@ -1951,7 +1969,13 @@ bool WebContents::SendIPCMessageToFrame(bool internal,
                                         bool send_to_all,
                                         int32_t frame_id,
                                         const std::string& channel,
-                                        const base::ListValue& args) {
+                                        v8::Local<v8::Value> args) {
+  blink::CloneableMessage message;
+  if (!mate::ConvertFromV8(isolate(), args, &message)) {
+    isolate()->ThrowException(v8::Exception::Error(
+        mate::StringToV8(isolate(), "Failed to serialize arguments")));
+    return false;
+  }
   auto frames = web_contents()->GetAllFrames();
   auto iter = std::find_if(frames.begin(), frames.end(), [frame_id](auto* f) {
     return f->GetRoutingID() == frame_id;
@@ -1964,7 +1988,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(message),
                         0 /* sender_id */);
   return true;
 }