From b6321cc22d98ad3460f4fbdc00a640373ff5fae8 Mon Sep 17 00:00:00 2001 From: Robo Date: Tue, 28 Jul 2020 13:00:44 -0700 Subject: [PATCH] fix: crash when navigating from a page with webview that has inherited zoom level (#24757) * fix: cleanup webview zoom level observers on navigation * add spec * webview should be on same partition * wait for webview to finish loading --- .../browser/api/electron_api_web_contents.cc | 5 +++++ shell/browser/web_view_guest_delegate.cc | 4 ++++ shell/browser/web_view_guest_delegate.h | 1 + spec-main/webview-spec.ts | 19 +++++++++++++++++++ .../pages/webview-zoom-inherited.html | 12 ++++++++++++ 5 files changed, 41 insertions(+) create mode 100644 spec/fixtures/pages/webview-zoom-inherited.html diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index b7825337aa89..25fa409e6f0e 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1413,6 +1413,11 @@ bool WebContents::OnMessageReceived(const IPC::Message& message) { // we need to make sure the api::WebContents is also deleted. // For #4, the WebContents will be destroyed by embedder. void WebContents::WebContentsDestroyed() { + // Give chance for guest delegate to cleanup its observers + // since the native class is only destroyed in the next tick. + if (guest_delegate_) + guest_delegate_->WillDestroy(); + // Cleanup relationships with other parts. RemoveFromWeakMap(); diff --git a/shell/browser/web_view_guest_delegate.cc b/shell/browser/web_view_guest_delegate.cc index aa39563657c1..dadb04d84f26 100644 --- a/shell/browser/web_view_guest_delegate.cc +++ b/shell/browser/web_view_guest_delegate.cc @@ -57,6 +57,10 @@ void WebViewGuestDelegate::AttachToIframe( api_web_contents_->Emit("did-attach"); } +void WebViewGuestDelegate::WillDestroy() { + ResetZoomController(); +} + void WebViewGuestDelegate::DidDetach() { ResetZoomController(); } diff --git a/shell/browser/web_view_guest_delegate.h b/shell/browser/web_view_guest_delegate.h index ccf4b53c0636..421f78722fab 100644 --- a/shell/browser/web_view_guest_delegate.h +++ b/shell/browser/web_view_guest_delegate.h @@ -24,6 +24,7 @@ class WebViewGuestDelegate : public content::BrowserPluginGuestDelegate, // Attach to the iframe. void AttachToIframe(content::WebContents* embedder_web_contents, int embedder_frame_id); + void WillDestroy(); protected: // content::BrowserPluginGuestDelegate: diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 91f29969bf12..fe3eba8109f3 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -315,6 +315,25 @@ describe(' tag', function () { const [, zoomLevel] = await emittedOnce(ipcMain, 'webview-origin-zoom-level'); expect(zoomLevel).to.equal(2.0); }); + + it('does not crash when navigating with zoom level inherited from parent', async () => { + const w = new BrowserWindow({ + show: false, + webPreferences: { + webviewTag: true, + nodeIntegration: true, + zoomFactor: 1.2, + session: webviewSession + } + }); + const attachPromise = emittedOnce(w.webContents, 'did-attach-webview'); + const readyPromise = emittedOnce(ipcMain, 'dom-ready'); + w.loadFile(path.join(fixtures, 'pages', 'webview-zoom-inherited.html')); + const [, webview] = await attachPromise; + await readyPromise; + expect(webview.getZoomFactor()).to.equal(1.2); + await w.loadURL(`${zoomScheme}://host1`); + }); }); describe('nativeWindowOpen option', () => { diff --git a/spec/fixtures/pages/webview-zoom-inherited.html b/spec/fixtures/pages/webview-zoom-inherited.html new file mode 100644 index 000000000000..0bff665231d0 --- /dev/null +++ b/spec/fixtures/pages/webview-zoom-inherited.html @@ -0,0 +1,12 @@ + + + + + +