chore: enable microtask queue per window agent (#36870)

* chore: enable microtask queue per window agent

* chore: switch policies on context microtask queue

* fix: ensure node::Environment is valid
This commit is contained in:
Robo 2023-01-12 01:59:32 +09:00 committed by GitHub
parent 2a7d0a84c0
commit fefb22a83d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 115 additions and 54 deletions

View file

@ -16,7 +16,6 @@
#include "media/base/media_switches.h"
#include "net/base/features.h"
#include "services/network/public/cpp/features.h"
#include "third_party/blink/renderer/platform/scheduler/common/features.h" // nogncheck
#if BUILDFLAG(IS_MAC)
#include "device/base/features.h" // nogncheck
@ -37,12 +36,6 @@ void InitializeFeatureList() {
disable_features +=
std::string(",") + features::kSpareRendererForSitePerProcess.name;
// Microtask queues per WindowAgent causes issues with Node, so disable
// this feature for now. See
// https://chromium-review.googlesource.com/c/chromium/src/+/4003663
disable_features +=
std::string(",") + blink::scheduler::kMicrotaskQueuePerWindowAgent.name;
#if BUILDFLAG(IS_MAC)
// Needed for WebUSB implementation
enable_features += std::string(",") + device::kNewUsbBackend.name;

View file

@ -251,9 +251,11 @@ void ElectronBindings::DidReceiveMemoryDump(
std::unique_ptr<memory_instrumentation::GlobalMemoryDump> global_dump) {
v8::Isolate* isolate = promise.isolate();
v8::HandleScope handle_scope(isolate);
gin_helper::MicrotasksScope microtasks_scope(isolate, true);
v8::Context::Scope context_scope(
v8::Local<v8::Context>::New(isolate, context));
v8::Local<v8::Context> local_context =
v8::Local<v8::Context>::New(isolate, context);
gin_helper::MicrotasksScope microtasks_scope(
isolate, local_context->GetMicrotaskQueue(), true);
v8::Context::Scope context_scope(local_context);
if (!success) {
promise.RejectWithErrorMessage("Failed to create memory dump");

View file

@ -48,9 +48,10 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
v8::EscapableHandleScope handle_scope(isolate);
if (!function.IsAlive())
return v8::Null(isolate);
gin_helper::MicrotasksScope microtasks_scope(isolate, true);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->GetCreationContextChecked();
gin_helper::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(), true);
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args{
gin::ConvertToV8(isolate, std::forward<ArgTypes>(raw))...};
@ -72,9 +73,10 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
v8::HandleScope handle_scope(isolate);
if (!function.IsAlive())
return;
gin_helper::MicrotasksScope microtasks_scope(isolate, true);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->GetCreationContextChecked();
gin_helper::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(), true);
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args{
gin::ConvertToV8(isolate, std::forward<ArgTypes>(raw))...};
@ -95,9 +97,10 @@ struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
ReturnType ret = ReturnType();
if (!function.IsAlive())
return ret;
gin_helper::MicrotasksScope microtasks_scope(isolate, true);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->GetCreationContextChecked();
gin_helper::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(), true);
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args{
gin::ConvertToV8(isolate, std::forward<ArgTypes>(raw))...};

View file

@ -15,7 +15,8 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
const char* method,
ValueVector* args) {
// Perform microtask checkpoint after running JavaScript.
gin_helper::MicrotasksScope microtasks_scope(isolate, true);
gin_helper::MicrotasksScope microtasks_scope(
isolate, obj->GetCreationContextChecked()->GetMicrotaskQueue(), true);
// Use node::MakeCallback to call the callback, and it will also run pending
// tasks in Node.js.
v8::MaybeLocal<v8::Value> ret = node::MakeCallback(

View file

@ -217,7 +217,9 @@ class Invoker<IndicesHolder<indices...>, ArgTypes...>
template <typename ReturnType>
void DispatchToCallback(
base::RepeatingCallback<ReturnType(ArgTypes...)> callback) {
gin_helper::MicrotasksScope microtasks_scope(args_->isolate(), true);
gin_helper::MicrotasksScope microtasks_scope(
args_->isolate(),
args_->GetHolderCreationContext()->GetMicrotaskQueue(), true);
args_->Return(
callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...));
}
@ -226,7 +228,9 @@ class Invoker<IndicesHolder<indices...>, ArgTypes...>
// expression to foo. As a result, we must specialize the case of Callbacks
// that have the void return type.
void DispatchToCallback(base::RepeatingCallback<void(ArgTypes...)> callback) {
gin_helper::MicrotasksScope microtasks_scope(args_->isolate(), true);
gin_helper::MicrotasksScope microtasks_scope(
args_->isolate(),
args_->GetHolderCreationContext()->GetMicrotaskQueue(), true);
callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...);
}

View file

@ -9,14 +9,15 @@
namespace gin_helper {
MicrotasksScope::MicrotasksScope(v8::Isolate* isolate,
v8::MicrotaskQueue* microtask_queue,
bool ignore_browser_checkpoint,
v8::MicrotasksScope::Type scope_type) {
if (Locker::IsBrowserProcess()) {
if (!ignore_browser_checkpoint)
v8::MicrotasksScope::PerformCheckpoint(isolate);
} else {
v8_microtasks_scope_ =
std::make_unique<v8::MicrotasksScope>(isolate, scope_type);
v8_microtasks_scope_ = std::make_unique<v8::MicrotasksScope>(
isolate, microtask_queue, scope_type);
}
}

View file

@ -16,6 +16,7 @@ namespace gin_helper {
class MicrotasksScope {
public:
explicit MicrotasksScope(v8::Isolate* isolate,
v8::MicrotaskQueue* microtask_queue,
bool ignore_browser_checkpoint = false,
v8::MicrotasksScope::Type scope_type =
v8::MicrotasksScope::kRunMicrotasks);

View file

@ -25,7 +25,8 @@ PromiseBase& PromiseBase::operator=(PromiseBase&&) = default;
v8::Maybe<bool> PromiseBase::Reject() {
v8::HandleScope handle_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(
isolate(), GetContext()->GetMicrotaskQueue());
v8::Context::Scope context_scope(GetContext());
return GetInner()->Reject(GetContext(), v8::Undefined(isolate()));
@ -33,7 +34,8 @@ v8::Maybe<bool> PromiseBase::Reject() {
v8::Maybe<bool> PromiseBase::Reject(v8::Local<v8::Value> except) {
v8::HandleScope handle_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(
isolate(), GetContext()->GetMicrotaskQueue());
v8::Context::Scope context_scope(GetContext());
return GetInner()->Reject(GetContext(), except);
@ -41,7 +43,8 @@ v8::Maybe<bool> PromiseBase::Reject(v8::Local<v8::Value> except) {
v8::Maybe<bool> PromiseBase::RejectWithErrorMessage(base::StringPiece message) {
v8::HandleScope handle_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(
isolate(), GetContext()->GetMicrotaskQueue());
v8::Context::Scope context_scope(GetContext());
v8::Local<v8::Value> error =
@ -83,7 +86,8 @@ v8::Local<v8::Promise> Promise<void>::ResolvedPromise(v8::Isolate* isolate) {
v8::Maybe<bool> Promise<void>::Resolve() {
v8::HandleScope handle_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(
isolate(), GetContext()->GetMicrotaskQueue());
v8::Context::Scope context_scope(GetContext());
return GetInner()->Resolve(GetContext(), v8::Undefined(isolate()));

View file

@ -118,7 +118,8 @@ class Promise : public PromiseBase {
v8::Maybe<bool> Resolve(const RT& value) {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(isolate());
gin_helper::MicrotasksScope microtasks_scope(
isolate(), GetContext()->GetMicrotaskQueue());
v8::Context::Scope context_scope(GetContext());
return GetInner()->Resolve(GetContext(),

View file

@ -192,11 +192,11 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
void ErrorMessageListener(v8::Local<v8::Message> message,
v8::Local<v8::Value> data) {
v8::Isolate* isolate = v8::Isolate::GetCurrent();
gin_helper::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
node::Environment* env = node::Environment::GetCurrent(isolate);
if (env) {
gin_helper::MicrotasksScope microtasks_scope(
isolate, env->context()->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
// Emit the after() hooks now that the exception has been handled.
// Analogous to node/lib/internal/process/execution.js#L176-L180
if (env->async_hooks()->fields()[node::AsyncHooks::kAfter]) {
@ -684,9 +684,10 @@ void NodeBindings::UvRunOnce() {
// checkpoints after every call into JavaScript. Since we use a different
// policy in the renderer - switch to `kExplicit` and then drop back to the
// previous policy value.
auto old_policy = env->isolate()->GetMicrotasksPolicy();
DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(env->isolate()), 0);
env->isolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
v8::MicrotaskQueue* microtask_queue = env->context()->GetMicrotaskQueue();
auto old_policy = microtask_queue->microtasks_policy();
DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0);
microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit);
if (browser_env_ != BrowserEnvironment::kBrowser)
TRACE_EVENT_BEGIN0("devtools.timeline", "FunctionCall");
@ -697,7 +698,7 @@ void NodeBindings::UvRunOnce() {
if (browser_env_ != BrowserEnvironment::kBrowser)
TRACE_EVENT_END0("devtools.timeline", "FunctionCall");
env->isolate()->SetMicrotasksPolicy(old_policy);
microtask_queue->set_microtasks_policy(old_policy);
if (r == 0)
base::RunLoop().QuitWhenIdle(); // Quit from uv.

View file

@ -34,7 +34,8 @@ class V8Serializer : public v8::ValueSerializer::Delegate {
bool Serialize(v8::Local<v8::Value> value, blink::CloneableMessage* out) {
gin_helper::MicrotasksScope microtasks_scope(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);
isolate_, isolate_->GetCurrentContext()->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
WriteBlinkEnvelope(19);
serializer_.WriteHeader();

View file

@ -217,14 +217,14 @@ void SpellCheckClient::SpellCheckWords(const SpellCheckScope& scope,
const std::set<std::u16string>& words) {
DCHECK(!scope.spell_check_.IsEmpty());
auto context = isolate_->GetCurrentContext();
gin_helper::MicrotasksScope microtasks_scope(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);
isolate_, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::FunctionTemplate> templ = gin_helper::CreateFunctionTemplate(
isolate_,
base::BindRepeating(&SpellCheckClient::OnSpellCheckDone, AsWeakPtr()));
auto context = isolate_->GetCurrentContext();
v8::Local<v8::Value> args[] = {gin::ConvertToV8(isolate_, words),
templ->GetFunction(context).ToLocalChecked()};
// Call javascript with the words and the callback function

View file

@ -87,7 +87,7 @@ void EmitIPCEvent(v8::Local<v8::Context> context,
v8::HandleScope handle_scope(isolate);
v8::Context::Scope context_scope(context);
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope script_scope(isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kRunMicrotasks);
std::vector<v8::Local<v8::Value>> argv = {

View file

@ -67,9 +67,10 @@ void ElectronRenderFrameObserver::DidClearWindowObject() {
!web_frame->IsOnInitialEmptyDocument()) {
v8::Isolate* isolate = blink::MainThreadIsolate();
v8::HandleScope handle_scope(isolate);
v8::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Handle<v8::Context> context = web_frame->MainWorldScriptContext();
v8::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Context::Scope context_scope(context);
// DidClearWindowObject only emits for the main world.
DidInstallConditionalFeatures(context, MAIN_WORLD_ID);
@ -112,7 +113,8 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
auto* isolate = context->GetIsolate();
v8::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
if (ShouldNotifyClient(world_id))
renderer_client_->DidCreateScriptContext(context, render_frame_);

View file

@ -138,10 +138,10 @@ void ElectronRendererClient::WillReleaseScriptContext(
// checkpoints after every call into JavaScript. Since we use a different
// policy in the renderer - switch to `kExplicit` and then drop back to the
// previous policy value.
v8::Isolate* isolate = context->GetIsolate();
auto old_policy = isolate->GetMicrotasksPolicy();
DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0);
isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
v8::MicrotaskQueue* microtask_queue = context->GetMicrotaskQueue();
auto old_policy = microtask_queue->microtasks_policy();
DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0);
microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit);
node::FreeEnvironment(env);
if (node_bindings_->uv_env() == nullptr) {
@ -149,7 +149,7 @@ void ElectronRendererClient::WillReleaseScriptContext(
node_bindings_->set_isolate_data(nullptr);
}
isolate->SetMicrotasksPolicy(old_policy);
microtask_queue->set_microtasks_policy(old_policy);
// ElectronBindings is tracking node environments.
electron_bindings_->EnvironmentDestroyed(env);

View file

@ -162,12 +162,12 @@ void ElectronSandboxedRendererClient::RunScriptsAtDocumentStart(
return;
auto* isolate = blink::MainThreadIsolate();
gin_helper::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context =
GetContext(render_frame->GetWebFrame(), isolate);
gin_helper::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Context::Scope context_scope(context);
InvokeHiddenCallback(context, kLifecycleKey, "onDocumentStart");
@ -180,12 +180,12 @@ void ElectronSandboxedRendererClient::RunScriptsAtDocumentEnd(
return;
auto* isolate = blink::MainThreadIsolate();
gin_helper::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context =
GetContext(render_frame->GetWebFrame(), isolate);
gin_helper::MicrotasksScope microtasks_scope(
isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Context::Scope context_scope(context);
InvokeHiddenCallback(context, kLifecycleKey, "onDocumentEnd");
@ -230,7 +230,8 @@ void ElectronSandboxedRendererClient::WillReleaseScriptContext(
auto* isolate = context->GetIsolate();
gin_helper::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::HandleScope handle_scope(isolate);
v8::Context::Scope context_scope(context);
InvokeHiddenCallback(context, kLifecycleKey, "onExit");

View file

@ -41,11 +41,14 @@ WebWorkerObserver::~WebWorkerObserver() {
// Node.js expects `kExplicit` microtasks policy and will run microtasks
// checkpoints after every call into JavaScript. Since we use a different
// policy in the renderer - switch to `kExplicit`
v8::Isolate* isolate = node_bindings_->uv_env()->isolate();
DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0);
isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
v8::MicrotaskQueue* microtask_queue =
node_bindings_->uv_env()->context()->GetMicrotaskQueue();
auto old_policy = microtask_queue->microtasks_policy();
DCHECK_EQ(microtask_queue->GetMicrotasksScopeDepth(), 0);
microtask_queue->set_microtasks_policy(v8::MicrotasksPolicy::kExplicit);
node::FreeEnvironment(node_bindings_->uv_env());
node::FreeIsolateData(node_bindings_->isolate_data());
microtask_queue->set_microtasks_policy(old_policy);
}
void WebWorkerObserver::WorkerScriptReadyForEvaluation(
@ -53,7 +56,8 @@ void WebWorkerObserver::WorkerScriptReadyForEvaluation(
v8::Context::Scope context_scope(worker_context);
auto* isolate = worker_context->GetIsolate();
v8::MicrotasksScope microtasks_scope(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
isolate, worker_context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
// Start the embed thread.
node_bindings_->PrepareEmbedThread();