From d8e5633d0c07136f59d46b62475bf16bc9bfcc93 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 27 Jul 2020 22:09:24 -0700 Subject: [PATCH 1/4] fix: cleanup webview zoom level observers on navigation --- shell/browser/api/electron_api_web_contents.cc | 5 +++++ shell/browser/web_view_guest_delegate.cc | 4 ++++ shell/browser/web_view_guest_delegate.h | 1 + 3 files changed, 10 insertions(+) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 7aa92a65853ac..07c20b932fcd6 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 aa39563657c15..dadb04d84f263 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 ccf4b53c06365..421f78722fab8 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: From 548cafb8e03077a8df671c4e3777f27046f37c3c Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 27 Jul 2020 22:58:46 -0700 Subject: [PATCH 2/4] add spec --- spec-main/webview-spec.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 26e7849f87ee4..743192bbfc351 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -301,6 +301,23 @@ 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'); + w.loadFile(path.join(fixtures, 'pages', 'webview-did-attach-event.html')); + const [, webview] = await attachPromise; + expect(webview.getZoomFactor()).to.equal(1.2); + await w.loadURL(`${zoomScheme}://host1`); + }); }); describe('nativeWindowOpen option', () => { From aaff618b5c60424949b0b84fb5a415e7ae32840b Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 27 Jul 2020 23:56:38 -0700 Subject: [PATCH 3/4] webview should be on same partition --- spec-main/webview-spec.ts | 2 +- spec/fixtures/pages/webview-zoom-inherited.html | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/pages/webview-zoom-inherited.html diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 743192bbfc351..13b1898a4630f 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -313,7 +313,7 @@ describe(' tag', function () { } }); const attachPromise = emittedOnce(w.webContents, 'did-attach-webview'); - w.loadFile(path.join(fixtures, 'pages', 'webview-did-attach-event.html')); + w.loadFile(path.join(fixtures, 'pages', 'webview-zoom-inherited.html')); const [, webview] = await attachPromise; expect(webview.getZoomFactor()).to.equal(1.2); await w.loadURL(`${zoomScheme}://host1`); diff --git a/spec/fixtures/pages/webview-zoom-inherited.html b/spec/fixtures/pages/webview-zoom-inherited.html new file mode 100644 index 0000000000000..6872e7aa7b48c --- /dev/null +++ b/spec/fixtures/pages/webview-zoom-inherited.html @@ -0,0 +1,5 @@ + + + + + From 79e216c50fc80104de6b178f8685f50c43c4460c Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 28 Jul 2020 10:44:53 -0700 Subject: [PATCH 4/4] wait for webview to finish loading --- spec-main/webview-spec.ts | 2 ++ spec/fixtures/pages/webview-zoom-inherited.html | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 13b1898a4630f..6521841d3e3c7 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -313,8 +313,10 @@ describe(' tag', function () { } }); 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`); }); diff --git a/spec/fixtures/pages/webview-zoom-inherited.html b/spec/fixtures/pages/webview-zoom-inherited.html index 6872e7aa7b48c..0bff665231d05 100644 --- a/spec/fixtures/pages/webview-zoom-inherited.html +++ b/spec/fixtures/pages/webview-zoom-inherited.html @@ -2,4 +2,11 @@ +