New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: manually manage WebContents of webview when it is detached (3-0-x) #14488

Merged
merged 1 commit into from Sep 7, 2018
Jump to file or symbol
Failed to load files and symbols.
+38 −10
Diff settings

Always

Just for now

@@ -483,16 +483,20 @@ WebContents::~WebContents() {
RenderViewDeleted(web_contents()->GetRenderViewHost());
if (type_ == BROWSER_WINDOW && owner_window()) {
for (ExtendedWebContentsObserver& observer : observers_)
observer.OnCloseContents();
if (type_ == WEB_VIEW) {
DestroyWebContents(false /* async */);
} else {
DestroyWebContents(!IsGuest() /* async */);
if (type_ == BROWSER_WINDOW && owner_window()) {
for (ExtendedWebContentsObserver& observer : observers_)
observer.OnCloseContents();
} else {
DestroyWebContents(true /* 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.
WebContentsDestroyed();
}
// 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.
WebContentsDestroyed();
}
}
@@ -1040,7 +1044,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 only #4 will happen, for BrowserWindow both #1 and #3 may
// For webview both #1 and #4 may 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.
@@ -70,6 +70,19 @@ 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)
@@ -319,6 +332,13 @@ ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event, par
event.returnValue = createGuest(event.sender, params)
})
ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', function (event, guestInstanceId) {
const guest = getGuest(guestInstanceId)
if (guest) {
guest.destroy()
}
})
ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) {
attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params)
})
@@ -94,6 +94,9 @@ module.exports = {
createGuestSync: function (params) {
return ipcRenderer.sendSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', params)
},
destroyGuest: function (guestInstanceId) {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId)
},
attachGuest: function (elementInstanceId, guestInstanceId, params, contentWindow) {
const embedderFrameId = webFrame.getWebFrameId(contentWindow)
if (embedderFrameId < 0) { // this error should not happen.
@@ -76,6 +76,7 @@ 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
}
ProTip! Use n and p to navigate between commits in a pull request.