From d3d44bdbc6643e1a0c5ec5b03ee319fb3c5bd162 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Fri, 7 Sep 2018 15:41:48 +0900
Subject: [PATCH] fix: manually manage WebContents of webview when it is
 detached

---
 atom/browser/api/atom_api_web_contents.cc    | 22 ++++++++++++--------
 lib/browser/guest-view-manager.js            | 20 ++++++++++++++++++
 lib/renderer/web-view/guest-view-internal.js |  3 +++
 lib/renderer/web-view/web-view.js            |  1 +
 vendor/libchromiumcontent                    |  2 +-
 5 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
index 9690b5959e29..41aeee9fd600 100644
--- a/atom/browser/api/atom_api_web_contents.cc
+++ b/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.
diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js
index 1426dc61055b..c44d033a85f9 100644
--- a/lib/browser/guest-view-manager.js
+++ b/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)
 })
diff --git a/lib/renderer/web-view/guest-view-internal.js b/lib/renderer/web-view/guest-view-internal.js
index 183c85af8504..3abef43dffda 100644
--- a/lib/renderer/web-view/guest-view-internal.js
+++ b/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.
diff --git a/lib/renderer/web-view/web-view.js b/lib/renderer/web-view/web-view.js
index 66199c6be831..bd2b1f09676d 100644
--- a/lib/renderer/web-view/web-view.js
+++ b/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
     }
 
diff --git a/vendor/libchromiumcontent b/vendor/libchromiumcontent
index 61d71f3f150c..0309588604ae 160000
--- a/vendor/libchromiumcontent
+++ b/vendor/libchromiumcontent
@@ -1 +1 @@
-Subproject commit 61d71f3f150c3ff5025560dee254a53313bfbaf6
+Subproject commit 0309588604aedd159793b611ae14a9015e4f65d0