From b67f643dc88f626c6380fc1d305202e1afe2cfd8 Mon Sep 17 00:00:00 2001 From: david <> Date: Fri, 5 Jul 2024 21:08:20 +0800 Subject: [PATCH] feat: support ScopedPersistent --- bridge/CMakeLists.txt | 1 - .../bindings/v8/platform/scoped_persistent.h | 4 +- bridge/bindings/v8/platform/script_state.cc | 87 ------ bridge/bindings/v8/platform/script_state.h | 293 ------------------ bridge/bindings/v8/script_value.cc | 30 +- bridge/bindings/v8/script_value.h | 2 +- bridge/bindings/v8/v8_initializer.cc | 2 +- bridge/core/dart_isolate_context.cc | 3 + bridge/core/executing_context.h | 4 + bridge/core/script_state.cc | 26 +- bridge/core/script_state.h | 10 +- 11 files changed, 54 insertions(+), 408 deletions(-) delete mode 100644 bridge/bindings/v8/platform/script_state.cc delete mode 100644 bridge/bindings/v8/platform/script_state.h diff --git a/bridge/CMakeLists.txt b/bridge/CMakeLists.txt index b6ad4822f0..8405858c22 100644 --- a/bridge/CMakeLists.txt +++ b/bridge/CMakeLists.txt @@ -575,7 +575,6 @@ elseif ($ENV{WEBF_JS_ENGINE} MATCHES "v8") bindings/v8/wrapper_type_info.cc bindings/v8/v8_interface_bridge_base.cc bindings/v8/v8_initializer.cc -# bindings/v8/platform/script_state.cc bindings/v8/platform/scoped_persistent.h bindings/v8/platform/v8_per_context_data.cc bindings/v8/platform/v8_per_isolate_data.cc diff --git a/bridge/bindings/v8/platform/scoped_persistent.h b/bridge/bindings/v8/platform/scoped_persistent.h index 22ca59fda6..6bb6c70d9d 100644 --- a/bridge/bindings/v8/platform/scoped_persistent.h +++ b/bridge/bindings/v8/platform/scoped_persistent.h @@ -28,7 +28,7 @@ class ScopedPersistent { ~ScopedPersistent() { Clear(); } - ALWAYS_INLINE v8::Local NewLocal(v8::Isolate* isolate) const { + inline v8::Local NewLocal(v8::Isolate* isolate) const { return v8::Local::New(isolate, handle_); } @@ -66,7 +66,7 @@ class ScopedPersistent { return handle_ == other; } - ALWAYS_INLINE v8::Persistent& Get() { return handle_; } + inline v8::Persistent& Get() { return handle_; } private: v8::Persistent handle_; diff --git a/bridge/bindings/v8/platform/script_state.cc b/bridge/bindings/v8/platform/script_state.cc deleted file mode 100644 index b67ecc72a4..0000000000 --- a/bridge/bindings/v8/platform/script_state.cc +++ /dev/null @@ -1,87 +0,0 @@ -/* -* Copyright (C) 2019-2022 The Kraken authors. All rights reserved. -* Copyright (C) 2022-present The WebF authors. All rights reserved. -*/ - -#include "bindings/v8/platform/script_state.h" -#include "bindings/v8/platform/heap/garbage_collected.h" -#include "bindings/v8/platform/v8_per_context_data.h" - -namespace webf { - -ScriptState::CreateCallback ScriptState::s_create_callback_ = nullptr; - -// static -void ScriptState::SetCreateCallback(CreateCallback create_callback) { - assert(create_callback); - assert(!s_create_callback_); - s_create_callback_ = create_callback; -} - -// static -ScriptState* ScriptState::Create(v8::Local context, - ExecutionContext* execution_context) { - return s_create_callback_(context, execution_context); -} - -ScriptState::ScriptState(v8::Local context, - ExecutionContext* execution_context) - : isolate_(context->GetIsolate()), - context_(isolate_, context), - per_context_data_(MakeGarbageCollected(context)) { - context_.SetWeak(this, &OnV8ContextCollectedCallback); - context->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex, this); -// RendererResourceCoordinator::Get()->OnScriptStateCreated(this, -// execution_context); -} - -ScriptState::~ScriptState() { - assert(!per_context_data_); - assert(context_.IsEmpty()); -// InstanceCounters::DecrementCounter( -// InstanceCounters::kDetachedScriptStateCounter); -// RendererResourceCoordinator::Get()->OnScriptStateDestroyed(this); -} - -void ScriptState::Trace(Visitor* visitor) const { - visitor->Trace(per_context_data_); -} - -void ScriptState::DetachGlobalObject() { - assert(!context_.IsEmpty()); - GetContext()->DetachGlobal(); -} - -void ScriptState::DisposePerContextData() { - per_context_data_->Dispose(); - per_context_data_ = nullptr; -// InstanceCounters::IncrementCounter( -// InstanceCounters::kDetachedScriptStateCounter); -// RendererResourceCoordinator::Get()->OnScriptStateDetached(this); -} - -void ScriptState::DissociateContext() { - assert(!per_context_data_); - - // On a worker thread we tear down V8's isolate without running a GC. - // Alternately we manually clear all references between V8 and Blink, and run - // operations that should have been invoked by weak callbacks if a GC were - // run. - - v8::HandleScope scope(GetIsolate()); - // Cut the reference from V8 context to ScriptState. - GetContext()->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex, - nullptr); - reference_from_v8_context_.Clear(); - - // Cut the reference from ScriptState to V8 context. - context_.Clear(); -} - -void ScriptState::OnV8ContextCollectedCallback( - const v8::WeakCallbackInfo& data) { - data.GetParameter()->reference_from_v8_context_.Clear(); - data.GetParameter()->context_.Clear(); -} - -} // namespace webf \ No newline at end of file diff --git a/bridge/bindings/v8/platform/script_state.h b/bridge/bindings/v8/platform/script_state.h deleted file mode 100644 index aa7cc7a064..0000000000 --- a/bridge/bindings/v8/platform/script_state.h +++ /dev/null @@ -1,293 +0,0 @@ -/* -* Copyright (C) 2019-2022 The Kraken authors. All rights reserved. -* Copyright (C) 2022-present The WebF authors. All rights reserved. -*/ - -#ifndef WEBF_SCRIPT_STATE_H -#define WEBF_SCRIPT_STATE_H - - -#include -#include -#include "bindings/v8/base/memory/stack_allocated.h" -#include "bindings/v8/platform/heap/garbage_collected.h" -#include "foundation/macros.h" -#include "bindings/v8/platform/scoped_persistent.h" -#include "bindings/v8/platform/heap/member.h" -#include "bindings/v8/platform/heap/self_keep_alive.h" -#include "foundation/macros.h" -#include "bindings/v8/gin/public/context_holder.h" -#include "bindings/v8/gin/public/gin_embedders.h" - -namespace webf { - -class ExecutionContext; -class ScriptValue; -class V8PerContextData; - -// ScriptState is an abstraction class that holds all information about script -// execution (e.g., v8::Isolate, v8::Context, DOMWrapperWorld, ExecutionContext -// etc). If you need any info about the script execution, you're expected to -// pass around ScriptState in the code base. ScriptState is in a 1:1 -// relationship with v8::Context. -// -// When you need ScriptState, you can add [CallWith=ScriptState] to IDL files -// and pass around ScriptState into a place where you need ScriptState. -// -// In some cases, you need ScriptState in code that doesn't have any JavaScript -// on the stack. Then you can store ScriptState on a C++ object using -// Member or Persistent. -// -// class SomeObject : public GarbageCollected { -// void someMethod(ScriptState* scriptState) { -// script_state_ = scriptState; // Record the ScriptState. -// ...; -// } -// -// void asynchronousMethod() { -// if (!script_state_->contextIsValid()) { -// // It's possible that the context is already gone. -// return; -// } -// // Enter the ScriptState. -// ScriptState::Scope scope(script_state_); -// // Do V8 related things. -// ToV8(...); -// } -// -// virtual void Trace(Visitor* visitor) const { -// visitor->Trace(script_state_); // ScriptState also needs to be traced. -// } -// -// Member script_state_; -// }; -// -// You should not store ScriptState on a C++ object that can be accessed -// by multiple worlds. For example, you can store ScriptState on -// ScriptPromiseResolverBase, ScriptValue etc because they can be accessed from -// one world. However, you cannot store ScriptState on a DOM object that has an -// IDL interface because the DOM object can be accessed from multiple worlds. If -// ScriptState of one world "leak"s to another world, you will end up with -// leaking any JavaScript objects from one Chrome extension to another Chrome -// extension, which is a severe security bug. -// -// Lifetime: -// ScriptState is created when v8::Context is created. -// ScriptState is destroyed when v8::Context is garbage-collected and -// all V8 proxy objects that have references to the ScriptState are destructed. -class ScriptState : public GarbageCollected { - public: - class Scope final { - WEBF_STACK_ALLOCATED(); - - public: - // You need to make sure that scriptState->context() is not empty before - // creating a Scope. - explicit Scope(ScriptState* script_state) - : handle_scope_(script_state->GetIsolate()), - context_(script_state->GetContext()) { - assert(script_state->ContextIsValid()); - context_->Enter(); - } - - ~Scope() { context_->Exit(); } - - private: - v8::HandleScope handle_scope_; - v8::Local context_; - }; - - // Use EscapableScope if you have to return a v8::Local to an outer scope. - // See v8::EscapableHandleScope. - class EscapableScope final { - WEBF_STACK_ALLOCATED(); - - public: - // You need to make sure that scriptState->context() is not empty before - // creating a Scope. - explicit EscapableScope(ScriptState* script_state) - : handle_scope_(script_state->GetIsolate()), - context_(script_state->GetContext()) { - assert(script_state->ContextIsValid()); - context_->Enter(); - } - - ~EscapableScope() { context_->Exit(); } - - v8::Local Escape(v8::Local value) { - return handle_scope_.Escape(value); - } - - private: - v8::EscapableHandleScope handle_scope_; - v8::Local context_; - }; - - static ScriptState* Create(v8::Local, - ExecutionContext*); - - ScriptState(const ScriptState&) = delete; - ScriptState& operator=(const ScriptState&) = delete; - virtual ~ScriptState(); - - virtual void Trace(Visitor*) const; - - static ScriptState* Current(v8::Isolate* isolate) { // DEPRECATED - return From(isolate->GetCurrentContext()); - } - - static ScriptState* ForCurrentRealm( - const v8::FunctionCallbackInfo& info) { - return From(info.GetIsolate()->GetCurrentContext()); - } - - static ScriptState* ForCurrentRealm( - const v8::PropertyCallbackInfo& info) { - return From(info.GetIsolate()->GetCurrentContext()); - } - - static ScriptState* ForRelevantRealm(v8::Local object) { - assert(!object.IsEmpty()); - ScriptState* script_state = static_cast( - object->GetAlignedPointerFromEmbedderDataInCreationContext( - kV8ContextPerContextDataIndex)); - // ScriptState::ForRelevantRealm() must be called only for objects having a - // creation context while the context must have a valid embedder data in - // the embedder field. -// SECURITY_CHECK(script_state); - return script_state; - } - - static ScriptState* From(v8::Local context) { - assert(!context.IsEmpty()); - ScriptState* script_state = - static_cast(context->GetAlignedPointerFromEmbedderData( - kV8ContextPerContextDataIndex)); - // ScriptState::From() must not be called for a context that does not have - // valid embedder data in the embedder field. -// SECURITY_CHECK(script_state); -// SECURITY_CHECK(script_state->context_ == context); - return script_state; - } - - // For use when it is not absolutely certain that the v8::Context is - // associated with a ScriptState. This is necessary in unit tests when a - // v8::Context is created directly on the v8 API without going through the - // usual blink codepaths. - // This is also called in some situations where DissociateContext() has - // already been called and therefore the ScriptState pointer on the - // v8::Context has already been nulled. - static ScriptState* MaybeFrom(v8::Local context) { - assert(!context.IsEmpty()); - if (context->GetNumberOfEmbedderDataFields() <= - kV8ContextPerContextDataIndex) { - return nullptr; - } - ScriptState* script_state = - static_cast(context->GetAlignedPointerFromEmbedderData( - kV8ContextPerContextDataIndex)); - assert(!script_state || script_state->context_ == context); - return script_state; - } - - v8::Isolate* GetIsolate() const { return isolate_; } -// DOMWrapperWorld& World() const { return *world_; } -// const V8ContextToken& GetToken() const { return token_; } - - // This can return an empty handle if the v8::Context is gone. - v8::Local GetContext() const { - return context_.NewLocal(isolate_); - } - bool ContextIsValid() const { - return !context_.IsEmpty() && per_context_data_; - } - void DetachGlobalObject(); - - V8PerContextData* PerContextData() const { return per_context_data_.Get(); } - void DisposePerContextData(); - - // This method is expected to be called only from - // WorkerOrWorkletScriptController to run operations that should have been - // invoked by a weak callback if a V8 GC were run, in a worker thread - // termination. - void DissociateContext(); - - protected: - ScriptState(v8::Local, ExecutionContext*); - - private: - static void OnV8ContextCollectedCallback( - const v8::WeakCallbackInfo&); - - v8::Isolate* isolate_; - // This persistent handle is weak. - ScopedPersistent context_; - - // This refptr doesn't cause a cycle because all persistent handles that - // DOMWrapperWorld holds are weak. -// Member world_; - - Member per_context_data_; - - // v8::Context has an internal field to this ScriptState* as a raw pointer, - // which is out of scope of Blink GC, but it must be a strong reference. We - // use |reference_from_v8_context_| to represent this strong reference. The - // lifetime of |reference_from_v8_context_| and the internal field must match - // exactly. - SelfKeepAlive reference_from_v8_context_{this}; - - // Serves as a unique ID for this context, which can be used to name the - // context in browser/renderer communications. -// V8ContextToken token_; - - using CreateCallback = ScriptState* (*)(v8::Local, - ExecutionContext*); - static CreateCallback s_create_callback_; - static void SetCreateCallback(CreateCallback); - friend class ScriptStateImpl; - - static constexpr int kV8ContextPerContextDataIndex = - static_cast(gin::kPerContextDataStartIndex) + - static_cast(gin::kEmbedderWebf); -}; - -// ScriptStateProtectingContext keeps the context associated with the -// ScriptState alive. You need to call Clear() once you no longer need the -// context. Otherwise, the context will leak. -class ScriptStateProtectingContext final - : public GarbageCollected { - public: - explicit ScriptStateProtectingContext(ScriptState* script_state) - : script_state_(script_state) { - if (script_state_) { - context_.Set(script_state_->GetIsolate(), script_state_->GetContext()); - context_.Get().AnnotateStrongRetainer( - "blink::ScriptStateProtectingContext::context_"); - } - } - ScriptStateProtectingContext(const ScriptStateProtectingContext&) = delete; - ScriptStateProtectingContext& operator=(const ScriptStateProtectingContext&) = - delete; - - void Trace(Visitor* visitor) const { visitor->Trace(script_state_); } - - ScriptState* Get() const { return script_state_.Get(); } - void Reset() { - script_state_ = nullptr; - context_.Clear(); - } - - // ScriptState like interface - bool ContextIsValid() const { return script_state_->ContextIsValid(); } - v8::Isolate* GetIsolate() const { return script_state_->GetIsolate(); } - v8::Local GetContext() const { - return script_state_->GetContext(); - } - - private: - Member script_state_; - ScopedPersistent context_; -}; -} // namespace webf - -#endif // WEBF_SCRIPT_STATE_H \ No newline at end of file diff --git a/bridge/bindings/v8/script_value.cc b/bridge/bindings/v8/script_value.cc index a09aea2304..00439e2bd5 100644 --- a/bridge/bindings/v8/script_value.cc +++ b/bridge/bindings/v8/script_value.cc @@ -9,23 +9,27 @@ namespace webf { v8::Local ScriptValue::V8Value() const { - if (IsEmpty()) - return v8::Local(); - - assert_m(GetIsolate()->InContext(), "The v8::Isolate is not in a valid context."); - assert_m(!v8_reference_.IsEmpty(), "The v8::TracedReference is empty"); - auto scriptState = ScriptState::From(isolate_->GetCurrentContext()); - return v8_reference_.Get(scriptState->GetIsolate()); + return v8::Local(); + +// if (IsEmpty()) +// return v8::Local(); +// +// assert_m(GetIsolate()->InContext(), "The v8::Isolate is not in a valid context."); +// assert_m(!v8_reference_.IsEmpty(), "The v8::TracedReference is empty"); +// auto scriptState = ScriptState::From(isolate_->GetCurrentContext()); +// return v8_reference_.Get(scriptState->GetIsolate()); } v8::Local ScriptValue::V8ValueFor( ScriptState* target_script_state) const { - if (IsEmpty()) - return v8::Local(); - - assert_m(!v8_reference_.IsEmpty(), "The v8::TracedReference is empty"); - v8::Isolate* isolate = target_script_state->GetIsolate(); - return v8_reference_.Get(isolate); + return v8::Local(); + +// if (IsEmpty()) +// return v8::Local(); +// +// assert_m(!v8_reference_.IsEmpty(), "The v8::TracedReference is empty"); +// v8::Isolate* isolate = target_script_state->GetIsolate(); +// return v8_reference_.Get(isolate); } bool ScriptValue::ToString(AtomicString& result) const { diff --git a/bridge/bindings/v8/script_value.h b/bridge/bindings/v8/script_value.h index 32d2062734..d85093496e 100644 --- a/bridge/bindings/v8/script_value.h +++ b/bridge/bindings/v8/script_value.h @@ -13,7 +13,7 @@ #include "dictionary_base.h" #include "foundation/macros.h" #include "foundation/native_string.h" -#include "platform/script_state.h" +//#include "platform/script_state.h" #include "union_base.h" //#include "bindings/v8/platform/wtf/vector_traits.h" #include "foundation/macros.h" diff --git a/bridge/bindings/v8/v8_initializer.cc b/bridge/bindings/v8/v8_initializer.cc index 237681fc32..ef0f7f0a79 100644 --- a/bridge/bindings/v8/v8_initializer.cc +++ b/bridge/bindings/v8/v8_initializer.cc @@ -15,7 +15,7 @@ #include #include "bindings/v8/platform/util/main_thread_util.h" #include -#include "bindings/v8/platform/script_state.h" +//#include "bindings/v8/platform/script_state.h" #include "bindings/v8/platform/v8_per_isolate_data.h" #if defined(V8_USE_EXTERNAL_STARTUP_DATA) diff --git a/bridge/core/dart_isolate_context.cc b/bridge/core/dart_isolate_context.cc index af1805fdce..45e445e2d3 100644 --- a/bridge/core/dart_isolate_context.cc +++ b/bridge/core/dart_isolate_context.cc @@ -178,6 +178,9 @@ void DartIsolateContext::InitializeNewPageInJSThread(PageGroup* page_group, AllocateNewPageCallback result_callback) { // dart_isolate_context->profiler()->StartTrackInitialize(); DartIsolateContext::InitializeJSRuntime(); + + v8::HandleScope handle_scope(dart_isolate_context->isolate()); + auto* page = new WebFPage(dart_isolate_context, true, sync_buffer_size, page_context_id, nullptr); // dart_isolate_context->profiler()->FinishTrackInitialize(); diff --git a/bridge/core/executing_context.h b/bridge/core/executing_context.h index 5343a3f0e3..bd617f32aa 100644 --- a/bridge/core/executing_context.h +++ b/bridge/core/executing_context.h @@ -204,7 +204,11 @@ class ExecutingContext { // ---------------------------------------------------------------------- // All members above ScriptState will be freed after ScriptState freed // ---------------------------------------------------------------------- +#if WEBF_QUICKJS_JS_ENGINE ScriptState script_state_{dart_isolate_context_}; +#elif WEBF_V8_JS_ENGINE + ScriptState script_state_{dart_isolate_context_, v8::Context::New(dart_isolate_context_->isolate())}; +#endif // ---------------------------------------------------------------------- // All members below will be free before ScriptState freed. // ---------------------------------------------------------------------- diff --git a/bridge/core/script_state.cc b/bridge/core/script_state.cc index db747a0de8..7b658919fa 100644 --- a/bridge/core/script_state.cc +++ b/bridge/core/script_state.cc @@ -6,32 +6,44 @@ //#include "event_factory.h" #include "html_element_factory.h" //#include "names_installer.h" -#include "dart_isolate_context.h" namespace webf { thread_local std::atomic runningContexts{0}; +#if WEBF_QUICKJS_JS_ENGINE ScriptState::ScriptState(DartIsolateContext* dart_context) : dart_isolate_context_(dart_context) { runningContexts++; -#if WEBF_QUICKJS_JS_ENGINE // Avoid stack overflow when running in multiple threads. ctx_ = JS_NewContext(dart_isolate_context_->runtime()); InitializeBuiltInStrings(ctx_); -#elif WEBF_V8_JS_ENGINE - ctx_ = v8::Context::New(dart_context->isolate()); - InitializeBuiltInStrings(dart_context->isolate()); -#endif } -#if WEBF_QUICKJS_JS_ENGINE JSRuntime* ScriptState::runtime() { return dart_isolate_context_->runtime(); } #elif WEBF_V8_JS_ENGINE +ScriptState::ScriptState(DartIsolateContext* dart_context, + v8::Local context) + : dart_isolate_context_(dart_context), + context_(dart_context->isolate(), context) { + runningContexts++; + // Avoid stack overflow when running in multiple threads. + +// context_.SetWeak(this, &OnV8ContextCollectedCallback); +// context->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex, this); +} + v8::Isolate* ScriptState::isolate() { return dart_isolate_context_->isolate(); } + +//void ScriptState::OnV8ContextCollectedCallback( +// const v8::WeakCallbackInfo& data) { +//// data.GetParameter()->reference_from_v8_context_.Clear(); +// data.GetParameter()->context_.Clear(); +//} + #endif #if WEBF_QUICKJS_JS_ENGINE diff --git a/bridge/core/script_state.h b/bridge/core/script_state.h index 9820909eb7..a782983c5c 100644 --- a/bridge/core/script_state.h +++ b/bridge/core/script_state.h @@ -7,10 +7,12 @@ #if WEBF_V8_JS_ENGINE #include +#include "bindings/v8/platform/scoped_persistent.h" #elif WEBF_QUICKJS_JS_ENGINE #include #endif +#include "dart_isolate_context.h" #include namespace webf { @@ -24,19 +26,21 @@ class DartIsolateContext; class ScriptState { public: ScriptState() = delete; - ScriptState(DartIsolateContext* dart_context); ~ScriptState(); inline bool Invalid() const { return !ctx_invalid_; } #if WEBF_QUICKJS_JS_ENGINE + ScriptState(DartIsolateContext* dart_context); inline JSContext* ctx() { assert(!ctx_invalid_ && "executingContext has been released"); return ctx_; } #elif WEBF_V8_JS_ENGINE + ScriptState(DartIsolateContext* dart_context, v8::Local context); inline v8::Local ctx() { assert(!ctx_invalid_ && "executingContext has been released"); - return ctx_; + v8::Isolate* isolate = dart_isolate_context_->isolate(); + return context_.NewLocal(isolate); } v8::Isolate* isolate(); #endif @@ -45,7 +49,7 @@ class ScriptState { #if WEBF_QUICKJS_JS_ENGINE JSContext* ctx_{nullptr}; #elif WEBF_V8_JS_ENGINE - v8::Local ctx_; + ScopedPersistent context_; #endif bool ctx_invalid_{false};