Skip to content

Commit

Permalink
Revert "Get WrapperTypeInfo via ScriptWrappable"
Browse files Browse the repository at this point in the history
This reverts commit 81b6e3d.

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 <japhet@chromium.org>
> Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
> 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 <gjc@google.com>
Auto-Submit: Timothy Loh <timloh@chromium.org>
Commit-Queue: Jiacheng Guo <gjc@google.com>
Owners-Override: Timothy Loh <timloh@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1288546}
  • Loading branch information
tim-loh authored and Chromium LUCI CQ committed Apr 17, 2024
1 parent fe114af commit c07cbfe
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 26 deletions.
Expand Up @@ -271,22 +271,6 @@ void DeserializeInternalFieldCallback(v8::Local<v8::Object> 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();
Expand Down Expand Up @@ -316,9 +300,11 @@ void TakeSnapshotForWorld(v8::SnapshotCreator* snapshot_creator,
v8::Local<v8::Object> document_wrapper = CreatePlatformObject(
isolate, context, world, document_wrapper_type_info);

V8DOMWrapper::SetNativeInfo(
isolate, document_wrapper, document_wrapper_type_info,
MakeGarbageCollected<DummyHTMLDocumentForSnapshot>());
int indices[] = {kV8DOMWrapperObjectIndex, kV8DOMWrapperTypeIndex};
void* values[] = {nullptr,
const_cast<WrapperTypeInfo*>(document_wrapper_type_info)};
document_wrapper->SetAlignedPointerInInternalFields(std::size(indices),
indices, values);

V8PrivateProperty::GetWindowDocumentCachedAccessor(isolate).Set(
context->Global(), document_wrapper);
Expand Down
Expand Up @@ -57,14 +57,13 @@ v8::Local<v8::Template> WrapperTypeInfo::GetV8ClassTemplate(
return v8_template;
}

const WrapperTypeInfo* ToWrapperTypeInfo(
const v8::TracedReference<v8::Object>& wrapper) {
return GetInternalField<WrapperTypeInfo, kV8DOMWrapperTypeIndex>(wrapper);
}

const WrapperTypeInfo* ToWrapperTypeInfo(v8::Local<v8::Object> wrapper) {
const auto* wrappable = ToScriptWrappable(wrapper->GetIsolate(), wrapper);
const WrapperTypeInfo* type_info =
wrappable ? wrappable->GetWrapperTypeInfo() : nullptr;
DCHECK_EQ(
type_info,
(GetInternalField<WrapperTypeInfo, kV8DOMWrapperTypeIndex>(wrapper)));
return type_info;
return GetInternalField<WrapperTypeInfo, kV8DOMWrapperTypeIndex>(wrapper);
}

} // namespace blink
Expand Up @@ -206,6 +206,9 @@ inline ScriptWrappable* ToScriptWrappable(v8::Isolate* isolate,
wrapper);
}

PLATFORM_EXPORT const WrapperTypeInfo* ToWrapperTypeInfo(
const v8::TracedReference<v8::Object>& wrapper);

PLATFORM_EXPORT const WrapperTypeInfo* ToWrapperTypeInfo(
v8::Local<v8::Object> wrapper);

Expand Down

0 comments on commit c07cbfe

Please sign in to comment.