From aacbdaf4ecce6e9d6e6848019b2fc4a49061ea3f Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 13 Feb 2025 12:53:59 +0100 Subject: [PATCH] fix: avoid IPC for renderer `webFrame.getZoom...` APIs (#45557) * fix: avoid IPC for renderer `webFrame.getZoom...` APIs Co-authored-by: clavin * Remove `DoGetZoomLevel` IPC Co-authored-by: clavin * Fix synchronous behavior & nullptr deref Co-authored-by: clavin * Use local root Co-authored-by: clavin --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: clavin --- .../browser/api/electron_api_web_contents.cc | 6 ----- shell/browser/api/electron_api_web_contents.h | 3 --- ...ctron_web_contents_utility_handler_impl.cc | 8 ------ ...ectron_web_contents_utility_handler_impl.h | 1 - shell/common/web_contents_utility.mojom | 3 --- shell/renderer/api/electron_api_web_frame.cc | 27 ++++++++++++------- spec/api-web-frame-spec.ts | 8 +++--- 7 files changed, 22 insertions(+), 34 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 2b6a9fe5f7c5..fc415e75f5b4 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/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 WebContents::GetPreloadScript() const { if (auto* web_preferences = WebContentsPreferences::From(web_contents())) { if (auto preload = web_preferences->GetPreloadPath()) { diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 7509ac2ddbb7..27e0757bad0a 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/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); diff --git a/shell/browser/electron_web_contents_utility_handler_impl.cc b/shell/browser/electron_web_contents_utility_handler_impl.cc index e1baff6b4eae..c4a00c38be89 100644 --- a/shell/browser/electron_web_contents_utility_handler_impl.cc +++ b/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, diff --git a/shell/browser/electron_web_contents_utility_handler_impl.h b/shell/browser/electron_web_contents_utility_handler_impl.h index c4b53c4a5193..517c28eaee6e 100644 --- a/shell/browser/electron_web_contents_utility_handler_impl.h +++ b/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, diff --git a/shell/common/web_contents_utility.mojom b/shell/common/web_contents_utility.mojom index a941ee42a71d..89e5f1b17b76 100644 --- a/shell/common/web_contents_utility.mojom +++ b/shell/common/web_contents_utility.mojom @@ -15,9 +15,6 @@ interface ElectronWebContentsUtility { SetTemporaryZoomLevel(double zoom_level); - [Sync] - DoGetZoomLevel() => (double result); - [Sync] CanAccessClipboardDeprecated( PermissionName name, diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index 323f4467d3b1..6958ccde4f36 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -441,25 +441,34 @@ class WebFrameRenderer final : public gin::Wrappable, if (!MaybeGetRenderFrame(isolate, "setZoomLevel", &render_frame)) return; + // Update the zoom controller. mojo::AssociatedRemote 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 - 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) { diff --git a/spec/api-web-frame-spec.ts b/spec/api-web-frame-spec.ts index d1dcc52d41cb..f58316db25e7 100644 --- a/spec/api-web-frame-spec.ts +++ b/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); }); });