From c07cbfed2f3969d2e94615599f6e9e8ce6069a3e Mon Sep 17 00:00:00 2001 From: Timothy Loh Date: Wed, 17 Apr 2024 07:20:55 +0000 Subject: [PATCH] Revert "Get WrapperTypeInfo via ScriptWrappable" This reverts commit 81b6e3de040c9675b2fad6e3b7d53989cb0e262e. Reason for revert: Breaks AutotestPrivateApiTest.AutotestPrivate on linux-chromeos-rel https://ci.chromium.org/ui/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel Sample failure: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-rel/75905/overview I think this is the relevant part of the stack trace: ../../content/public/test/no_renderer_crashes_assertion.cc:102: Failure Failed Unexpected termination of a renderer process; status: 3, exit_code: 139 Stack trace: #0 0x563e8b77c0da content::NoRendererCrashesAssertion::RenderProcessExited() #1 0x563e8967d8c4 content::RenderProcessHostImpl::ProcessDied() #2 0x563e8967d72c content::RenderProcessHostImpl::OnChannelError() #3 0x563e8ae3ecb5 base::TaskAnnotator::RunTaskImpl() #4 0x563e8ae583dd base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl() #5 0x563e8ae57e60 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() #6 0x563e8ae58845 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() #7 0x563e8aec2fef base::MessagePumpEpoll::Run() #8 0x563e8ae58bb2 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run() #9 0x563e8ae1e7fd base::RunLoop::Run() #10 0x563e91a0c6c9 extensions::ResultCatcher::GetNextResult() #11 0x563e8ad7fcf6 extensions::ExtensionApiTest::RunExtensionTest() #12 0x563e8ad7f999 extensions::ExtensionApiTest::RunExtensionTest() #13 0x563e854402ed extensions::AutotestPrivateApiTest::RunAutotestPrivateExtensionTest() Original change's description: > Get WrapperTypeInfo via ScriptWrappable > > as opposed to using a dedicated internal field for that. > > Bug: 328117814 > Change-Id: I01f9aff3ad8a41fafbd2655d23f076a0f76fdc57 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5455405 > Reviewed-by: Nate Chapin > Commit-Queue: Andrey Kosyakov > Cr-Commit-Position: refs/heads/main@{#1288405} Bug: 328117814 Change-Id: Id0ad5b6bcab7a99cf31d551df00928708dd93465 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5459075 Reviewed-by: Jiacheng Guo Auto-Submit: Timothy Loh Commit-Queue: Jiacheng Guo Owners-Override: Timothy Loh Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#1288546} --- .../modules/v8/v8_context_snapshot_impl.cc | 24 ++++--------------- .../platform/bindings/wrapper_type_info.cc | 13 +++++----- .../platform/bindings/wrapper_type_info.h | 3 +++ 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc b/third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc index 0e5881b79ddc9f..c923c0ab75cd16 100644 --- a/third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc +++ b/third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc @@ -271,22 +271,6 @@ void DeserializeInternalFieldCallback(v8::Local object, } } -namespace { -// We only care for WrapperTypeInfo and do not supply an actual instance of -// the document. Since we need a script wrappable to get type info now, this -// class is a minimal implementation of ScriptWrappable that returns correct -// type info for HTMLDocument. -class DummyHTMLDocumentForSnapshot : public ScriptWrappable { - public: - DummyHTMLDocumentForSnapshot() = default; - - private: - const WrapperTypeInfo* GetWrapperTypeInfo() const override { - return V8HTMLDocument::GetWrapperTypeInfo(); - } -}; -} // namespace - void TakeSnapshotForWorld(v8::SnapshotCreator* snapshot_creator, const DOMWrapperWorld& world) { v8::Isolate* isolate = snapshot_creator->GetIsolate(); @@ -316,9 +300,11 @@ void TakeSnapshotForWorld(v8::SnapshotCreator* snapshot_creator, v8::Local document_wrapper = CreatePlatformObject( isolate, context, world, document_wrapper_type_info); - V8DOMWrapper::SetNativeInfo( - isolate, document_wrapper, document_wrapper_type_info, - MakeGarbageCollected()); + int indices[] = {kV8DOMWrapperObjectIndex, kV8DOMWrapperTypeIndex}; + void* values[] = {nullptr, + const_cast(document_wrapper_type_info)}; + document_wrapper->SetAlignedPointerInInternalFields(std::size(indices), + indices, values); V8PrivateProperty::GetWindowDocumentCachedAccessor(isolate).Set( context->Global(), document_wrapper); diff --git a/third_party/blink/renderer/platform/bindings/wrapper_type_info.cc b/third_party/blink/renderer/platform/bindings/wrapper_type_info.cc index f618f00697f1fc..e823d598033a54 100644 --- a/third_party/blink/renderer/platform/bindings/wrapper_type_info.cc +++ b/third_party/blink/renderer/platform/bindings/wrapper_type_info.cc @@ -57,14 +57,13 @@ v8::Local WrapperTypeInfo::GetV8ClassTemplate( return v8_template; } +const WrapperTypeInfo* ToWrapperTypeInfo( + const v8::TracedReference& wrapper) { + return GetInternalField(wrapper); +} + const WrapperTypeInfo* ToWrapperTypeInfo(v8::Local wrapper) { - const auto* wrappable = ToScriptWrappable(wrapper->GetIsolate(), wrapper); - const WrapperTypeInfo* type_info = - wrappable ? wrappable->GetWrapperTypeInfo() : nullptr; - DCHECK_EQ( - type_info, - (GetInternalField(wrapper))); - return type_info; + return GetInternalField(wrapper); } } // namespace blink diff --git a/third_party/blink/renderer/platform/bindings/wrapper_type_info.h b/third_party/blink/renderer/platform/bindings/wrapper_type_info.h index a153137318feca..6cd34f0a2ace5f 100644 --- a/third_party/blink/renderer/platform/bindings/wrapper_type_info.h +++ b/third_party/blink/renderer/platform/bindings/wrapper_type_info.h @@ -206,6 +206,9 @@ inline ScriptWrappable* ToScriptWrappable(v8::Isolate* isolate, wrapper); } +PLATFORM_EXPORT const WrapperTypeInfo* ToWrapperTypeInfo( + const v8::TracedReference& wrapper); + PLATFORM_EXPORT const WrapperTypeInfo* ToWrapperTypeInfo( v8::Local wrapper);