fix: do not initialize extension system in in-memory sessions (#23472)

* fix: do not initialize any extension related logic in OffTheRecord contexts

* chore: fix linting

Co-authored-by: Samuel Attard <[email protected]>

shell/browser/electron_browser_client.cc

@@ -916,24 +916,26 @@ void ElectronBrowserClient::SiteInstanceGotProcess(
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
   auto* browser_context =
       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext());
-  extensions::ExtensionRegistry* registry =
-      extensions::ExtensionRegistry::Get(browser_context);
-  const extensions::Extension* extension =
-      registry->enabled_extensions().GetExtensionOrAppByURL(
-          site_instance->GetSiteURL());
-  if (!extension)
-    return;
+  if (!browser_context->IsOffTheRecord()) {
+    extensions::ExtensionRegistry* registry =
+        extensions::ExtensionRegistry::Get(browser_context);
+    const extensions::Extension* extension =
+        registry->enabled_extensions().GetExtensionOrAppByURL(
+            site_instance->GetSiteURL());
+    if (!extension)
+      return;
 
-  extensions::ProcessMap::Get(browser_context)
-      ->Insert(extension->id(), site_instance->GetProcess()->GetID(),
-               site_instance->GetId());
+    extensions::ProcessMap::Get(browser_context)
+        ->Insert(extension->id(), site_instance->GetProcess()->GetID(),
+                 site_instance->GetId());
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&extensions::InfoMap::RegisterExtensionProcess,
-                     browser_context->extension_system()->info_map(),
-                     extension->id(), site_instance->GetProcess()->GetID(),
-                     site_instance->GetId()));
+    base::PostTask(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&extensions::InfoMap::RegisterExtensionProcess,
+                       browser_context->extension_system()->info_map(),
+                       extension->id(), site_instance->GetProcess()->GetID(),
+                       site_instance->GetId()));
+  }
 #endif  // BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 }
 
@@ -998,25 +1000,27 @@ void ElectronBrowserClient::SiteInstanceDeleting(
 
   auto* browser_context =
       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext());
-  // If this isn't an extension renderer there's nothing to do.
-  extensions::ExtensionRegistry* registry =
-      extensions::ExtensionRegistry::Get(browser_context);
-  const extensions::Extension* extension =
-      registry->enabled_extensions().GetExtensionOrAppByURL(
-          site_instance->GetSiteURL());
-  if (!extension)
-    return;
+  if (!browser_context->IsOffTheRecord()) {
+    // If this isn't an extension renderer there's nothing to do.
+    extensions::ExtensionRegistry* registry =
+        extensions::ExtensionRegistry::Get(browser_context);
+    const extensions::Extension* extension =
+        registry->enabled_extensions().GetExtensionOrAppByURL(
+            site_instance->GetSiteURL());
+    if (!extension)
+      return;
 
-  extensions::ProcessMap::Get(browser_context)
-      ->Remove(extension->id(), site_instance->GetProcess()->GetID(),
-               site_instance->GetId());
+    extensions::ProcessMap::Get(browser_context)
+        ->Remove(extension->id(), site_instance->GetProcess()->GetID(),
+                 site_instance->GetId());
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&extensions::InfoMap::UnregisterExtensionProcess,
-                     browser_context->extension_system()->info_map(),
-                     extension->id(), site_instance->GetProcess()->GetID(),
-                     site_instance->GetId()));
+    base::PostTask(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&extensions::InfoMap::UnregisterExtensionProcess,
+                       browser_context->extension_system()->info_map(),
+                       extension->id(), site_instance->GetProcess()->GetID(),
+                       site_instance->GetId()));
+  }
 #endif
 }
 

shell/browser/electron_browser_context.cc

@@ -139,13 +139,15 @@ ElectronBrowserContext::ElectronBrowserContext(const std::string& partition,
   cookie_change_notifier_ = std::make_unique<CookieChangeNotifier>(this);
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-  BrowserContextDependencyManager::GetInstance()->CreateBrowserContextServices(
-      this);
-
-  extension_system_ = static_cast<extensions::ElectronExtensionSystem*>(
-      extensions::ExtensionSystem::Get(this));
-  extension_system_->InitForRegularProfile(true /* extensions_enabled */);
-  extension_system_->FinishInitialization();
+  if (!in_memory_) {
+    BrowserContextDependencyManager::GetInstance()
+        ->CreateBrowserContextServices(this);
+
+    extension_system_ = static_cast<extensions::ElectronExtensionSystem*>(
+        extensions::ExtensionSystem::Get(this));
+    extension_system_->InitForRegularProfile(true /* extensions_enabled */);
+    extension_system_->FinishInitialization();
+  }
 #endif
 }
 
@@ -171,10 +173,12 @@ void ElectronBrowserContext::InitPrefs() {
   prefs_factory.set_user_prefs(pref_store);
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-  auto* ext_pref_store = new ExtensionPrefStore(
-      ExtensionPrefValueMapFactory::GetForBrowserContext(this),
-      IsOffTheRecord());
-  prefs_factory.set_extension_prefs(ext_pref_store);
+  if (!in_memory_) {
+    auto* ext_pref_store = new ExtensionPrefStore(
+        ExtensionPrefValueMapFactory::GetForBrowserContext(this),
+        IsOffTheRecord());
+    prefs_factory.set_extension_prefs(ext_pref_store);
+  }
 #endif
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) || \
@@ -196,7 +200,8 @@ void ElectronBrowserContext::InitPrefs() {
   ZoomLevelDelegate::RegisterPrefs(registry.get());
   PrefProxyConfigTrackerImpl::RegisterPrefs(registry.get());
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-  extensions::ExtensionPrefs::RegisterProfilePrefs(registry.get());
+  if (!in_memory_)
+    extensions::ExtensionPrefs::RegisterProfilePrefs(registry.get());
 #endif
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)

shell/browser/electron_browser_context.h

@@ -142,6 +142,8 @@ class ElectronBrowserContext
   }
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+  // Guard usages of extension_system() with !IsOffTheRecord()
+  // There is no extension system for in-memory sessions
   extensions::ElectronExtensionSystem* extension_system() {
     return extension_system_;
   }

spec-main/crash-spec.ts

@@ -0,0 +1,54 @@
+import { expect } from 'chai';
+import * as cp from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
+
+const fixturePath = path.resolve(__dirname, 'fixtures', 'crash-cases');
+
+let children: cp.ChildProcessWithoutNullStreams[] = [];
+
+const runFixtureAndEnsureCleanExit = (args: string[]) => {
+  let out = '';
+  const child = cp.spawn(process.execPath, args);
+  children.push(child);
+  child.stdout.on('data', (chunk: Buffer) => {
+    out += chunk.toString();
+  });
+  child.stderr.on('data', (chunk: Buffer) => {
+    out += chunk.toString();
+  });
+  return new Promise((resolve) => {
+    child.on('exit', (code, signal) => {
+      if (code !== 0 || signal !== null) {
+        console.error(out);
+      }
+      expect(signal).to.equal(null, 'exit signal should be null');
+      expect(code).to.equal(0, 'should have exited with code 0');
+      children = children.filter(c => c !== child);
+      resolve();
+    });
+  });
+};
+
+describe('crash cases', () => {
+  afterEach(() => {
+    for (const child of children) {
+      child.kill();
+    }
+    expect(children).to.have.lengthOf(0, 'all child processes should have exited cleanly');
+    children.length = 0;
+  });
+  const cases = fs.readdirSync(fixturePath);
+
+  for (const crashCase of cases) {
+    it(`the "${crashCase}" case should not crash`, () => {
+      const fixture = path.resolve(fixturePath, crashCase);
+      const argsFile = path.resolve(fixture, 'electron.args');
+      const args = [fixture];
+      if (fs.existsSync(argsFile)) {
+        args.push(...fs.readFileSync(argsFile, 'utf8').trim().split('\n'));
+      }
+      return runFixtureAndEnsureCleanExit(args);
+    });
+  }
+});

spec-main/fixtures/crash-cases/early-in-memory-session-create/index.js

@@ -0,0 +1,6 @@
+const { app, session } = require('electron');
+
+app.on('ready', () => {
+  session.fromPartition('in-memory');
+  process.exit(0);
+});