Browse Source

fix: avoid IPC for renderer `webFrame.getZoom...` APIs (#45499)

* fix: avoid IPC for renderer `webFrame.getZoom...` APIs

* Remove `DoGetZoomLevel` IPC

* Fix synchronous behavior & nullptr deref

* Use local root
Calvin 2 months ago
parent
commit
f5025b6246

+ 0 - 6
shell/browser/api/electron_api_web_contents.cc

@@ -3693,12 +3693,6 @@ void WebContents::SetTemporaryZoomLevel(double level) {
   zoom_controller_->SetTemporaryZoomLevel(level);
 }
 
-void WebContents::DoGetZoomLevel(
-    electron::mojom::ElectronWebContentsUtility::DoGetZoomLevelCallback
-        callback) {
-  std::move(callback).Run(GetZoomLevel());
-}
-
 std::optional<PreloadScript> WebContents::GetPreloadScript() const {
   if (auto* web_preferences = WebContentsPreferences::From(web_contents())) {
     if (auto preload = web_preferences->GetPreloadPath()) {

+ 0 - 3
shell/browser/api/electron_api_web_contents.h

@@ -470,9 +470,6 @@ class WebContents final : public ExclusiveAccessContext,
   // mojom::ElectronWebContentsUtility
   void OnFirstNonEmptyLayout(content::RenderFrameHost* render_frame_host);
   void SetTemporaryZoomLevel(double level);
-  void DoGetZoomLevel(
-      electron::mojom::ElectronWebContentsUtility::DoGetZoomLevelCallback
-          callback);
 
   void SetImageAnimationPolicy(const std::string& new_policy);
 

+ 0 - 8
shell/browser/electron_web_contents_utility_handler_impl.cc

@@ -58,14 +58,6 @@ void ElectronWebContentsUtilityHandlerImpl::SetTemporaryZoomLevel(
   }
 }
 
-void ElectronWebContentsUtilityHandlerImpl::DoGetZoomLevel(
-    DoGetZoomLevelCallback callback) {
-  api::WebContents* api_web_contents = api::WebContents::From(web_contents());
-  if (api_web_contents) {
-    api_web_contents->DoGetZoomLevel(std::move(callback));
-  }
-}
-
 void ElectronWebContentsUtilityHandlerImpl::CanAccessClipboardDeprecated(
     mojom::PermissionName name,
     const blink::LocalFrameToken& frame_token,

+ 0 - 1
shell/browser/electron_web_contents_utility_handler_impl.h

@@ -42,7 +42,6 @@ class ElectronWebContentsUtilityHandlerImpl
   // mojom::ElectronWebContentsUtility:
   void OnFirstNonEmptyLayout() override;
   void SetTemporaryZoomLevel(double level) override;
-  void DoGetZoomLevel(DoGetZoomLevelCallback callback) override;
   void CanAccessClipboardDeprecated(
       mojom::PermissionName name,
       const blink::LocalFrameToken& frame_token,

+ 0 - 3
shell/common/web_contents_utility.mojom

@@ -15,9 +15,6 @@ interface ElectronWebContentsUtility {
 
   SetTemporaryZoomLevel(double zoom_level);
 
-  [Sync]
-  DoGetZoomLevel() => (double result);
-
   [Sync]
   CanAccessClipboardDeprecated(
       PermissionName name,

+ 18 - 9
shell/renderer/api/electron_api_web_frame.cc

@@ -441,25 +441,34 @@ class WebFrameRenderer final : public gin::Wrappable<WebFrameRenderer>,
     if (!MaybeGetRenderFrame(isolate, "setZoomLevel", &render_frame))
       return;
 
+    // Update the zoom controller.
     mojo::AssociatedRemote<mojom::ElectronWebContentsUtility>
         web_contents_utility_remote;
     render_frame->GetRemoteAssociatedInterfaces()->GetInterface(
         &web_contents_utility_remote);
     web_contents_utility_remote->SetTemporaryZoomLevel(level);
+
+    // Update the local web frame for coherence with synchronous calls to
+    // |GetZoomLevel|.
+    if (blink::WebFrameWidget* web_frame =
+            render_frame->GetWebFrame()->LocalRoot()->FrameWidget()) {
+      web_frame->SetZoomLevel(level);
+    }
   }
 
   double GetZoomLevel(v8::Isolate* isolate) {
-    double result = 0.0;
     content::RenderFrame* render_frame;
-    if (!MaybeGetRenderFrame(isolate, "getZoomLevel", &render_frame))
-      return result;
+    if (!MaybeGetRenderFrame(isolate, "getZoomLevel", &render_frame)) {
+      return 0.0f;
+    }
 
-    mojo::AssociatedRemote<mojom::ElectronWebContentsUtility>
-        web_contents_utility_remote;
-    render_frame->GetRemoteAssociatedInterfaces()->GetInterface(
-        &web_contents_utility_remote);
-    web_contents_utility_remote->DoGetZoomLevel(&result);
-    return result;
+    blink::WebFrameWidget* web_frame =
+        render_frame->GetWebFrame()->LocalRoot()->FrameWidget();
+    if (!web_frame) {
+      return 0.0f;
+    }
+
+    return web_frame->GetZoomLevel();
   }
 
   void SetZoomFactor(gin_helper::ErrorThrower thrower, double factor) {

+ 4 - 4
spec/api-web-frame-spec.ts

@@ -176,15 +176,15 @@ describe('webFrame module', () => {
 
     describe('setZoomFactor()', () => {
       it('works', async () => {
-        const equal = await w.executeJavaScript('childFrame.setZoomFactor(2.0); childFrame.getZoomFactor() === 2.0');
-        expect(equal).to.be.true();
+        const zoom = await w.executeJavaScript('childFrame.setZoomFactor(2.0); childFrame.getZoomFactor()');
+        expect(zoom).to.equal(2.0);
       });
     });
 
     describe('setZoomLevel()', () => {
       it('works', async () => {
-        const equal = await w.executeJavaScript('childFrame.setZoomLevel(5); childFrame.getZoomLevel() === 5');
-        expect(equal).to.be.true();
+        const zoom = await w.executeJavaScript('childFrame.setZoomLevel(5); childFrame.getZoomLevel()');
+        expect(zoom).to.equal(5);
       });
     });