From 89340008ee7d8d64ebebd410cdaf12c1e0d38307 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2020 10:52:39 -0700 Subject: [PATCH] fix: add patch to prevent crash during frame swap with ctx isolation enabled (#23895) * fix: add patch to prevent crash during frame swap with ctx isolation enabled * Update .patches * chore: update patches Co-authored-by: Samuel Attard Co-authored-by: Samuel Attard --- patches/chromium/.patches | 1 + ...ore_initializing_the_windows_proxies.patch | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 patches/chromium/fix_swap_global_proxies_before_initializing_the_windows_proxies.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 5a62e332ecfa2..a17b7c1c32d53 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -99,3 +99,4 @@ breakpad_treat_node_processes_as_browser_processes.patch upload_list_add_loadsync_method.patch breakpad_allow_getting_string_values_for_crash_keys.patch fix_hunspell_crash.patch +fix_swap_global_proxies_before_initializing_the_windows_proxies.patch diff --git a/patches/chromium/fix_swap_global_proxies_before_initializing_the_windows_proxies.patch b/patches/chromium/fix_swap_global_proxies_before_initializing_the_windows_proxies.patch new file mode 100644 index 0000000000000..bf2222d407caf --- /dev/null +++ b/patches/chromium/fix_swap_global_proxies_before_initializing_the_windows_proxies.patch @@ -0,0 +1,79 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Wed, 20 May 2020 13:48:51 -0700 +Subject: fix: swap global proxies before initializing the windows proxies + +Electron's Context Isolation implementation has a side-effect of initializing +the isolated worlds WindowProxy during the initialization of the main world +WindowProxy as a result of creating the isolated world inside the DidCreateScriptContext +hook. This results in an assertion failing in Chromium during a frame +swap where it expects to be able to set a new global_proxy in the WindowProxy +of the isolated world BEFORE it is initialized. + +To meet this assumption this patch splits SetGlobalProxy into two calls, +SetGlobalProxyWithoutInitializing and InitializeIfNeeded which has the same +resultant effect but means that all of the global_proxy objects are set +BEFORE any WindowProxy's are initialized. + +This could probably be upstreamed as it doesn't affect the way Chromium works +but also it has no benefit for them at this time. + +diff --git a/third_party/blink/renderer/bindings/core/v8/window_proxy.cc b/third_party/blink/renderer/bindings/core/v8/window_proxy.cc +index 389dcd940e93ee043cf7a4435991c64d67386a38..0b5c44d75ae590b493da5584a82ea38a8246a9bf 100644 +--- a/third_party/blink/renderer/bindings/core/v8/window_proxy.cc ++++ b/third_party/blink/renderer/bindings/core/v8/window_proxy.cc +@@ -104,10 +104,7 @@ v8::Local WindowProxy::ReleaseGlobalProxy() { + } + + void WindowProxy::SetGlobalProxy(v8::Local global_proxy) { +- DCHECK_EQ(lifecycle_, Lifecycle::kContextIsUninitialized); +- +- CHECK(global_proxy_.IsEmpty()); +- global_proxy_.Set(isolate_, global_proxy); ++ SetGlobalProxyWithoutInitializing(global_proxy); + + // Initialize the window proxy now, to re-establish the connection between + // the global object and the v8::Context. This is really only needed for a +@@ -118,6 +115,13 @@ void WindowProxy::SetGlobalProxy(v8::Local global_proxy) { + Initialize(); + } + ++void WindowProxy::SetGlobalProxyWithoutInitializing(v8::Local global_proxy) { ++ DCHECK_EQ(lifecycle_, Lifecycle::kContextIsUninitialized); ++ ++ CHECK(global_proxy_.IsEmpty()); ++ global_proxy_.Set(isolate_, global_proxy); ++} ++ + // Create a new environment and setup the global object. + // + // The global object corresponds to a DOMWindow instance. However, to +diff --git a/third_party/blink/renderer/bindings/core/v8/window_proxy.h b/third_party/blink/renderer/bindings/core/v8/window_proxy.h +index ce6aac22126e0a39fa28fcde56e903abd3a9d510..1cf5e056252af5f8ba4725722ac8d9f9d4a88de2 100644 +--- a/third_party/blink/renderer/bindings/core/v8/window_proxy.h ++++ b/third_party/blink/renderer/bindings/core/v8/window_proxy.h +@@ -156,6 +156,7 @@ class WindowProxy : public GarbageCollected { + CORE_EXPORT v8::Local GlobalProxyIfNotDetached(); + v8::Local ReleaseGlobalProxy(); + void SetGlobalProxy(v8::Local); ++ void SetGlobalProxyWithoutInitializing(v8::Local); + + // TODO(dcheng): Temporarily exposed to avoid include cycles. Remove the need + // for this and remove this getter. +diff --git a/third_party/blink/renderer/bindings/core/v8/window_proxy_manager.cc b/third_party/blink/renderer/bindings/core/v8/window_proxy_manager.cc +index ba9845625ea960c1f0be69251a41d51a3a1fd313..a1aa5de76a86cc0a135ae775e4d8455993f10e75 100644 +--- a/third_party/blink/renderer/bindings/core/v8/window_proxy_manager.cc ++++ b/third_party/blink/renderer/bindings/core/v8/window_proxy_manager.cc +@@ -55,8 +55,11 @@ void WindowProxyManager::ReleaseGlobalProxies( + + void WindowProxyManager::SetGlobalProxies( + const GlobalProxyVector& global_proxies) { ++ for (const auto& entry : global_proxies) { ++ WindowProxyMaybeUninitialized(*entry.first)->SetGlobalProxyWithoutInitializing(entry.second); ++ } + for (const auto& entry : global_proxies) +- WindowProxyMaybeUninitialized(*entry.first)->SetGlobalProxy(entry.second); ++ WindowProxyMaybeUninitialized(*entry.first)->InitializeIfNeeded(); + } + + WindowProxyManager::WindowProxyManager(Frame& frame, FrameType frame_type)