refactor: replace webFrame.routingId with sync IPC (#47941)

* refactor: replace webFrame.routingId with sync IPC

Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

* fix: GetConstructor missing isolate

Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

* fix: missing isolate

Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

docs/api/web-frame.md

@@ -253,7 +253,7 @@ and intend to stay there).
 
 * `selector` string - CSS selector for a frame element.
 
-Returns `WebFrame` - The frame element in `webFrame's` document selected by
+Returns `WebFrame | null` - The frame element in `webFrame's` document selected by
 `selector`, `null` would be returned if `selector` does not select a frame or
 if the frame is not in the current renderer process.
 
@@ -261,7 +261,7 @@ if the frame is not in the current renderer process.
 
 * `name` string
 
-Returns `WebFrame` - A child of `webFrame` with the supplied `name`, `null`
+Returns `WebFrame | null` - A child of `webFrame` with the supplied `name`, `null`
 would be returned if there's no such frame or if the frame is not in the current
 renderer process.
 
@@ -272,7 +272,9 @@ renderer process.
    instances (`webFrame.routingId`) and are also passed by frame
    specific `WebContents` navigation events (e.g. `did-frame-navigate`)
 
-Returns `WebFrame` - that has the supplied `routingId`, `null` if not found.
+Returns `WebFrame | null` - that has the supplied `routingId`, `null` if not found.
+
+**Deprecated:** Use the new `webFrame.findFrameByToken` API.
 
 ### `webFrame.findFrameByToken(frameToken)`
 
@@ -281,7 +283,7 @@ Returns `WebFrame` - that has the supplied `routingId`, `null` if not found.
    instances (`webFrame.frameToken`) and can also be retrieved from
    `WebFrameMain` instances using `webFrameMain.frameToken`.
 
-Returns `WebFrame` - that has the supplied `frameToken`, `null` if not found.
+Returns `WebFrame | null` - that has the supplied `frameToken`, `null` if not found.
 
 ### `webFrame.isWordMisspelled(word)`
 
@@ -333,6 +335,8 @@ An `Integer` representing the unique frame id in the current renderer process.
 Distinct WebFrame instances that refer to the same underlying frame will have
 the same `routingId`.
 
+**Deprecated:** Use the new `webFrame.frameToken` API.
+
 ### `webFrame.frameToken` _Readonly_
 
 A `string` representing the unique frame token in the current renderer process.

filenames.auto.gni

@@ -167,6 +167,7 @@ auto_filenames = {
   sandbox_bundle_deps = [
     "lib/common/api/native-image.ts",
     "lib/common/define-properties.ts",
+    "lib/common/deprecate.ts",
     "lib/common/ipc-messages.ts",
     "lib/common/web-view-methods.ts",
     "lib/common/webpack-globals-provider.ts",
@@ -298,6 +299,7 @@ auto_filenames = {
     "lib/common/api/native-image.ts",
     "lib/common/api/shell.ts",
     "lib/common/define-properties.ts",
+    "lib/common/deprecate.ts",
     "lib/common/init.ts",
     "lib/common/ipc-messages.ts",
     "lib/common/web-view-methods.ts",
@@ -338,6 +340,7 @@ auto_filenames = {
     "lib/common/api/native-image.ts",
     "lib/common/api/shell.ts",
     "lib/common/define-properties.ts",
+    "lib/common/deprecate.ts",
     "lib/common/init.ts",
     "lib/common/ipc-messages.ts",
     "lib/common/webpack-provider.ts",

lib/browser/rpc-server.ts

@@ -3,6 +3,7 @@ import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-util
 import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages';
 
 import { clipboard } from 'electron/common';
+import { webFrameMain } from 'electron/main';
 
 import * as fs from 'fs';
 import * as path from 'path';
@@ -109,3 +110,17 @@ ipcMainInternal.on(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, function (event, preloadP
   if (event.type !== 'frame') return;
   event.sender?.emit('preload-error', event, preloadPath, error);
 });
+
+ipcMainUtils.handleSync(IPC_MESSAGES.BROWSER_GET_FRAME_ROUTING_ID_SYNC, function (event, frameToken: string) {
+  if (event.type !== 'frame') return;
+  const senderFrame = event.senderFrame;
+  if (!senderFrame || senderFrame.isDestroyed()) return;
+  return webFrameMain.fromFrameToken(senderFrame.processId, frameToken)?.routingId;
+});
+
+ipcMainUtils.handleSync(IPC_MESSAGES.BROWSER_GET_FRAME_TOKEN_SYNC, function (event, routingId: number) {
+  if (event.type !== 'frame') return;
+  const senderFrame = event.senderFrame;
+  if (!senderFrame || senderFrame.isDestroyed()) return;
+  return webFrameMain.fromId(senderFrame.processId, routingId)?.frameToken;
+});

lib/common/ipc-messages.ts

@@ -6,6 +6,8 @@ export const enum IPC_MESSAGES {
   BROWSER_NONSANDBOX_LOAD = 'BROWSER_NONSANDBOX_LOAD',
   BROWSER_WINDOW_CLOSE = 'BROWSER_WINDOW_CLOSE',
   BROWSER_GET_PROCESS_MEMORY_INFO = 'BROWSER_GET_PROCESS_MEMORY_INFO',
+  BROWSER_GET_FRAME_ROUTING_ID_SYNC = 'BROWSER_GET_FRAME_ROUTING_ID_SYNC',
+  BROWSER_GET_FRAME_TOKEN_SYNC = 'BROWSER_GET_FRAME_TOKEN_SYNC',
 
   GUEST_INSTANCE_VISIBILITY_CHANGE = 'GUEST_INSTANCE_VISIBILITY_CHANGE',
 

lib/renderer/api/web-frame.ts

@@ -1,3 +1,36 @@
-const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
+import * as deprecate from '@electron/internal/common/deprecate';
+import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages';
+import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils';
+
+const { mainFrame, WebFrame } = process._linkedBinding('electron_renderer_web_frame');
+
+// @ts-expect-error - WebFrame types are cursed. It's an instanced class, but
+// the docs define it as a static module.
+// TODO(smaddock): Fix web-frame.md to define it as an instance class.
+const WebFramePrototype: Electron.WebFrame = WebFrame.prototype;
+
+const routingIdDeprecated = deprecate.warnOnce('webFrame.routingId', 'webFrame.frameToken');
+Object.defineProperty(WebFramePrototype, 'routingId', {
+  configurable: true,
+  get: function (this: Electron.WebFrame) {
+    routingIdDeprecated();
+    return ipcRendererUtils.invokeSync<number>(
+      IPC_MESSAGES.BROWSER_GET_FRAME_ROUTING_ID_SYNC,
+      this.frameToken
+    );
+  }
+});
+
+const findFrameByRoutingIdDeprecated = deprecate.warnOnce('webFrame.findFrameByRoutingId', 'webFrame.findFrameByToken');
+WebFramePrototype.findFrameByRoutingId = function (
+  routingId: number
+): Electron.WebFrame | null {
+  findFrameByRoutingIdDeprecated();
+  const frameToken = ipcRendererUtils.invokeSync<string | undefined>(
+    IPC_MESSAGES.BROWSER_GET_FRAME_TOKEN_SYNC,
+    routingId
+  );
+  return frameToken ? this.findFrameByToken(frameToken) : null;
+};
 
 export default mainFrame;

shell/renderer/api/electron_api_web_frame.cc

@@ -26,9 +26,11 @@
 #include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
+#include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/function_template_extensions.h"
+#include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/gin_helper/promise.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/node_util.h"
@@ -332,26 +334,13 @@ class SpellCheckerHolder final : private content::RenderFrameObserver {
 
 class WebFrameRenderer final
     : public gin::DeprecatedWrappable<WebFrameRenderer>,
+      public gin_helper::Constructible<WebFrameRenderer>,
       private content::RenderFrameObserver {
  public:
-  static gin::DeprecatedWrapperInfo kWrapperInfo;
-
-  static gin::Handle<WebFrameRenderer> Create(
-      v8::Isolate* isolate,
-      content::RenderFrame* render_frame) {
-    return gin::CreateHandle(isolate, new WebFrameRenderer(render_frame));
-  }
-
-  explicit WebFrameRenderer(content::RenderFrame* render_frame)
-      : content::RenderFrameObserver(render_frame) {
-    DCHECK(render_frame);
-  }
-
-  // gin::Wrappable:
-  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
-      v8::Isolate* isolate) override {
-    return gin::DeprecatedWrappable<WebFrameRenderer>::GetObjectTemplateBuilder(
-               isolate)
+  // gin_helper::Constructible
+  static void FillObjectTemplate(v8::Isolate* isolate,
+                                 v8::Local<v8::ObjectTemplate> templ) {
+    gin_helper::ObjectTemplateBuilder(isolate, templ)
         .SetMethod("setName", &WebFrameRenderer::SetName)
         .SetMethod("setZoomLevel", &WebFrameRenderer::SetZoomLevel)
         .SetMethod("getZoomLevel", &WebFrameRenderer::GetZoomLevel)
@@ -380,8 +369,6 @@ class WebFrameRenderer final
         .SetMethod("setSpellCheckProvider",
                    &WebFrameRenderer::SetSpellCheckProvider)
         // Frame navigators
-        .SetMethod("findFrameByRoutingId",
-                   &WebFrameRenderer::FindFrameByRoutingId)
         .SetMethod("findFrameByToken", &WebFrameRenderer::FindFrameByToken)
         .SetMethod("getFrameForSelector",
                    &WebFrameRenderer::GetFrameForSelector)
@@ -393,10 +380,25 @@ class WebFrameRenderer final
         .SetProperty("top", &WebFrameRenderer::GetTop)
         .SetProperty("firstChild", &WebFrameRenderer::GetFirstChild)
         .SetProperty("nextSibling", &WebFrameRenderer::GetNextSibling)
-        .SetProperty("routingId", &WebFrameRenderer::GetRoutingId);
+        .Build();
+  }
+  static const char* GetClassName() { return "WebFrame"; }
+  static gin::Handle<WebFrameRenderer> New(v8::Isolate* isolate) { return {}; }
+
+  static gin::DeprecatedWrapperInfo kWrapperInfo;
+
+  static gin::Handle<WebFrameRenderer> Create(
+      v8::Isolate* isolate,
+      content::RenderFrame* render_frame) {
+    return gin::CreateHandle(isolate, new WebFrameRenderer(render_frame));
   }
 
-  const char* GetTypeName() override { return "WebFrameRenderer"; }
+  explicit WebFrameRenderer(content::RenderFrame* render_frame)
+      : content::RenderFrameObserver(render_frame) {
+    DCHECK(render_frame);
+  }
+
+  const char* GetTypeName() override { return GetClassName(); }
 
   void OnDestruct() override {}
 
@@ -806,14 +808,6 @@ class WebFrameRenderer final
         base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL);
   }
 
-  v8::Local<v8::Value> FindFrameByRoutingId(v8::Isolate* isolate,
-                                            int routing_id) {
-    util::EmitDeprecationWarning(isolate,
-                                 "findFrameByRoutingId() is deprecated, use "
-                                 "findFrameByToken() instead.");
-    return v8::Null(isolate);
-  }
-
   v8::Local<v8::Value> FindFrameByToken(v8::Isolate* isolate,
                                         std::string frame_token) {
     auto token = base::Token::FromString(frame_token);
@@ -938,12 +932,6 @@ class WebFrameRenderer final
         blink::WebString::FromUTF8(name));
     return CreateWebFrameRenderer(isolate, frame);
   }
-
-  int GetRoutingId(v8::Isolate* isolate) {
-    util::EmitDeprecationWarning(
-        isolate, "routingId is deprecated, use frameToken instead.");
-    return -1;
-  }
 };
 }  // namespace
 
@@ -966,7 +954,8 @@ void Initialize(v8::Local<v8::Object> exports,
   using namespace electron::api;  // NOLINT(build/namespaces)
 
   v8::Isolate* const isolate = v8::Isolate::GetCurrent();
-  gin_helper::Dictionary dict{isolate, exports};
+  gin_helper::Dictionary dict(isolate, exports);
+  dict.Set("WebFrame", WebFrameRenderer::GetConstructor(isolate, context));
   dict.Set("mainFrame",
            WebFrameRenderer::Create(
                isolate, electron::GetRenderFrame(isolate, exports)));