Skip to content

Commit

Permalink
chore: restore //url dchecks (#15637)
Browse files Browse the repository at this point in the history
  • Loading branch information
nornagon committed Nov 22, 2018
1 parent fb52fdc commit a8a881c
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 50 deletions.
4 changes: 4 additions & 0 deletions atom/app/atom_main_delegate.cc
Expand Up @@ -278,4 +278,8 @@ bool AtomMainDelegate::DelaySandboxInitialization(
}
#endif

bool AtomMainDelegate::ShouldLockSchemeRegistry() {
return false;
}

} // namespace atom
1 change: 1 addition & 0 deletions atom/app/atom_main_delegate.h
Expand Up @@ -35,6 +35,7 @@ class AtomMainDelegate : public content::ContentMainDelegate {
bool ShouldSendMachPort(const std::string& process_type) override;
bool DelaySandboxInitialization(const std::string& process_type) override;
#endif
bool ShouldLockSchemeRegistry() override;

private:
#if defined(OS_MACOSX)
Expand Down
3 changes: 3 additions & 0 deletions atom/browser/atom_browser_main_parts.cc
Expand Up @@ -421,6 +421,9 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
node_bindings_->PrepareMessageLoop();
node_bindings_->RunMessageLoop();

// url::Add*Scheme are not threadsafe, this helps prevent data races.
url::LockSchemeRegistries();

#if defined(USE_X11)
ui::TouchFactory::SetTouchDeviceListFromCommandLine();
#endif
Expand Down
1 change: 1 addition & 0 deletions patches/common/chromium/.patches
Expand Up @@ -75,3 +75,4 @@ fix_zoom_display.patch
chrome_process_finder.patch
customizable_app_indicator_id_prefix.patch
cross_site_document_resource_handler.patch
content_allow_embedder_to_prevent_locking_scheme_registry.patch
@@ -0,0 +1,56 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp <nornagon@nornagon.net>
Date: Wed, 21 Nov 2018 14:31:34 -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.

Without this patch, calling `registerStandardSchemes` during initialization
when in debug mode will cause a DCHECK to fire.


diff --git a/content/app/content_main_runner_impl.cc b/content/app/content_main_runner_impl.cc
index 034fdcad958cb25780c304d184b2fbb721a13200..c2090368f6678950690463e0b90620cb9fdf5f41 100644
--- a/content/app/content_main_runner_impl.cc
+++ b/content/app/content_main_runner_impl.cc
@@ -808,7 +808,7 @@ int ContentMainRunnerImpl::Initialize(const ContentMainParams& params) {
#endif

RegisterPathProvider();
- RegisterContentSchemes(true);
+ RegisterContentSchemes(delegate_->ShouldLockSchemeRegistry());

#if defined(OS_ANDROID) && (ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE)
int icudata_fd = g_fds->MaybeGet(kAndroidICUDataDescriptor);
diff --git a/content/public/app/content_main_delegate.cc b/content/public/app/content_main_delegate.cc
index c4bdfd36ad0c34b3a91b59502414bc98c091ccee..d7ac740d0088c002bee98238faea1ef9593eee92 100644
--- a/content/public/app/content_main_delegate.cc
+++ b/content/public/app/content_main_delegate.cc
@@ -63,6 +63,10 @@ bool ContentMainDelegate::ShouldEnableProfilerRecording() {
return false;
}

+bool ContentMainDelegate::ShouldLockSchemeRegistry() {
+ return true;
+}
+
service_manager::ProcessType ContentMainDelegate::OverrideProcessType() {
return service_manager::ProcessType::kDefault;
}
diff --git a/content/public/app/content_main_delegate.h b/content/public/app/content_main_delegate.h
index 979e25d1c1b9ff4b16adbab28cb44d8473f8232a..7aca113058ed111a98e396df57ebc36c06e3b896 100644
--- a/content/public/app/content_main_delegate.h
+++ b/content/public/app/content_main_delegate.h
@@ -101,6 +101,9 @@ class CONTENT_EXPORT ContentMainDelegate {
// Returns whether or not profiler recording should be enabled.
virtual bool ShouldEnableProfilerRecording();

+ // Allows the embedder to prevent locking the scheme registry.
+ virtual bool ShouldLockSchemeRegistry();
+
// Fatal errors during initialization are reported by this function, so that
// the embedder can implement graceful exit by displaying some message and
// returning initialization error code. Default behavior is CHECK(false).
50 changes: 0 additions & 50 deletions patches/common/chromium/dcheck.patch
Expand Up @@ -16,41 +16,6 @@ example, the checks might be disabled for a whole build target, but actually
only one or two specific checks fail. Then it's better to simply comment out the
failing checks and allow the rest of the target to have them enabled.

Please keep the following lists updated.

The ELECTRON_NO_DCHECK build flag disables debug checks universally.
This patch applies the flag to the following GN targets:

third_party/blink/renderer/core/loader:loader
url:url

These files have debug checks explicitly commented out:

base/memory/weak_ptr.cc
base/process/kill_win.cc
components/viz/service/display/program_binding.h
components/viz/service/display_embedder/server_shared_bitmap_manager.cc
content/browser/frame_host/navigation_controller_impl.cc
content/browser/frame_host/render_frame_host_impl.cc
content/browser/renderer_host/render_widget_host_view_mac.mm
ppapi/host/ppapi_host.cc
third_party/blink/renderer/core/dom/node.cc
third_party/blink/renderer/platform/wtf/text/string_impl.cc
ui/base/clipboard/clipboard_win.cc

diff --git a/base/logging.h b/base/logging.h
index 08c1f0fc59672b2134c634e081f0c4df4d261b75..7a758ce3ef90145d2b68e07e17f00fc30cfb30a6 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -874,7 +874,7 @@ const LogSeverity LOG_DCHECK = LOG_FATAL;

#else // !(defined(_PREFAST_) && defined(OS_WIN))

-#if DCHECK_IS_ON()
+#if DCHECK_IS_ON() && !defined(ELECTRON_NO_DCHECK)

#define DCHECK(condition) \
LAZY_STREAM(LOG_STREAM(DCHECK), !ANALYZER_ASSUME_TRUE(condition)) \
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index d8aca9e2cbffdfd0bbb0bd67e8adfb53547132bb..86224ab7a3c5637422559da25bc8c1040a03e937 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
Expand Down Expand Up @@ -138,18 +103,3 @@ index e49dd8c81270cdd9794ddee11bad036c2f444af5..9e61c901cd2df168520b83a8522ca8c0
FreeData(format, handle);
}
}
diff --git a/url/BUILD.gn b/url/BUILD.gn
index 57bbe16c15ea442963a53f89b009e2c99c7b090a..07725187c595784d9e5f49d45a8835da78144830 100644
--- a/url/BUILD.gn
+++ b/url/BUILD.gn
@@ -98,6 +98,10 @@ component("url") {
]
deps += [ "//third_party/icu" ]
}
+
+ if (is_electron_build) {
+ defines += [ "ELECTRON_NO_DCHECK" ]
+ }
}

if (is_android) {

0 comments on commit a8a881c

Please sign in to comment.