fix: do not initialize any extension related logic in OffTheRecord contexts (#22772)
This commit is contained in:
		
					parent
					
						
							
								a3ee61f963
							
						
					
				
			
			
				commit
				
					
						21900fe4f4
					
				
			
		
					 5 changed files with 115 additions and 44 deletions
				
			
		|  | @ -917,6 +917,7 @@ void ElectronBrowserClient::SiteInstanceGotProcess( | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | ||||||
|   auto* browser_context = |   auto* browser_context = | ||||||
|       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext()); |       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext()); | ||||||
|  |   if (!browser_context->IsOffTheRecord()) { | ||||||
|     extensions::ExtensionRegistry* registry = |     extensions::ExtensionRegistry* registry = | ||||||
|         extensions::ExtensionRegistry::Get(browser_context); |         extensions::ExtensionRegistry::Get(browser_context); | ||||||
|     const extensions::Extension* extension = |     const extensions::Extension* extension = | ||||||
|  | @ -935,6 +936,7 @@ void ElectronBrowserClient::SiteInstanceGotProcess( | ||||||
|                        browser_context->extension_system()->info_map(), |                        browser_context->extension_system()->info_map(), | ||||||
|                        extension->id(), site_instance->GetProcess()->GetID(), |                        extension->id(), site_instance->GetProcess()->GetID(), | ||||||
|                        site_instance->GetId())); |                        site_instance->GetId())); | ||||||
|  |   } | ||||||
| #endif  // BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 | #endif  // BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -999,6 +1001,7 @@ void ElectronBrowserClient::SiteInstanceDeleting( | ||||||
| 
 | 
 | ||||||
|   auto* browser_context = |   auto* browser_context = | ||||||
|       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext()); |       static_cast<ElectronBrowserContext*>(site_instance->GetBrowserContext()); | ||||||
|  |   if (!browser_context->IsOffTheRecord()) { | ||||||
|     // If this isn't an extension renderer there's nothing to do.
 |     // If this isn't an extension renderer there's nothing to do.
 | ||||||
|     extensions::ExtensionRegistry* registry = |     extensions::ExtensionRegistry* registry = | ||||||
|         extensions::ExtensionRegistry::Get(browser_context); |         extensions::ExtensionRegistry::Get(browser_context); | ||||||
|  | @ -1018,6 +1021,7 @@ void ElectronBrowserClient::SiteInstanceDeleting( | ||||||
|                        browser_context->extension_system()->info_map(), |                        browser_context->extension_system()->info_map(), | ||||||
|                        extension->id(), site_instance->GetProcess()->GetID(), |                        extension->id(), site_instance->GetProcess()->GetID(), | ||||||
|                        site_instance->GetId())); |                        site_instance->GetId())); | ||||||
|  |   } | ||||||
| #endif | #endif | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -139,13 +139,15 @@ ElectronBrowserContext::ElectronBrowserContext(const std::string& partition, | ||||||
|   cookie_change_notifier_ = std::make_unique<CookieChangeNotifier>(this); |   cookie_change_notifier_ = std::make_unique<CookieChangeNotifier>(this); | ||||||
| 
 | 
 | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | ||||||
|   BrowserContextDependencyManager::GetInstance()->CreateBrowserContextServices( |   if (!in_memory_) { | ||||||
|       this); |     BrowserContextDependencyManager::GetInstance() | ||||||
|  |         ->CreateBrowserContextServices(this); | ||||||
| 
 | 
 | ||||||
|     extension_system_ = static_cast<extensions::ElectronExtensionSystem*>( |     extension_system_ = static_cast<extensions::ElectronExtensionSystem*>( | ||||||
|         extensions::ExtensionSystem::Get(this)); |         extensions::ExtensionSystem::Get(this)); | ||||||
|     extension_system_->InitForRegularProfile(true /* extensions_enabled */); |     extension_system_->InitForRegularProfile(true /* extensions_enabled */); | ||||||
|     extension_system_->FinishInitialization(); |     extension_system_->FinishInitialization(); | ||||||
|  |   } | ||||||
| #endif | #endif | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -171,10 +173,12 @@ void ElectronBrowserContext::InitPrefs() { | ||||||
|   prefs_factory.set_user_prefs(pref_store); |   prefs_factory.set_user_prefs(pref_store); | ||||||
| 
 | 
 | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | ||||||
|  |   if (!in_memory_) { | ||||||
|     auto* ext_pref_store = new ExtensionPrefStore( |     auto* ext_pref_store = new ExtensionPrefStore( | ||||||
|         ExtensionPrefValueMapFactory::GetForBrowserContext(this), |         ExtensionPrefValueMapFactory::GetForBrowserContext(this), | ||||||
|         IsOffTheRecord()); |         IsOffTheRecord()); | ||||||
|     prefs_factory.set_extension_prefs(ext_pref_store); |     prefs_factory.set_extension_prefs(ext_pref_store); | ||||||
|  |   } | ||||||
| #endif | #endif | ||||||
| 
 | 
 | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) || \ | #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) || \ | ||||||
|  | @ -196,6 +200,7 @@ void ElectronBrowserContext::InitPrefs() { | ||||||
|   ZoomLevelDelegate::RegisterPrefs(registry.get()); |   ZoomLevelDelegate::RegisterPrefs(registry.get()); | ||||||
|   PrefProxyConfigTrackerImpl::RegisterPrefs(registry.get()); |   PrefProxyConfigTrackerImpl::RegisterPrefs(registry.get()); | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | ||||||
|  |   if (!in_memory_) | ||||||
|     extensions::ExtensionPrefs::RegisterProfilePrefs(registry.get()); |     extensions::ExtensionPrefs::RegisterProfilePrefs(registry.get()); | ||||||
| #endif | #endif | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -139,6 +139,8 @@ class ElectronBrowserContext | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | #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() { |   extensions::ElectronExtensionSystem* extension_system() { | ||||||
|     return extension_system_; |     return extension_system_; | ||||||
|   } |   } | ||||||
|  |  | ||||||
							
								
								
									
										54
									
								
								spec-main/crash-spec.ts
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										54
									
								
								spec-main/crash-spec.ts
									
										
									
									
									
										Normal file
									
								
							|  | @ -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) | ||||||
|  |     }) | ||||||
|  |   } | ||||||
|  | }) | ||||||
|  | @ -0,0 +1,6 @@ | ||||||
|  | const { app, session } = require('electron') | ||||||
|  | 
 | ||||||
|  | app.on('ready', () => { | ||||||
|  |   session.fromPartition('in-memory') | ||||||
|  |   process.exit(0) | ||||||
|  | }) | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Samuel Attard
				Samuel Attard