refactor: use v8 serialization for ipc

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'])

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);
@@ -1048,6 +1048,18 @@ void WebContents::MessageHost(const std::string& channel,
                  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_)

shell/browser/api/atom_api_web_contents.h

@@ -488,21 +488,24 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // mojom::ElectronBrowser
   void Message(bool internal,
                const std::string& channel,
-               base::Value arguments) override;
+               blink::CloneableMessage arguments) override;
   void Invoke(bool internal,
               const std::string& channel,
-              base::Value arguments,
+              blink::CloneableMessage arguments,
               InvokeCallback callback) override;
   void MessageSync(bool internal,
                    const std::string& channel,
-                   base::Value arguments,
+                   blink::CloneableMessage 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;
+  void DereferenceRemoteJSObject(const std::string& context_id,
+                                 int object_id,
+                                 int ref_count) override;
   void UpdateDraggableRegions(
       std::vector<mojom::DraggableRegionPtr> regions) override;
   void SetTemporaryZoomLevel(double level) override;

shell/browser/api/event.cc

@@ -7,7 +7,7 @@
 #include <utility>
 
 #include "native_mate/object_template_builder_deprecated.h"
-#include "shell/common/native_mate_converters/value_converter.h"
+#include "shell/common/native_mate_converters/blink_converter.h"
 
 namespace mate {
 
@@ -17,7 +17,7 @@ Event::Event(v8::Isolate* isolate) {
 
 Event::~Event() = default;
 
-void Event::SetCallback(base::Optional<MessageSyncCallback> callback) {
+void Event::SetCallback(base::Optional<InvokeCallback> callback) {
   DCHECK(!callback_);
   callback_ = std::move(callback);
 }
@@ -29,11 +29,11 @@ void Event::PreventDefault(v8::Isolate* isolate) {
       .Check();
 }
 
-bool Event::SendReply(const base::Value& result) {
+bool Event::SendReply(const blink::CloneableMessage& result) {
   if (!callback_)
     return false;
 
-  std::move(*callback_).Run(result.Clone());
+  std::move(*callback_).Run(result.ShallowClone());
   callback_.reset();
   return true;
 }

shell/browser/api/event.h

@@ -18,30 +18,29 @@ namespace mate {
 
 class Event : public Wrappable<Event> {
  public:
-  using MessageSyncCallback =
-      electron::mojom::ElectronBrowser::MessageSyncCallback;
+  using InvokeCallback = electron::mojom::ElectronBrowser::InvokeCallback;
   static Handle<Event> Create(v8::Isolate* isolate);
 
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::FunctionTemplate> prototype);
 
   // Pass the callback to be invoked.
-  void SetCallback(base::Optional<MessageSyncCallback> callback);
+  void SetCallback(base::Optional<InvokeCallback> callback);
 
   // event.PreventDefault().
   void PreventDefault(v8::Isolate* isolate);
 
   // event.sendReply(value), used for replying to synchronous messages and
   // `invoke` calls.
-  bool SendReply(const base::Value& result);
+  bool SendReply(const blink::CloneableMessage& result);
 
  protected:
   explicit Event(v8::Isolate* isolate);
   ~Event() override;
 
  private:
   // Replyer for the synchronous messages.
-  base::Optional<MessageSyncCallback> callback_;
+  base::Optional<InvokeCallback> callback_;
 
   DISALLOW_COPY_AND_ASSIGN(Event);
 };

shell/browser/api/event_emitter.h

@@ -82,8 +82,7 @@ class EventEmitter : public Wrappable<T> {
   bool EmitWithSender(
       base::StringPiece name,
       content::RenderFrameHost* sender,
-      base::Optional<electron::mojom::ElectronBrowser::MessageSyncCallback>
-          callback,
+      base::Optional<electron::mojom::ElectronBrowser::InvokeCallback> callback,
       Args&&... args) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     v8::Locker locker(isolate());

shell/browser/net/node_stream_loader.cc

@@ -91,10 +91,18 @@ void NodeStreamLoader::NotifyComplete(int result) {
 }
 
 void NodeStreamLoader::ReadMore() {
+  if (is_reading_) {
+    // Calling read() can trigger the "readable" event again, making this
+    // function re-entrant. If we're already reading, we don't want to start
+    // a nested read, so short-circuit.
+    return;
+  }
   is_reading_ = true;
+  auto weak = weak_factory_.GetWeakPtr();
   // buffer = emitter.read()
   v8::MaybeLocal<v8::Value> ret = node::MakeCallback(
       isolate_, emitter_.Get(isolate_), "read", 0, nullptr, {0, 0});
+  DCHECK(weak) << "We shouldn't have been destroyed when calling read()";
 
   // If there is no buffer read, wait until |readable| is emitted again.
   v8::Local<v8::Value> buffer;
@@ -110,13 +118,12 @@ void NodeStreamLoader::ReadMore() {
   // Write buffer to mojo pipe asyncronously.
   is_reading_ = false;
   is_writing_ = true;
-  producer_->Write(
-      std::make_unique<mojo::StringDataSource>(
-          base::StringPiece(node::Buffer::Data(buffer),
-                            node::Buffer::Length(buffer)),
-          mojo::StringDataSource::AsyncWritingMode::
-              STRING_STAYS_VALID_UNTIL_COMPLETION),
-      base::BindOnce(&NodeStreamLoader::DidWrite, weak_factory_.GetWeakPtr()));
+  producer_->Write(std::make_unique<mojo::StringDataSource>(
+                       base::StringPiece(node::Buffer::Data(buffer),
+                                         node::Buffer::Length(buffer)),
+                       mojo::StringDataSource::AsyncWritingMode::
+                           STRING_STAYS_VALID_UNTIL_COMPLETION),
+                   base::BindOnce(&NodeStreamLoader::DidWrite, weak));
 }
 
 void NodeStreamLoader::DidWrite(MojoResult result) {

shell/common/api/BUILD.gn

@@ -7,6 +7,7 @@ mojom("mojo") {
 
   public_deps = [
     "//mojo/public/mojom/base",
+    "//third_party/blink/public/mojom:mojom_core",
     "//ui/gfx/geometry/mojom",
   ]
 }

shell/common/api/api.mojom

@@ -3,6 +3,7 @@ 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 "third_party/blink/public/mojom/messaging/cloneable_message.mojom";
 
 interface ElectronRenderer {
   Message(
@@ -37,14 +38,14 @@ interface ElectronBrowser {
   Message(
       bool internal,
       string channel,
-      mojo_base.mojom.ListValue arguments);
+      blink.mojom.CloneableMessage arguments);
 
   // Emits an event on |channel| from the ipcMain JavaScript object in the main
   // process, and returns the response.
   Invoke(
       bool internal,
       string channel,
-      mojo_base.mojom.ListValue arguments) => (mojo_base.mojom.Value result);
+      blink.mojom.CloneableMessage arguments) => (blink.mojom.CloneableMessage result);
 
   // Emits an event on |channel| from the ipcMain JavaScript object in the main
   // process, and waits synchronously for a response.
@@ -55,7 +56,7 @@ interface ElectronBrowser {
   MessageSync(
     bool internal,
     string channel,
-    mojo_base.mojom.ListValue arguments) => (mojo_base.mojom.Value result);
+    blink.mojom.CloneableMessage arguments) => (blink.mojom.CloneableMessage result);
 
   // Emits an event from the |ipcRenderer| JavaScript object in the target
   // WebContents's main frame, specified by |web_contents_id|.
@@ -70,6 +71,13 @@ interface ElectronBrowser {
     string channel,
     mojo_base.mojom.ListValue arguments);
 
+  // This is an API specific to the "remote" module, and will ultimately be
+  // replaced by generic IPC once WeakRef is generally available.
+  DereferenceRemoteJSObject(
+    string context_id,
+    int32 object_id,
+    int32 ref_count);
+
   UpdateDraggableRegions(
     array<DraggableRegion> regions);
 

shell/common/api/remote/remote_object_freer.cc

@@ -8,6 +8,8 @@
 #include "base/values.h"
 #include "content/public/renderer/render_frame.h"
 #include "electron/shell/common/api/api.mojom.h"
+#include "electron/shell/common/native_mate_converters/blink_converter.h"
+#include "electron/shell/common/native_mate_converters/value_converter.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 
@@ -80,17 +82,10 @@ void RemoteObjectFreer::RunDestructor() {
       ref_mapper_.erase(objects_it);
   }
 
-  auto* channel = "ELECTRON_BROWSER_DEREFERENCE";
-
-  base::ListValue args;
-  args.AppendString(context_id_);
-  args.AppendInteger(object_id_);
-  args.AppendInteger(ref_count);
-
   mojom::ElectronBrowserAssociatedPtr electron_ptr;
   render_frame->GetRemoteAssociatedInterfaces()->GetInterface(
       mojo::MakeRequest(&electron_ptr));
-  electron_ptr->Message(true, channel, args.Clone());
+  electron_ptr->DereferenceRemoteJSObject(context_id_, object_id_, ref_count);
 }
 
 }  // namespace electron

shell/common/gin_converters/blink_converter_gin_adapter.h

@@ -0,0 +1,30 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_GIN_CONVERTERS_BLINK_CONVERTER_GIN_ADAPTER_H_
+#define SHELL_COMMON_GIN_CONVERTERS_BLINK_CONVERTER_GIN_ADAPTER_H_
+
+#include "gin/converter.h"
+#include "shell/common/native_mate_converters/blink_converter.h"
+
+// TODO(zcbenz): Move the implementations from native_mate_converters to here.
+
+namespace gin {
+
+template <>
+struct Converter<blink::CloneableMessage> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     blink::CloneableMessage* out) {
+    return mate::ConvertFromV8(isolate, val, out);
+  }
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const blink::CloneableMessage& val) {
+    return mate::ConvertToV8(isolate, val);
+  }
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_BLINK_CONVERTER_GIN_ADAPTER_H_