Skip to content

Commit

Permalink
Revert: #14487 (#19011)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexstrat authored and John Kleinschmidt committed Jul 11, 2019
1 parent 75a020e commit e26f366
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 67 deletions.
22 changes: 0 additions & 22 deletions lib/browser/guest-view-manager.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -66,19 +66,6 @@ const createGuest = function (embedder, params) {
} }


// Clear the guest from map when it is destroyed. // 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', () => { guest.once('destroyed', () => {
if (guestInstanceId in guestInstances) { if (guestInstanceId in guestInstances) {
detachGuest(embedder, guestInstanceId) detachGuest(embedder, guestInstanceId)
Expand Down Expand Up @@ -340,15 +327,6 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, param
return createGuest(event.sender, params) 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) { handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) {
try { try {
attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params) attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params)
Expand Down
5 changes: 0 additions & 5 deletions lib/renderer/web-view/guest-view-internal.ts
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ export function createGuestSync (params: Record<string, any>): number {
return invokeSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) 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 ( export function attachGuest (
elementInstanceId: number, guestInstanceId: number, params: Record<string, any>, contentWindow: Window elementInstanceId: number, guestInstanceId: number, params: Record<string, any>, contentWindow: Window
) { ) {
Expand All @@ -118,6 +114,5 @@ export const guestViewInternalModule = {
deregisterEvents, deregisterEvents,
createGuest, createGuest,
createGuestSync, createGuestSync,
destroyGuest,
attachGuest attachGuest
} }
1 change: 0 additions & 1 deletion lib/renderer/web-view/web-view-impl.ts
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export class WebViewImpl {
// heard back from createGuest yet. We will not reset the flag in this case so // 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. // that we don't end up allocating a second guest.
if (this.guestInstanceId) { if (this.guestInstanceId) {
guestViewInternal.destroyGuest(this.guestInstanceId)
this.guestInstanceId = void 0 this.guestInstanceId = void 0
} }


Expand Down
1 change: 0 additions & 1 deletion patches/chromium/.patches
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ enable_widevine.patch
chrome_key_systems.patch chrome_key_systems.patch
allow_nested_error_trackers.patch allow_nested_error_trackers.patch
blink_initialization_order.patch blink_initialization_order.patch
disable_detach_webview_frame.patch
ssl_security_state_tab_helper.patch ssl_security_state_tab_helper.patch
exclude-a-few-test-files-from-build.patch exclude-a-few-test-files-from-build.patch
expose-net-observer-api.patch expose-net-observer-api.patch
Expand Down
30 changes: 0 additions & 30 deletions patches/chromium/disable_detach_webview_frame.patch

This file was deleted.

11 changes: 3 additions & 8 deletions shell/browser/api/atom_api_web_contents.cc
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -495,12 +495,7 @@ WebContents::~WebContents() {


RenderViewDeleted(web_contents()->GetRenderViewHost()); RenderViewDeleted(web_contents()->GetRenderViewHost());


if (type_ == Type::WEB_VIEW) { if (type_ == Type::BROWSER_WINDOW && owner_window()) {
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()) {
// For BrowserWindow we should close the window and clean up everything // For BrowserWindow we should close the window and clean up everything
// before WebContents is destroyed. // before WebContents is destroyed.
for (ExtendedWebContentsObserver& observer : observers_) for (ExtendedWebContentsObserver& observer : observers_)
Expand All @@ -514,7 +509,7 @@ WebContents::~WebContents() {
} else { } else {
// Destroy WebContents asynchronously unless app is shutting down, // Destroy WebContents asynchronously unless app is shutting down,
// because destroy() might be called inside WebContents's event handler. // 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 // The WebContentsDestroyed will not be called automatically because we
// destroy the webContents in the next tick. So we have to manually // destroy the webContents in the next tick. So we have to manually
// call it here to make sure "destroyed" event is emitted. // call it here to make sure "destroyed" event is emitted.
Expand Down Expand Up @@ -1190,7 +1185,7 @@ bool WebContents::OnMessageReceived(const IPC::Message& message) {
// 2. garbage collection; // 2. garbage collection;
// 3. user closes the window of webContents; // 3. user closes the window of webContents;
// 4. the embedder detaches the frame. // 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 // happen. The #2 should never happen for webContents, because webview is
// managed by GuestViewManager, and BrowserWindow's webContents is managed // managed by GuestViewManager, and BrowserWindow's webContents is managed
// by api::BrowserWindow. // by api::BrowserWindow.
Expand Down

0 comments on commit e26f366

Please sign in to comment.