From 6b66e5e198b63defe705bdf54d2feab217ddc9a9 Mon Sep 17 00:00:00 2001 From: samuelmaddock Date: Mon, 2 Mar 2020 09:42:13 -0500 Subject: [PATCH] fix(extensions): release background page WebContents to avoid crash The background page WebContents instance is managed by the ExtensionHost. --- .../browser/api/electron_api_web_contents.cc | 33 ++++++++++++++----- shell/browser/api/electron_api_web_contents.h | 7 ++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 04d7daf6197ff..df4f2bba37517 100644 --- 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,6 +606,23 @@ 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()) { @@ -620,6 +630,13 @@ WebContents::~WebContents() { 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 index 4218944b5667f..08ced351c9dcc 100644 --- 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, gin::Handle 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,