chore: [28-x-y] cherry-pick 1 changes from 0-M124

* 0d9350b71fd0 from chromium
electron · Apr 28, 2024 · de617ff · de617ff
1 parent 1768c3b
commit de617ff
Show file tree

Hide file tree

Showing 2 changed files with 204 additions and 0 deletions.
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
@@ -152,3 +152,4 @@ m122_webcodecs_disable_async_videoframe_readback_to_mitigate_a.patch
 fix_paintimage_deserialization_arbitrary-read_issue.patch
 reland_sensors_winrt_call_onreadingchangedcallback_via.patch
 cherry-pick-1b1f34234346.patch
+cherry-pick-0d9350b71fd0.patch
diff --git a/patches/chromium/cherry-pick-0d9350b71fd0.patch b/patches/chromium/cherry-pick-0d9350b71fd0.patch
@@ -0,0 +1,203 @@
+From 0d9350b71fd0bffe5052342850cf591958322924 Mon Sep 17 00:00:00 2001
+From: Marja Hölttä <marja@google.com>
+Date: Tue, 09 Apr 2024 08:32:27 +0000
+Subject: [PATCH] Merge "Fix DOMArrayBuffer::IsDetached()" and "Comment out a CHECK that a DOMAB has maximally one non-detached JSAB"
+
+1)
+
+A DOMArrayBuffer was maintaining its own "is_detached_" state, and
+would consider itself non-detached even if the corresponding
+JSArrayBuffer (or, all of them, in case there are several) was
+detached.
+
+Piping in the v8::Isolate would be a too big change for this fix, so this is using v8::Isolate::GetCurrent() for now.
+
+2)
+
+Comment out a CHECK that a DOMAB has maximally one non-detached JSAB
+
+Based on crash reports, this assumption is not true and has to be
+investigated.
+
+Removing this newly introduced CHECK to be able to merge fixes in this
+area - we still violate this invariant but the fixes are a step into
+the right direction.
+
+Fix in question:
+https://chromium-review.googlesource.com/5387887
+which also introduced this CHECK.
+
+(cherry picked from commit 04e7550d7aa3bf4ac4e49d7074972d357de139e6)
+
+Change-Id: I6a46721e24c6f04fe8252bc4a5e94caeec3a8b51
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5435035
+Commit-Queue: Marja Hölttä <marja@chromium.org>
+Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
+Cr-Commit-Position: refs/branch-heads/6367@{#667}
+Cr-Branched-From: d158c6dc6e3604e6f899041972edf26087a49740-refs/heads/main@{#1274542}
+---
+
+diff --git a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
+index 87db88b..c178ce5 100644
+--- a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
++++ b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
+@@ -46,8 +46,19 @@
+
+ static void AccumulateArrayBuffersForAllWorlds(
+     v8::Isolate* isolate,
+-    DOMArrayBuffer* object,
++    const DOMArrayBuffer* object,
+     v8::LocalVector<v8::ArrayBuffer>& buffers) {
++  if (!object->has_non_main_world_wrappers() && IsMainThread()) {
++    const DOMWrapperWorld& world = DOMWrapperWorld::MainWorld(isolate);
++    v8::Local<v8::Object> wrapper;
++    if (world.DomDataStore()
++            .Get</*entered_context=*/false>(isolate, object)
++            .ToLocal(&wrapper)) {
++      buffers.push_back(v8::Local<v8::ArrayBuffer>::Cast(wrapper));
++    }
++    return;
++  }
++
+   HeapVector<Member<DOMWrapperWorld>> worlds;
+   DOMWrapperWorld::AllWorldsInIsolate(isolate, worlds);
+   for (const auto& world : worlds) {
+@@ -259,6 +270,52 @@
+                               wrapper);
+ }
+
++bool DOMArrayBuffer::IsDetached() const {
++  if (contents_.BackingStore() == nullptr) {
++    return is_detached_;
++  }
++  if (is_detached_) {
++    return true;
++  }
++
++  v8::Isolate* isolate = v8::Isolate::GetCurrent();
++  v8::HandleScope handle_scope(isolate);
++  v8::LocalVector<v8::ArrayBuffer> buffer_handles(isolate);
++  AccumulateArrayBuffersForAllWorlds(isolate, this, buffer_handles);
++
++  // There may be several v8::ArrayBuffers corresponding to the DOMArrayBuffer,
++  // but at most one of them may be non-detached.
++  int nondetached_count = 0;
++  int detached_count = 0;
++
++  for (const auto& buffer_handle : buffer_handles) {
++    if (buffer_handle->WasDetached()) {
++      ++detached_count;
++    } else {
++      ++nondetached_count;
++    }
++  }
++
++  // This CHECK fires even though it should not. TODO(330759272): Investigate
++  // under which conditions we end up with multiple non-detached JSABs for the
++  // same DOMAB and potentially restore this check.
++
++  // CHECK_LE(nondetached_count, 1);
++
++  return nondetached_count == 0 && detached_count > 0;
++}
++
++v8::Local<v8::Object> DOMArrayBuffer::AssociateWithWrapper(
++    v8::Isolate* isolate,
++    const WrapperTypeInfo* wrapper_type_info,
++    v8::Local<v8::Object> wrapper) {
++  if (!DOMWrapperWorld::Current(isolate).IsMainWorld()) {
++    has_non_main_world_wrappers_ = true;
++  }
++  return ScriptWrappable::AssociateWithWrapper(isolate, wrapper_type_info,
++                                               wrapper);
++}
++
+ DOMArrayBuffer* DOMArrayBuffer::Slice(size_t begin, size_t end) const {
+   begin = std::min(begin, ByteLength());
+   end = std::min(end, ByteLength());
+diff --git a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
+index a7d4bac..9d4827f5 100644
+--- a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
++++ b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
+@@ -91,6 +91,17 @@
+
+   void Trace(Visitor*) const override;
+
++  bool IsDetached() const override;
++
++  v8::Local<v8::Object> AssociateWithWrapper(
++      v8::Isolate* isolate,
++      const WrapperTypeInfo* wrapper_type_info,
++      v8::Local<v8::Object> wrapper) override;
++
++  bool has_non_main_world_wrappers() const {
++    return has_non_main_world_wrappers_;
++  }
++
+  private:
+   v8::Maybe<bool> TransferDetachable(v8::Isolate*,
+                                      v8::Local<v8::Value> detach_key,
+@@ -101,6 +112,8 @@
+   // support only v8::String as the detach key type. It's also convenient that
+   // we can write `array_buffer->SetDetachKey(isolate, "my key")`.
+   TraceWrapperV8Reference<v8::String> detach_key_;
++
++  bool has_non_main_world_wrappers_ = false;
+ };
+
+ }  // namespace blink
+diff --git a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
+index 90c4c70..43618b8 100644
+--- a/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
++++ b/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
+@@ -27,7 +27,9 @@
+
+   size_t ByteLength() const { return contents_.DataLength(); }
+
+-  bool IsDetached() const { return is_detached_; }
++  // TODO(331348222): It doesn't make sense to detach DomSharedArrayBuffers,
++  // remove that possibility.
++  virtual bool IsDetached() const { return is_detached_; }
+
+   void Detach() { is_detached_ = true; }
+
+diff --git a/third_party/blink/renderer/modules/gamepad/BUILD.gn b/third_party/blink/renderer/modules/gamepad/BUILD.gn
+index 572d8ce..a871cbd 100644
+--- a/third_party/blink/renderer/modules/gamepad/BUILD.gn
++++ b/third_party/blink/renderer/modules/gamepad/BUILD.gn
+@@ -55,6 +55,7 @@
+     "//testing/gtest",
+     "//third_party/blink/renderer/modules",
+     "//third_party/blink/renderer/platform",
++    "//third_party/blink/renderer/platform:test_support",
+     "//third_party/blink/renderer/platform/wtf",
+   ]
+ }
+diff --git a/third_party/blink/renderer/modules/gamepad/gamepad_comparisons_test.cc b/third_party/blink/renderer/modules/gamepad/gamepad_comparisons_test.cc
+index e0a7f48..b9f422e 100644
+--- a/third_party/blink/renderer/modules/gamepad/gamepad_comparisons_test.cc
++++ b/third_party/blink/renderer/modules/gamepad/gamepad_comparisons_test.cc
+@@ -4,9 +4,11 @@
+
+ #include "third_party/blink/renderer/modules/gamepad/gamepad_comparisons.h"
+
++#include "base/test/task_environment.h"
+ #include "device/gamepad/public/cpp/gamepad.h"
+ #include "testing/gtest/include/gtest/gtest.h"
+ #include "third_party/blink/renderer/modules/gamepad/gamepad.h"
++#include "third_party/blink/renderer/platform/testing/main_thread_isolate.h"
+
+ namespace blink {
+
+@@ -241,6 +243,11 @@
+     list[0] = gamepad;
+     return list;
+   }
++
++ private:
++  // Needed so we can do v8::Isolate::GetCurrent().
++  base::test::TaskEnvironment task_environment_;
++  blink::test::MainThreadIsolate isolate_;
+ };
+
+ TEST_F(GamepadComparisonsTest, EmptyListCausesNoActivation) {