fix(extensions): release background page WebContents to avoid crash

The background page WebContents instance is managed by the ExtensionHost.
electron · Mar 2, 2020 · 6b66e5e · 6b66e5e
1 parent fa5da0f
commit 6b66e5e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 8 deletions.
diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
@@ -403,16 +403,9 @@ WebContents::WebContents(v8::Isolate* isolate,
       weak_factory_(this) {
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
   // WebContents created by extension host will have valid ViewType set.
-  // Must reassign type prior to calling 'Init'.
   extensions::ViewType view_type = extensions::GetViewType(web_contents);
   if (view_type != extensions::VIEW_TYPE_INVALID) {
-    type_ = GetTypeFromViewType(view_type);
-
-    // Allow toggling DevTools for background pages
-    // TODO(samuelmaddock): this causes a crash on shutdown
-    Observe(web_contents);
-    InitWithWebContents(web_contents, GetBrowserContext(), IsGuest());
-    managed_web_contents()->GetView()->SetDelegate(this);
+    InitWithExtensionView(isolate, web_contents, view_type);
   }
 
   extensions::ElectronExtensionWebContentsObserver::CreateForWebContents(
@@ -613,13 +606,37 @@ void WebContents::InitWithSessionAndOptions(
   AttachAsUserData(web_contents());
 }
 
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+void WebContents::InitWithExtensionView(v8::Isolate* isolate,
+                                        content::WebContents* web_contents,
+                                        extensions::ViewType view_type) {
+  // Must reassign type prior to calling `Init`.
+  type_ = GetTypeFromViewType(view_type);
+  if (GetType() == Type::REMOTE)
+    return;
+
+  // Allow toggling DevTools for background pages
+  Observe(web_contents);
+  InitWithWebContents(web_contents, GetBrowserContext(), IsGuest());
+  managed_web_contents()->GetView()->SetDelegate(this);
+  SecurityStateTabHelper::CreateForWebContents(web_contents);
+}
+#endif
+
 WebContents::~WebContents() {
   // The destroy() is called.
   if (managed_web_contents()) {
     managed_web_contents()->GetView()->SetDelegate(nullptr);
 
     RenderViewDeleted(web_contents()->GetRenderViewHost());
 
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+    if (type_ == Type::BACKGROUND_PAGE) {
+      // Background pages are owned by extensions::ExtensionHost
+      managed_web_contents()->ReleaseWebContents();
+    }
+#endif
+
     if (type_ == Type::BROWSER_WINDOW && owner_window()) {
       // For BrowserWindow we should close the window and clean up everything
       // before WebContents is destroyed.

diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h
@@ -44,6 +44,7 @@
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 #include "extensions/browser/script_executor.h"
+#include "extensions/common/view_type.h"
 #endif
 
 namespace blink {
@@ -372,6 +373,12 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
       gin::Handle<class Session> session,
       const gin_helper::Dictionary& options);
 
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+  void InitWithExtensionView(v8::Isolate* isolate,
+                             content::WebContents* web_contents,
+                             extensions::ViewType view_type);
+#endif
+
   // content::WebContentsDelegate:
   bool DidAddMessageToConsole(content::WebContents* source,
                               blink::mojom::ConsoleMessageLevel level,