Merge pull request #14488 from electron/manage-webview-webcontents-3-0-x

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

atom/browser/api/atom_api_web_contents.cc

@@ -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.

lib/browser/guest-view-manager.js

@@ -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)
 })

lib/renderer/web-view/guest-view-internal.js

@@ -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.

lib/renderer/web-view/web-view.js

@@ -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
     }
 

vendor/libchromiumcontent

@@ -1 +1 @@
-Subproject commit 61d71f3f150c3ff5025560dee254a53313bfbaf6
+Subproject commit 0309588604aedd159793b611ae14a9015e4f65d0