From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Andy Locascio Date: Tue, 18 Feb 2020 14:35:04 -0800 Subject: content: allow embedder to prevent locking scheme registry The //content layer requires all schemes to be registered during startup, because Add*Scheme aren't threadsafe. However, Electron exposes the option to register additional schemes via JavaScript in the main process before the app is ready, but after the //content layer has already locked the registry. This allows embedders to optionally keep the scheme registry unlocked, and it is their responsibility to ensure that it is not accessed in a way that would cause potential thread-safety issues. Previously upstreamed patch: https://chromium-review.googlesource.com/c/chromium/src/+/1637040 This change was lost during upstream refactor in https://chromium-review.googlesource.com/c/chromium/src/+/1901591, we should try re-submitting the patch. diff --git a/content/app/content_main_runner_impl.cc b/content/app/content_main_runner_impl.cc index 8b557bdeb4b8f3660cb9aa1adaf2bf28b6a326bf..aa8ffdd580292b88ff32ea536d34080c12db1778 100644 --- a/content/app/content_main_runner_impl.cc +++ b/content/app/content_main_runner_impl.cc @@ -737,7 +737,7 @@ int ContentMainRunnerImpl::Initialize(const ContentMainParams& params) { } #endif - RegisterContentSchemes(); + RegisterContentSchemes(delegate_->ShouldLockSchemeRegistry()); ContentClientInitializer::Set(process_type, delegate_); #if !defined(OS_ANDROID) diff --git a/content/common/url_schemes.cc b/content/common/url_schemes.cc index dc37f121130d83e200d73dd1ad566847548ac0fd..63080c1bc486a488841fc5d2081f4d5d4a00bde3 100644 --- a/content/common/url_schemes.cc +++ b/content/common/url_schemes.cc @@ -49,7 +49,7 @@ std::vector& GetMutableServiceWorkerSchemes() { } // namespace -void RegisterContentSchemes() { +void RegisterContentSchemes(bool should_lock_registry) { // On Android and in tests, schemes may have been registered already. if (g_registered_url_schemes) return; @@ -106,7 +106,8 @@ void RegisterContentSchemes() { // threadsafe so must be called when GURL isn't used on any other thread. This // is really easy to mess up, so we say that all calls to Add*Scheme in Chrome // must be inside this function. - url::LockSchemeRegistries(); + if (should_lock_registry) + url::LockSchemeRegistries(); // Combine the default savable schemes with the additional ones given. GetMutableSavableSchemes().assign(std::begin(kDefaultSavableSchemes), diff --git a/content/common/url_schemes.h b/content/common/url_schemes.h index 3038f9d25798f36811b6398f8cc0e7d83ecc41b0..68189c36c47ef85b345b0ccc40c456f889977bee 100644 --- a/content/common/url_schemes.h +++ b/content/common/url_schemes.h @@ -16,7 +16,7 @@ namespace content { // parsed as "standard" or "referrer" with the src/url/ library, then locks the // sets of schemes down. The embedder can add additional schemes by // overriding the ContentClient::AddAdditionalSchemes method. -CONTENT_EXPORT void RegisterContentSchemes(); +CONTENT_EXPORT void RegisterContentSchemes(bool should_lock_registry = true); // Re-initializes schemes for tests. CONTENT_EXPORT void ReRegisterContentSchemesForTests(); diff --git a/content/public/app/content_main_delegate.cc b/content/public/app/content_main_delegate.cc index 17d256b852b6e7615cf4e52e4a2d7fa326a4df44..bd0512902df4484e8714cda9ab1352b1c672a1cd 100644 --- a/content/public/app/content_main_delegate.cc +++ b/content/public/app/content_main_delegate.cc @@ -42,6 +42,10 @@ bool ContentMainDelegate::ShouldHandleConsoleControlEvents() { } #endif +bool ContentMainDelegate::ShouldLockSchemeRegistry() { + return true; +} + bool ContentMainDelegate::ShouldCreateFeatureList() { return true; } diff --git a/content/public/app/content_main_delegate.h b/content/public/app/content_main_delegate.h index dd16f2c082665e74cc3c0aaa1ddb53977ce4a7ea..1806baf2d97b36e5c5097516c82d14fce7ca41a5 100644 --- a/content/public/app/content_main_delegate.h +++ b/content/public/app/content_main_delegate.h @@ -65,6 +65,20 @@ class CONTENT_EXPORT ContentMainDelegate { // returning initialization error code. Default behavior is CHECK(false). virtual int TerminateForFatalInitializationError(); + // Allows the embedder to prevent locking the scheme registry. The scheme + // registry is the list of URL schemes we recognize, with some additional + // information about each scheme such as whether it expects a host. The + // scheme registry is not thread-safe, so by default it is locked before any + // threads are created to ensure single-threaded access. An embedder can + // override this to prevent the scheme registry from being locked during + // startup, but if they do so then they are responsible for making sure that + // the registry is only accessed in a thread-safe way, and for calling + // url::LockSchemeRegistries() when initialization is complete. If possible, + // prefer registering additional schemes through + // ContentClient::AddAdditionalSchemes over preventing the scheme registry + // from being locked. + virtual bool ShouldLockSchemeRegistry(); + // Allows the embedder to perform platform-specific initialization before // BrowserMain() is invoked (i.e. before BrowserMainRunner, BrowserMainLoop, // BrowserMainParts, etc. are created).