fix: detach webview instead of destroying it

Chromium no longer cleans up everything when a guest webcontents is
destroyed, we have to force detaching it and let Chromium destroy everything.

BUILD.gn

@@ -421,7 +421,6 @@ static_library("electron_lib") {
 
   if (enable_osr) {
     sources += [
-      "atom/browser/api/atom_api_web_contents_osr.cc",
       "atom/browser/osr/osr_output_device.cc",
       "atom/browser/osr/osr_output_device.h",
       "atom/browser/osr/osr_render_widget_host_view.cc",

atom/browser/api/atom_api_web_contents.cc

@@ -470,9 +470,9 @@ WebContents::~WebContents() {
     RenderViewDeleted(web_contents()->GetRenderViewHost());
 
     if (type_ == WEB_VIEW) {
+      DCHECK(!web_contents()->GetOuterWebContents())
+          << "Should never manually destroy an attached webview";
       // For webview simply destroy the WebContents immediately.
-      // TODO(zcbenz): Add an internal API for webview instead of using
-      // destroy(), so we don't have to add a special branch here.
       DestroyWebContents(false /* async */);
     } else if (type_ == BROWSER_WINDOW && owner_window()) {
       // For BrowserWindow we should close the window and clean up everything
@@ -2150,6 +2150,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("startDrag", &WebContents::StartDrag)
       .SetMethod("isGuest", &WebContents::IsGuest)
       .SetMethod("attachToIframe", &WebContents::AttachToIframe)
+      .SetMethod("detachFromOuterFrame", &WebContents::DetachFromOuterFrame)
       .SetMethod("isOffscreen", &WebContents::IsOffScreen)
 #if BUILDFLAG(ENABLE_OSR)
       .SetMethod("startPainting", &WebContents::StartPainting)

atom/browser/api/atom_api_web_contents.h

@@ -242,6 +242,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
   bool IsGuest() const;
   void AttachToIframe(content::WebContents* embedder_web_contents,
                       int embedder_frame_id);
+  void DetachFromOuterFrame();
 
   // Methods for offscreen rendering
   bool IsOffScreen() const;

atom/browser/api/atom_api_web_contents_impl.cc

@@ -4,9 +4,12 @@
 
 #include "atom/browser/api/atom_api_web_contents.h"
 
+#include "content/browser/web_contents/web_contents_impl.h"
+
+#if BUILDFLAG(ENABLE_OSR)
 #include "atom/browser/osr/osr_render_widget_host_view.h"
 #include "atom/browser/osr/osr_web_contents_view.h"
-#include "content/browser/web_contents/web_contents_impl.h"
+#endif
 
 // Including both web_contents_impl.h and node.h would introduce a error, we
 // have to isolate the usage of WebContentsImpl into a clean file to fix it:
@@ -16,6 +19,13 @@ namespace atom {
 
 namespace api {
 
+void WebContents::DetachFromOuterFrame() {
+  // See detach_webview_frame.patch on how to detach.
+  auto* impl = static_cast<content::WebContentsImpl*>(web_contents());
+  impl->GetRenderManagerForTesting()->RemoveOuterDelegateFrame();
+}
+
+#if BUILDFLAG(ENABLE_OSR)
 OffScreenWebContentsView* WebContents::GetOffScreenWebContentsView() const {
   if (IsOffScreen()) {
     const auto* impl =
@@ -35,6 +45,7 @@ OffScreenRenderWidgetHostView* WebContents::GetOffScreenRenderWidgetHostView()
     return nullptr;
   }
 }
+#endif
 
 }  // namespace api
 

filenames.gni

@@ -177,6 +177,7 @@ filenames = {
     "atom/browser/api/atom_api_view.h",
     "atom/browser/api/atom_api_web_contents.cc",
     "atom/browser/api/atom_api_web_contents.h",
+    "atom/browser/api/atom_api_web_contents_impl.cc",
     "atom/browser/api/atom_api_web_contents_mac.mm",
     "atom/browser/api/atom_api_web_contents_view.cc",
     "atom/browser/api/atom_api_web_contents_view.h",

lib/browser/guest-view-manager.js

@@ -181,7 +181,7 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
 
     const oldGuestInstance = guestInstances[oldGuestInstanceId]
     if (oldGuestInstance) {
-      oldGuestInstance.guest.destroy()
+      oldGuestInstance.guest.detachFromOuterFrame()
     }
   }
 
@@ -360,7 +360,7 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event,
 handleMessage('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', function (event, guestInstanceId) {
   try {
     const guest = getGuestForWebContents(guestInstanceId, event.sender)
-    guest.destroy()
+    guest.detachFromOuterFrame()
   } catch (error) {
     console.error(`Guest destroy failed: ${error}`)
   }

patches/common/chromium/disable_detach_webview_frame.patch

@@ -9,6 +9,8 @@ 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 cfa3fd15714c3743cb9d2900b35570c94545fa87..80785902890c57ffae2fa841831d3bd60a8fa11a 100644
 --- a/content/browser/frame_host/render_frame_proxy_host.cc