Browse Source

fix: add patch to prevent crash during frame swap with ctx isolation enabled (#23894)

* fix: add patch to prevent crash during frame swap with ctx isolation enabled

* Update .patches

* chore: update patches

Co-authored-by: Samuel Attard <[email protected]>
Co-authored-by: Samuel Attard <[email protected]>
trop[bot] 4 years ago
parent
commit
b045ee77af

+ 1 - 0
patches/chromium/.patches

@@ -103,3 +103,4 @@ cherry-pick-86c02c5dcd37.patch
 fix_hunspell_crash.patch
 avoid_nullptr_dereference_in_rtcpeerconnectionhandler.patch
 reland_onstate_handler_is_allowed_to_close_a_peerconnection.patch
+fix_swap_global_proxies_before_initializing_the_windows_proxies.patch

+ 79 - 0
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 <[email protected]>
+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 86076cb3f39cdca7dbf31c668b8aaafb44e42d39..e7108f9548e0f01863339b12f6bd5e51b951b3fc 100644
+--- a/third_party/blink/renderer/bindings/core/v8/window_proxy.cc
++++ b/third_party/blink/renderer/bindings/core/v8/window_proxy.cc
+@@ -109,10 +109,7 @@ v8::Local<v8::Object> WindowProxy::ReleaseGlobalProxy() {
+ }
+ 
+ void WindowProxy::SetGlobalProxy(v8::Local<v8::Object> 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
+@@ -123,6 +120,13 @@ void WindowProxy::SetGlobalProxy(v8::Local<v8::Object> global_proxy) {
+   Initialize();
+ }
+ 
++void WindowProxy::SetGlobalProxyWithoutInitializing(v8::Local<v8::Object> 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 777bb67e5f7a1eb42438f91023a53c82fd041af2..3735f9b49899b4d36f005a3c2b741e3791ba0709 100644
+--- a/third_party/blink/renderer/bindings/core/v8/window_proxy.h
++++ b/third_party/blink/renderer/bindings/core/v8/window_proxy.h
+@@ -157,6 +157,7 @@ class WindowProxy : public GarbageCollected<WindowProxy> {
+   CORE_EXPORT v8::Local<v8::Object> GlobalProxyIfNotDetached();
+   v8::Local<v8::Object> ReleaseGlobalProxy();
+   void SetGlobalProxy(v8::Local<v8::Object>);
++  void SetGlobalProxyWithoutInitializing(v8::Local<v8::Object>);
+ 
+   // 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 b8054889808b39f216bd85b019e2b05660c828de..5306426d84209c428a59fec6b468f57fbddd1fed 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)