refactor: use mojo for electron internal IPC (#17406)

* refactor: use mojo for electron internal IPC

* add sender_id, drop MessageSync

* remove usages of AtomFrameMsg_Message

* iwyu

* first draft of renderer->browser direction

* refactor to reuse a single ipc interface

* implement TakeHeapSnapshot through mojo

* the rest of the owl^WtakeHeapSnapshot mojofication

* remove no-op overrides in AtomRendererClient

* delete renderer-side ElectronApiServiceImpl when its pipe is destroyed

* looks like we don't need to overlay the renderer manifest after all

* don't try to send 2 replies to a sync rpc

* undo changes to manifests.cc

* unify sandboxed + unsandboxed ipc events

* lint

* register ElectronBrowser mojo service on devtools WebContents

* fix takeHeapSnapshopt failure paths

* {electron_api => atom}::mojom

* add send_to_all to ElectronRenderer::Message

* keep interface alive until callback is called

* review comments

* use GetContext from RendererClientBase

* robustify a test that uses window.open

* MessageSync posts a task to put sync messages in the same queue as async ones

* add v8::MicrotasksScope and node::CallbackScope

* iwyu

* use weakptr to api::WebContents instead of Unretained

* make MessageSync an asynchronous message & use non-associated interface

* iwyu + comments

* remove unused WeakPtrFactory

* inline OnRendererMessage[Sync]

* cleanups & comments

* use helper methods instead of inline lambdas

* remove unneeded async in test

* add mojo to manifests deps

* add gn check for //electron/manifests and mojo

* don't register renderer side service until preload has been run

* update gn check targets list

* move interface registration back to RenderFrameCreated

.circleci/config.yml

@@ -237,6 +237,8 @@ step-gn-check: &step-gn-check
       cd src
       gn check out/Default //electron:electron_lib
       gn check out/Default //electron:electron_app
+      gn check out/Default //electron:manifests
+      gn check out/Default //electron/atom/common/api:mojo
 
 step-electron-build: &step-electron-build
   run:

BUILD.gn

@@ -361,8 +361,10 @@ source_set("manifests") {
   include_dirs = [ "//electron" ]
 
   deps = [
+    "//electron/atom/common/api:mojo",
     "//printing/buildflags",
     "//services/proxy_resolver/public/cpp:manifest",
+    "//services/service_manager/public/cpp",
   ]
 
   if (enable_basic_printing) {
@@ -384,6 +386,7 @@ static_library("electron_lib") {
     ":atom_js2c",
     ":manifests",
     ":resources",
+    "atom/common/api:mojo",
     "buildflags",
     "chromium_src:chrome",
     "native_mate",

appveyor.yml

@@ -58,6 +58,8 @@ build_script:
   - gn gen out/Default "--args=import(\"%BUILD_CONFIG_PATH%\") %GN_EXTRA_ARGS%"
   - gn check out/Default //electron:electron_lib
   - gn check out/Default //electron:electron_app
+  - gn check out/Default //electron:manifests
+  - gn check out/Default //electron/atom/common/api:mojo
   - ninja -C out/Default electron:electron_app
   - gn gen out/ffmpeg "--args=import(\"//electron/build/args/ffmpeg.gn\") %GN_EXTRA_ARGS%"
   - ninja -C out/ffmpeg electron:electron_ffmpeg_zip

atom/app/manifests.cc

@@ -5,6 +5,7 @@
 #include "atom/app/manifests.h"
 
 #include "base/no_destructor.h"
+#include "electron/atom/common/api/api.mojom.h"
 #include "printing/buildflags/buildflags.h"
 #include "services/proxy_resolver/public/cpp/manifest.h"
 #include "services/service_manager/public/cpp/manifest_builder.h"
@@ -25,6 +26,10 @@ const service_manager::Manifest& GetElectronContentBrowserOverlayManifest() {
           .RequireCapability("proxy_resolver", "factory")
           .RequireCapability("chrome_printing", "converter")
           .RequireCapability("pdf_compositor", "compositor")
+          .ExposeInterfaceFilterCapability_Deprecated(
+              "navigation:frame", "renderer",
+              service_manager::Manifest::InterfaceList<
+                  atom::mojom::ElectronBrowser>())
           .Build()};
   return *manifest;
 }

atom/browser/api/atom_api_event.cc

@@ -10,7 +10,7 @@ namespace {
 
 v8::Local<v8::Object> CreateWithSender(v8::Isolate* isolate,
                                        v8::Local<v8::Object> sender) {
-  return mate::internal::CreateJSEvent(isolate, sender, nullptr, nullptr);
+  return mate::internal::CreateJSEvent(isolate, sender, nullptr, base::nullopt);
 }
 
 void Initialize(v8::Local<v8::Object> exports,

atom/browser/api/atom_api_web_contents.cc

@@ -78,10 +78,13 @@
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/context_menu_params.h"
+#include "electron/atom/common/api/api.mojom.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "native_mate/converter.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
 #include "net/url_request/url_request_context.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/mojom/frame/find_in_page.mojom.h"
 #include "third_party/blink/public/platform/web_input_event.h"
 #include "ui/display/screen.h"
@@ -260,14 +263,6 @@ struct WebContents::FrameDispatchHelper {
   void OnGetZoomLevel(IPC::Message* reply_msg) {
     api_web_contents->OnGetZoomLevel(rfh, reply_msg);
   }
-
-  void OnRendererMessageSync(bool internal,
-                             const std::string& channel,
-                             const base::ListValue& args,
-                             IPC::Message* message) {
-    api_web_contents->OnRendererMessageSync(rfh, internal, channel, args,
-                                            message);
-  }
 };
 
 WebContents::WebContents(v8::Isolate* isolate,
@@ -278,6 +273,8 @@ WebContents::WebContents(v8::Isolate* isolate,
   Init(isolate);
   AttachAsUserData(web_contents);
   InitZoomController(web_contents, mate::Dictionary::CreateEmpty(isolate));
+  registry_.AddInterface(base::BindRepeating(&WebContents::BindElectronBrowser,
+                                             base::Unretained(this)));
 }
 
 WebContents::WebContents(v8::Isolate* isolate,
@@ -425,6 +422,9 @@ void WebContents::InitWithSessionAndOptions(
   // Initialize zoom controller.
   InitZoomController(web_contents(), options);
 
+  registry_.AddInterface(base::BindRepeating(&WebContents::BindElectronBrowser,
+                                             base::Unretained(this)));
+
   web_contents()->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
                                        false);
 
@@ -823,6 +823,13 @@ void WebContents::DidChangeThemeColor(SkColor theme_color) {
   }
 }
 
+void WebContents::OnInterfaceRequestFromFrame(
+    content::RenderFrameHost* render_frame_host,
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle* interface_pipe) {
+  registry_.TryBindInterface(interface_name, interface_pipe, render_frame_host);
+}
+
 void WebContents::DocumentLoadedInFrame(
     content::RenderFrameHost* render_frame_host) {
   if (!render_frame_host->GetParent())
@@ -886,6 +893,47 @@ bool WebContents::EmitNavigationEvent(
               frame_routing_id);
 }
 
+void WebContents::BindElectronBrowser(
+    mojom::ElectronBrowserRequest request,
+    content::RenderFrameHost* render_frame_host) {
+  auto id = bindings_.AddBinding(this, std::move(request), render_frame_host);
+  frame_to_bindings_map_[render_frame_host].push_back(id);
+}
+
+void WebContents::Message(bool internal,
+                          const std::string& channel,
+                          base::Value 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::MessageSync(bool internal,
+                              const std::string& channel,
+                              base::Value arguments,
+                              MessageSyncCallback callback) {
+  // webContents.emit('-ipc-message-sync', new Event(sender, message), internal,
+  // channel, arguments);
+  EmitWithSender("-ipc-message-sync", bindings_.dispatch_context(),
+                 std::move(callback), internal, channel, std::move(arguments));
+}
+
+void WebContents::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+  // A RenderFrameHost can be destroyed before the related Mojo binding is
+  // closed, which can result in Mojo calls being sent for RenderFrameHosts
+  // that no longer exist. To prevent this from happening, when a
+  // RenderFrameHost goes away, we close all the bindings related to that
+  // frame.
+  auto it = frame_to_bindings_map_.find(render_frame_host);
+  if (it == frame_to_bindings_map_.end())
+    return;
+  for (auto id : it->second)
+    bindings_.RemoveBinding(id);
+  frame_to_bindings_map_.erase(it);
+}
+
 void WebContents::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   EmitNavigationEvent("did-start-navigation", navigation_handle);
@@ -1049,9 +1097,6 @@ bool WebContents::OnMessageReceived(const IPC::Message& message,
   bool handled = true;
   FrameDispatchHelper helper = {this, frame_host};
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(WebContents, message, frame_host)
-    IPC_MESSAGE_HANDLER(AtomFrameHostMsg_Message, OnRendererMessage)
-    IPC_MESSAGE_FORWARD_DELAY_REPLY(AtomFrameHostMsg_Message_Sync, &helper,
-                                    FrameDispatchHelper::OnRendererMessageSync)
     IPC_MESSAGE_HANDLER(AtomFrameHostMsg_Message_To, OnRendererMessageTo)
     IPC_MESSAGE_HANDLER(AtomFrameHostMsg_Message_Host, OnRendererMessageHost)
     IPC_MESSAGE_FORWARD_DELAY_REPLY(
@@ -1659,14 +1704,13 @@ bool WebContents::SendIPCMessageWithSender(bool internal,
     target_hosts = web_contents()->GetAllFrames();
   }
 
-  bool handled = false;
   for (auto* frame_host : target_hosts) {
-    handled = frame_host->Send(
-                  new AtomFrameMsg_Message(frame_host->GetRoutingID(), internal,
-                                           false, channel, args, sender_id)) ||
-              handled;
+    mojom::ElectronRendererAssociatedPtr electron_ptr;
+    frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
+        mojo::MakeRequest(&electron_ptr));
+    electron_ptr->Message(internal, false, channel, args.Clone(), sender_id);
   }
-  return handled;
+  return true;
 }
 
 bool WebContents::SendIPCMessageToFrame(bool internal,
@@ -1682,8 +1726,13 @@ bool WebContents::SendIPCMessageToFrame(bool internal,
     return false;
   if (!(*iter)->IsRenderFrameLive())
     return false;
-  return (*iter)->Send(new AtomFrameMsg_Message(
-      frame_id, internal, send_to_all, channel, args, 0 /* sender_id */));
+
+  mojom::ElectronRendererAssociatedPtr electron_ptr;
+  (*iter)->GetRemoteAssociatedInterfaces()->GetInterface(
+      mojo::MakeRequest(&electron_ptr));
+  electron_ptr->Message(internal, send_to_all, channel, args.Clone(),
+                        0 /* sender_id */);
+  return true;
 }
 
 void WebContents::SendInputEvent(v8::Isolate* isolate,
@@ -2062,22 +2111,36 @@ void WebContents::GrantOriginAccess(const GURL& url) {
       url::Origin::Create(url));
 }
 
-bool WebContents::TakeHeapSnapshot(const base::FilePath& file_path,
-                                   const std::string& channel) {
+void WebContents::TakeHeapSnapshot(const base::FilePath& file_path,
+                                   base::Callback<void(bool)> callback) {
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
   base::File file(file_path,
                   base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
-  if (!file.IsValid())
-    return false;
+  if (!file.IsValid()) {
+    std::move(callback).Run(false);
+    return;
+  }
 
   auto* frame_host = web_contents()->GetMainFrame();
-  if (!frame_host)
-    return false;
+  if (!frame_host) {
+    std::move(callback).Run(false);
+    return;
+  }
 
-  return frame_host->Send(new AtomFrameMsg_TakeHeapSnapshot(
-      frame_host->GetRoutingID(),
-      IPC::TakePlatformFileForTransit(std::move(file)), channel));
+  // This dance with `base::Owned` is to ensure that the interface stays alive
+  // until the callback is called. Otherwise it would be closed at the end of
+  // this function.
+  auto electron_ptr = std::make_unique<mojom::ElectronRendererAssociatedPtr>();
+  frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
+      mojo::MakeRequest(electron_ptr.get()));
+  auto* raw_ptr = electron_ptr.get();
+  (*raw_ptr)->TakeHeapSnapshot(
+      mojo::WrapPlatformFile(file.TakePlatformFile()),
+      base::BindOnce([](mojom::ElectronRendererAssociatedPtr* ep,
+                        base::Callback<void(bool)> callback,
+                        bool success) { callback.Run(success); },
+                     base::Owned(std::move(electron_ptr)), callback));
 }
 
 // static
@@ -2195,25 +2258,6 @@ AtomBrowserContext* WebContents::GetBrowserContext() const {
   return static_cast<AtomBrowserContext*>(web_contents()->GetBrowserContext());
 }
 
-void WebContents::OnRendererMessage(content::RenderFrameHost* frame_host,
-                                    bool internal,
-                                    const std::string& channel,
-                                    const base::ListValue& args) {
-  // webContents.emit('-ipc-message', new Event(), internal, channel, args);
-  EmitWithSender("-ipc-message", frame_host, nullptr, internal, channel, args);
-}
-
-void WebContents::OnRendererMessageSync(content::RenderFrameHost* frame_host,
-                                        bool internal,
-                                        const std::string& channel,
-                                        const base::ListValue& args,
-                                        IPC::Message* message) {
-  // webContents.emit('-ipc-message-sync', new Event(sender, message), internal,
-  // channel, args);
-  EmitWithSender("-ipc-message-sync", frame_host, message, internal, channel,
-                 args);
-}
-
 void WebContents::OnRendererMessageTo(content::RenderFrameHost* frame_host,
                                       bool internal,
                                       bool send_to_all,
@@ -2233,7 +2277,7 @@ void WebContents::OnRendererMessageHost(content::RenderFrameHost* frame_host,
                                         const std::string& channel,
                                         const base::ListValue& args) {
   // webContents.emit('ipc-message-host', new Event(), channel, args);
-  EmitWithSender("ipc-message-host", frame_host, nullptr, channel, args);
+  EmitWithSender("ipc-message-host", frame_host, base::nullopt, channel, args);
 }
 
 // static