From 223efdc6ae457745f720f37d5125dd3fc207441b Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Sun, 28 Apr 2024 01:24:27 +0100 Subject: [PATCH] chore: [29-x-y] cherry-pick 1 changes from 0-M124 * 0d9350b71fd0 from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-0d9350b71fd0.patch | 217 ++++++++++++++++++ 2 files changed, 218 insertions(+) create mode 100644 patches/chromium/cherry-pick-0d9350b71fd0.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 8aec0f1cf10f9..9399abf46d772 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -134,3 +134,4 @@ fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch remove_dxdiag_telemetry_code.patch cherry-pick-2607ddacd643.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 new file mode 100644 index 0000000000000..7f72389bd48dd --- /dev/null +++ b/patches/chromium/cherry-pick-0d9350b71fd0.patch @@ -0,0 +1,217 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= +Date: Tue, 9 Apr 2024 08:32:27 +0000 +Subject: Merge "Fix DOMArrayBuffer::IsDetached()" and "Comment out a CHECK + that a DOMAB has maximally one non-detached JSAB" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +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ä +Reviewed-by: Michael Lippautz +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 69060a1e5f6937896d385b5ceb70f2d49ef8669c..44898805c8c43f1f09a193e3533a6efb71a97462 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,17 @@ const WrapperTypeInfo& DOMArrayBuffer::wrapper_type_info_ = + + static void AccumulateArrayBuffersForAllWorlds( + v8::Isolate* isolate, +- DOMArrayBuffer* object, ++ const DOMArrayBuffer* object, + v8::LocalVector& buffers) { ++ if (!object->has_non_main_world_wrappers() && IsMainThread()) { ++ const DOMWrapperWorld& world = DOMWrapperWorld::MainWorld(isolate); ++ v8::Local wrapper = world.DomDataStore() ++ .Get(object, isolate); ++ buffers.push_back(v8::Local::Cast(wrapper)); ++ ++ return; ++ } ++ + Vector> worlds; + DOMWrapperWorld::AllWorldsInIsolate(isolate, worlds); + for (const auto& world : worlds) { +@@ -256,6 +265,52 @@ v8::Local DOMArrayBuffer::Wrap(ScriptState* script_state) { + 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 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 DOMArrayBuffer::AssociateWithWrapper( ++ v8::Isolate* isolate, ++ const WrapperTypeInfo* wrapper_type_info, ++ v8::Local 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 a7d4bac99e608bcfaa02dc9bfcef20b5a29d0ddc..9d4827f548ca7db3be85011c68d8346fc8dfa909 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 @@ class CORE_EXPORT DOMArrayBuffer : public DOMArrayBufferBase { + + void Trace(Visitor*) const override; + ++ bool IsDetached() const override; ++ ++ v8::Local AssociateWithWrapper( ++ v8::Isolate* isolate, ++ const WrapperTypeInfo* wrapper_type_info, ++ v8::Local wrapper) override; ++ ++ bool has_non_main_world_wrappers() const { ++ return has_non_main_world_wrappers_; ++ } ++ + private: + v8::Maybe TransferDetachable(v8::Isolate*, + v8::Local detach_key, +@@ -101,6 +112,8 @@ class CORE_EXPORT DOMArrayBuffer : public DOMArrayBufferBase { + // support only v8::String as the detach key type. It's also convenient that + // we can write `array_buffer->SetDetachKey(isolate, "my key")`. + TraceWrapperV8Reference 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 90c4c70755babdc8c88a7c6bf02803c5858c2194..43618b8ef4b831678b45c72ca47f5729c4f2aaef 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 @@ class CORE_EXPORT DOMArrayBufferBase : public ScriptWrappable { + + 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 572d8ce27fa808707ae17ed05f14e2ed103f131e..a871cbd002795bf49ad48f0002ac4996135f8b10 100644 +--- a/third_party/blink/renderer/modules/gamepad/BUILD.gn ++++ b/third_party/blink/renderer/modules/gamepad/BUILD.gn +@@ -55,6 +55,7 @@ source_set("unit_tests") { + "//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 e0a7f48630ba423b19641232c026d72ba71dfc4b..b9f422e6fff36da62575d803f132573a24d03c05 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 @@ class GamepadComparisonsTest : public testing::Test { + 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) { +diff --git a/third_party/blink/renderer/platform/bindings/dom_data_store.h b/third_party/blink/renderer/platform/bindings/dom_data_store.h +index db1c7e1592b01ed6b29a607a598b810b81aadde4..f95ccb80b08ca72d160c1169e5d10f09ac0fed50 100644 +--- a/third_party/blink/renderer/platform/bindings/dom_data_store.h ++++ b/third_party/blink/renderer/platform/bindings/dom_data_store.h +@@ -114,7 +114,7 @@ class DOMDataStore final : public GarbageCollected { + // Clears all references. + void Dispose(); + +- v8::Local Get(ScriptWrappable* object, v8::Isolate* isolate) { ++ v8::Local Get(const ScriptWrappable* object, v8::Isolate* isolate) { + if (is_main_world_) + return object->MainWorldWrapper(isolate); + auto it = wrapper_map_.find(object);