fix(extensions): devtools now open for background pages (#22217)

refactor(extensions): remove unused InitWithBrowserContext method

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

The background page WebContents instance is managed by the ExtensionHost.

fix(extensions): open background page devtools detached by default

test(extensions): add background page devtools test

chore: test fix for null web_contents()

fix: close background page devtools in test after opening

shell/browser/api/electron_api_web_contents.cc

@@ -132,6 +132,7 @@
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 #include "extensions/browser/script_executor.h"
+#include "extensions/browser/view_type_utils.h"
 #include "shell/browser/extensions/electron_extension_web_contents_observer.h"
 #endif
 
@@ -409,12 +410,45 @@ const void* kElectronApiWebContentsKey = &kElectronApiWebContentsKey;
 
 }  // namespace
 
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+
+WebContents::Type GetTypeFromViewType(extensions::ViewType view_type) {
+  switch (view_type) {
+    case extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE:
+      return WebContents::Type::BACKGROUND_PAGE;
+
+    case extensions::VIEW_TYPE_APP_WINDOW:
+    case extensions::VIEW_TYPE_COMPONENT:
+    case extensions::VIEW_TYPE_EXTENSION_DIALOG:
+    case extensions::VIEW_TYPE_EXTENSION_POPUP:
+    case extensions::VIEW_TYPE_BACKGROUND_CONTENTS:
+    case extensions::VIEW_TYPE_EXTENSION_GUEST:
+    case extensions::VIEW_TYPE_TAB_CONTENTS:
+    case extensions::VIEW_TYPE_INVALID:
+      return WebContents::Type::REMOTE;
+  }
+}
+
+#endif
+
 WebContents::WebContents(v8::Isolate* isolate,
                          content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       type_(Type::REMOTE),
       id_(GetAllWebContents().Add(this)),
       weak_factory_(this) {
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+  // WebContents created by extension host will have valid ViewType set.
+  extensions::ViewType view_type = extensions::GetViewType(web_contents);
+  if (view_type != extensions::VIEW_TYPE_INVALID) {
+    InitWithExtensionView(isolate, web_contents, view_type);
+  }
+
+  extensions::ElectronExtensionWebContentsObserver::CreateForWebContents(
+      web_contents);
+  script_executor_.reset(new extensions::ScriptExecutor(web_contents));
+#endif
+
   auto session = Session::CreateFrom(isolate, GetBrowserContext());
   session_.Reset(isolate, session.ToV8());
 
@@ -424,11 +458,7 @@ WebContents::WebContents(v8::Isolate* isolate,
   web_contents->SetUserData(kElectronApiWebContentsKey,
                             std::make_unique<UserDataLink>(GetWeakPtr()));
   InitZoomController(web_contents, gin::Dictionary::CreateEmpty(isolate));
-#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+
-  extensions::ElectronExtensionWebContentsObserver::CreateForWebContents(
-      web_contents);
-  script_executor_.reset(new extensions::ScriptExecutor(web_contents));
-#endif
   registry_.AddInterface(base::BindRepeating(&WebContents::BindElectronBrowser,
                                              base::Unretained(this)));
   receivers_.set_disconnect_handler(base::BindRepeating(
@@ -636,13 +666,39 @@ void WebContents::InitWithSessionAndOptions(
                               std::make_unique<UserDataLink>(GetWeakPtr()));
 }
 
+#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() {
   MarkDestroyed();
   // The destroy() is called.
   if (managed_web_contents()) {
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+    if (type_ == Type::BACKGROUND_PAGE) {
+      // Background pages are owned by extensions::ExtensionHost
+      managed_web_contents()->ReleaseWebContents();
+    }
+#endif
+
     managed_web_contents()->GetView()->SetDelegate(nullptr);
 
-    RenderViewDeleted(web_contents()->GetRenderViewHost());
+    if (web_contents()) {
+      RenderViewDeleted(web_contents()->GetRenderViewHost());
+    }
 
     if (type_ == Type::BROWSER_WINDOW && owner_window()) {
       // For BrowserWindow we should close the window and clean up everything
@@ -1379,6 +1435,7 @@ void WebContents::DevToolsOpened() {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
+  DCHECK(managed_web_contents());
   auto handle =
       FromOrCreate(isolate, managed_web_contents()->GetDevToolsWebContents());
   devtools_web_contents_.Reset(isolate, handle.ToV8());
@@ -1720,7 +1777,8 @@ void WebContents::OpenDevTools(gin::Arguments* args) {
     return;
 
   std::string state;
-  if (type_ == Type::WEB_VIEW || !owner_window()) {
+  if (type_ == Type::WEB_VIEW || type_ == Type::BACKGROUND_PAGE ||
+      !owner_window()) {
     state = "detach";
   }
   bool activate = true;
@@ -1731,6 +1789,8 @@ void WebContents::OpenDevTools(gin::Arguments* args) {
       options.Get("activate", &activate);
     }
   }
+
+  DCHECK(managed_web_contents());
   managed_web_contents()->SetDockState(state);
   managed_web_contents()->ShowDevTools(activate);
 }
@@ -1739,6 +1799,7 @@ void WebContents::CloseDevTools() {
   if (type_ == Type::REMOTE)
     return;
 
+  DCHECK(managed_web_contents());
   managed_web_contents()->CloseDevTools();
 }
 
@@ -1746,6 +1807,7 @@ bool WebContents::IsDevToolsOpened() {
   if (type_ == Type::REMOTE)
     return false;
 
+  DCHECK(managed_web_contents());
   return managed_web_contents()->IsDevToolsViewShowing();
 }
 
@@ -1753,6 +1815,7 @@ bool WebContents::IsDevToolsFocused() {
   if (type_ == Type::REMOTE)
     return false;
 
+  DCHECK(managed_web_contents());
   return managed_web_contents()->GetView()->IsDevToolsViewFocused();
 }
 
@@ -1761,6 +1824,7 @@ void WebContents::EnableDeviceEmulation(
   if (type_ == Type::REMOTE)
     return;
 
+  DCHECK(web_contents());
   auto* frame_host = web_contents()->GetMainFrame();
   if (frame_host) {
     auto* widget_host_impl =
@@ -1778,6 +1842,7 @@ void WebContents::DisableDeviceEmulation() {
   if (type_ == Type::REMOTE)
     return;
 
+  DCHECK(web_contents());
   auto* frame_host = web_contents()->GetMainFrame();
   if (frame_host) {
     auto* widget_host_impl =
@@ -1805,6 +1870,7 @@ void WebContents::InspectElement(int x, int y) {
   if (!enable_devtools_)
     return;
 
+  DCHECK(managed_web_contents());
   if (!managed_web_contents()->GetDevToolsWebContents())
     OpenDevTools(nullptr);
   managed_web_contents()->InspectElement(x, y);

shell/browser/api/electron_api_web_contents.h

@@ -48,6 +48,8 @@
 #endif
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+#include "extensions/common/view_type.h"
+
 namespace extensions {
 class ScriptExecutor;
 }
@@ -144,7 +146,7 @@ class WebContents : public gin::Wrappable<WebContents>,
                     public mojom::ElectronBrowser {
  public:
   enum class Type {
-    BACKGROUND_PAGE,  // A DevTools extension background page.
+    BACKGROUND_PAGE,  // An extension background page.
     BROWSER_WINDOW,   // Used by BrowserWindow.
     BROWSER_VIEW,     // Used by BrowserView.
     REMOTE,           // Thin wrap around an existing WebContents.
@@ -453,6 +455,12 @@ class WebContents : public gin::Wrappable<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,

shell/browser/extensions/electron_extensions_browser_client.h

@@ -32,8 +32,6 @@ namespace electron {
 
 // An ExtensionsBrowserClient that supports a single content::BrowserContext
 // with no related incognito context.
-// Must be initialized via InitWithBrowserContext() once the BrowserContext is
-// created.
 class ElectronExtensionsBrowserClient
     : public extensions::ExtensionsBrowserClient {
  public:
@@ -122,11 +120,6 @@ class ElectronExtensionsBrowserClient
       content::RenderFrameHost* render_frame_host,
       const extensions::Extension* extension) const override;
 
-  // |context| is the single BrowserContext used for IsValidContext().
-  // |pref_service| is used for GetPrefServiceForContext().
-  void InitWithBrowserContext(content::BrowserContext* context,
-                              PrefService* pref_service);
-
   // Sets the API client.
   void SetAPIClientForTest(extensions::ExtensionsAPIClient* api_client);
 

spec-main/extensions-spec.ts

@@ -317,16 +317,28 @@ describe('chrome extensions', () => {
       const receivedMessage = await w.webContents.executeJavaScript('window.completionPromise');
       expect(receivedMessage).to.deep.equal({ some: 'message' });
     });
-  });
 
-  it('has session in background page', async () => {
+    it('has session in background page', async () => {
-    const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
+      const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
-    await customSession.loadExtension(path.join(fixtures, 'extensions', 'persistent-background-page'));
+      await customSession.loadExtension(path.join(fixtures, 'extensions', 'persistent-background-page'));
-    const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } });
+      const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } });
-    const promise = emittedOnce(app, 'web-contents-created');
+      const promise = emittedOnce(app, 'web-contents-created');
-    await w.loadURL('about:blank');
+      await w.loadURL('about:blank');
-    const [, bgPageContents] = await promise;
+      const [, bgPageContents] = await promise;
-    expect(bgPageContents.session).to.not.equal(undefined);
+      expect(bgPageContents.session).to.not.equal(undefined);
+    });
+
+    it('can open devtools of background page', async () => {
+      const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
+      await customSession.loadExtension(path.join(fixtures, 'extensions', 'persistent-background-page'));
+      const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } });
+      const promise = emittedOnce(app, 'web-contents-created');
+      await w.loadURL('about:blank');
+      const [, bgPageContents] = await promise;
+      expect(bgPageContents.getType()).to.equal('backgroundPage');
+      bgPageContents.openDevTools();
+      bgPageContents.closeDevTools();
+    });
   });
 
   describe('devtools extensions', () => {