From e26f366405b2e441b31d176da55c5af7481c64ca Mon Sep 17 00:00:00 2001 From: Alexandre Lacheze Date: Fri, 12 Jul 2019 00:25:27 +0200 Subject: [PATCH] Revert: electron/electron#14487 (#19011) --- lib/browser/guest-view-manager.js | 22 -------------- lib/renderer/web-view/guest-view-internal.ts | 5 ---- lib/renderer/web-view/web-view-impl.ts | 1 - patches/chromium/.patches | 1 - .../disable_detach_webview_frame.patch | 30 ------------------- shell/browser/api/atom_api_web_contents.cc | 11 ++----- 6 files changed, 3 insertions(+), 67 deletions(-) delete mode 100644 patches/chromium/disable_detach_webview_frame.patch diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index 26d8eda2bd3..03462d4e567 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -66,19 +66,6 @@ const createGuest = function (embedder, params) { } // Clear the guest from map when it is destroyed. - // - // The guest WebContents is usually destroyed in 2 cases: - // 1. The embedder frame is closed (reloaded or destroyed), and it - // automatically closes the guest frame. - // 2. The guest frame is detached dynamically via JS, and it is manually - // destroyed when the renderer sends the GUEST_VIEW_MANAGER_DESTROY_GUEST - // message. - // The second case relies on the libcc patch: - // https://github.com/electron/libchromiumcontent/pull/676 - // The patch was introduced to work around a bug in Chromium: - // https://github.com/electron/electron/issues/14211 - // We should revisit the bug to see if we can remove our libcc patch, the - // patch was introduced in Chrome 66. guest.once('destroyed', () => { if (guestInstanceId in guestInstances) { detachGuest(embedder, guestInstanceId) @@ -340,15 +327,6 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, param return createGuest(event.sender, params) }) -handleMessage('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', function (event, guestInstanceId) { - try { - const guest = getGuestForWebContents(guestInstanceId, event.sender) - guest.detachFromOuterFrame() - } catch (error) { - console.error(`Guest destroy failed: ${error}`) - } -}) - handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) { try { attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params) diff --git a/lib/renderer/web-view/guest-view-internal.ts b/lib/renderer/web-view/guest-view-internal.ts index 583049eb221..551e6f8539e 100644 --- a/lib/renderer/web-view/guest-view-internal.ts +++ b/lib/renderer/web-view/guest-view-internal.ts @@ -100,10 +100,6 @@ export function createGuestSync (params: Record): number { return invokeSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) } -export function destroyGuest (guestInstanceId: number): void { - invoke('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId) -} - export function attachGuest ( elementInstanceId: number, guestInstanceId: number, params: Record, contentWindow: Window ) { @@ -118,6 +114,5 @@ export const guestViewInternalModule = { deregisterEvents, createGuest, createGuestSync, - destroyGuest, attachGuest } diff --git a/lib/renderer/web-view/web-view-impl.ts b/lib/renderer/web-view/web-view-impl.ts index ab59234e0e9..014a66c3177 100644 --- a/lib/renderer/web-view/web-view-impl.ts +++ b/lib/renderer/web-view/web-view-impl.ts @@ -69,7 +69,6 @@ export class WebViewImpl { // heard back from createGuest yet. We will not reset the flag in this case so // that we don't end up allocating a second guest. if (this.guestInstanceId) { - guestViewInternal.destroyGuest(this.guestInstanceId) this.guestInstanceId = void 0 } diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 352b8bf3ef9..2f92fa2dd20 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -41,7 +41,6 @@ enable_widevine.patch chrome_key_systems.patch allow_nested_error_trackers.patch blink_initialization_order.patch -disable_detach_webview_frame.patch ssl_security_state_tab_helper.patch exclude-a-few-test-files-from-build.patch expose-net-observer-api.patch diff --git a/patches/chromium/disable_detach_webview_frame.patch b/patches/chromium/disable_detach_webview_frame.patch deleted file mode 100644 index e62eff83075..00000000000 --- a/patches/chromium/disable_detach_webview_frame.patch +++ /dev/null @@ -1,30 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: zcbenz -Date: Thu, 20 Sep 2018 17:50:27 -0700 -Subject: disable_detach_webview_frame.patch - -Don't detach the frame for webview, we will manage the WebContents -manually. -This is part of the fixes for https://github.com/electron/electron/issues/14211. -We should revisit this bug after upgrading to newer versions of Chrome, -this patch was introduced in Chrome 66. - -Update(zcbenz): The bug is still in Chrome 72. - -diff --git a/content/browser/frame_host/render_frame_proxy_host.cc b/content/browser/frame_host/render_frame_proxy_host.cc -index ab3be3430e2d074bbb8b4c2d5fe5a5f98baf6848..acb78da664aae9320e65ffbe3a273025723652d6 100644 ---- a/content/browser/frame_host/render_frame_proxy_host.cc -+++ b/content/browser/frame_host/render_frame_proxy_host.cc -@@ -268,6 +268,12 @@ void RenderFrameProxyHost::BubbleLogicalScroll( - - void RenderFrameProxyHost::OnDetach() { - if (frame_tree_node_->render_manager()->IsMainFrameForInnerDelegate()) { -+ // Don't detach the frame for webview, we will manage the WebContents -+ // manually. -+ // We should revisit this bug after upgrading to newer versions of Chrome, -+ // this patch was introduced in Chrome 66. -+ return; -+ - frame_tree_node_->render_manager()->RemoveOuterDelegateFrame(); - return; - } diff --git a/shell/browser/api/atom_api_web_contents.cc b/shell/browser/api/atom_api_web_contents.cc index 85a948a499f..47412e5788f 100644 --- a/shell/browser/api/atom_api_web_contents.cc +++ b/shell/browser/api/atom_api_web_contents.cc @@ -495,12 +495,7 @@ WebContents::~WebContents() { RenderViewDeleted(web_contents()->GetRenderViewHost()); - if (type_ == Type::WEB_VIEW) { - DCHECK(!web_contents()->GetOuterWebContents()) - << "Should never manually destroy an attached webview"; - // For webview simply destroy the WebContents immediately. - DestroyWebContents(false /* async */); - } else if (type_ == Type::BROWSER_WINDOW && owner_window()) { + if (type_ == Type::BROWSER_WINDOW && owner_window()) { // For BrowserWindow we should close the window and clean up everything // before WebContents is destroyed. for (ExtendedWebContentsObserver& observer : observers_) @@ -514,7 +509,7 @@ WebContents::~WebContents() { } else { // Destroy WebContents asynchronously unless app is shutting down, // because destroy() might be called inside WebContents's event handler. - DestroyWebContents(true /* async */); + DestroyWebContents(!IsGuest() /* async */); // The WebContentsDestroyed will not be called automatically because we // destroy the webContents in the next tick. So we have to manually // call it here to make sure "destroyed" event is emitted. @@ -1190,7 +1185,7 @@ bool WebContents::OnMessageReceived(const IPC::Message& message) { // 2. garbage collection; // 3. user closes the window of webContents; // 4. the embedder detaches the frame. -// For webview both #1 and #4 may happen, for BrowserWindow both #1 and #3 may +// For webview only #4 will happen, for BrowserWindow both #1 and #3 may // happen. The #2 should never happen for webContents, because webview is // managed by GuestViewManager, and BrowserWindow's webContents is managed // by api::BrowserWindow.