Skip to content

Commit

Permalink
fix: don't specify v8::MicrotasksScope for explicit policy
Browse files Browse the repository at this point in the history
  • Loading branch information
deepak1556 committed Jun 16, 2020
1 parent 38669f8 commit 9b6ef9a
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 32 deletions.
2 changes: 2 additions & 0 deletions filenames.gni
Expand Up @@ -525,6 +525,8 @@ filenames = {
"shell/common/gin_helper/function_template_extensions.h",
"shell/common/gin_helper/locker.cc",
"shell/common/gin_helper/locker.h",
"shell/common/gin_helper/microtasks_scope.cc",
"shell/common/gin_helper/microtasks_scope.h",
"shell/common/gin_helper/object_template_builder.cc",
"shell/common/gin_helper/object_template_builder.h",
"shell/common/gin_helper/persistent_dictionary.cc",
Expand Down
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_url_loader.cc
Expand Up @@ -28,6 +28,7 @@
#include "shell/common/gin_converters/gurl_converter.h"
#include "shell/common/gin_converters/net_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/microtasks_scope.h"
#include "shell/common/gin_helper/object_template_builder.h"
#include "shell/common/node_includes.h"

Expand Down Expand Up @@ -135,8 +136,7 @@ class JSChunkedDataPipeGetter : public gin::Wrappable<JSChunkedDataPipeGetter>,
data_producer_ = std::make_unique<mojo::DataPipeProducer>(std::move(pipe));

v8::HandleScope handle_scope(isolate_);
v8::MicrotasksScope script_scope(isolate_,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate_);
auto maybe_wrapper = GetWrapper(isolate_);
v8::Local<v8::Value> wrapper;
if (!maybe_wrapper.ToLocal(&wrapper)) {
Expand Down
10 changes: 6 additions & 4 deletions shell/browser/microtasks_runner.cc
Expand Up @@ -28,10 +28,12 @@ void MicrotasksRunner::DidProcessTask(const base::PendingTask& pending_task) {
// contention for performing checkpoint between Node.js and chromium, ending
// up Node.js dealying its callbacks. To fix this, now we always lets Node.js
// handle the checkpoint in the browser process.
auto* node_env = electron::ElectronBrowserMainParts::Get()->node_env();
node::InternalCallbackScope microtask_scope(
node_env->env(), v8::Local<v8::Object>(), {0, 0},
node::InternalCallbackScope::kAllowEmptyResource);
{
auto* node_env = electron::ElectronBrowserMainParts::Get()->node_env();
node::InternalCallbackScope microtasks_scope(
node_env->env(), v8::Local<v8::Object>(), {0, 0},
node::InternalCallbackScope::kAllowEmptyResource);
}
}

} // namespace electron
4 changes: 2 additions & 2 deletions shell/common/api/electron_bindings.cc
Expand Up @@ -25,6 +25,7 @@
#include "shell/common/gin_converters/file_path_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/locker.h"
#include "shell/common/gin_helper/microtasks_scope.h"
#include "shell/common/gin_helper/promise.h"
#include "shell/common/heap_snapshot.h"
#include "shell/common/node_includes.h"
Expand Down Expand Up @@ -271,8 +272,7 @@ void ElectronBindings::DidReceiveMemoryDump(
v8::Isolate* isolate = promise.isolate();
gin_helper::Locker locker(isolate);
v8::HandleScope handle_scope(isolate);
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate);
v8::Context::Scope context_scope(
v8::Local<v8::Context>::New(isolate, context));

Expand Down
10 changes: 4 additions & 6 deletions shell/common/gin_helper/callback.h
Expand Up @@ -13,6 +13,7 @@
#include "shell/common/gin_converters/std_converter.h"
#include "shell/common/gin_helper/function_template.h"
#include "shell/common/gin_helper/locker.h"
#include "shell/common/gin_helper/microtasks_scope.h"

// Implements safe convertions between JS functions and base::Callback.

Expand Down Expand Up @@ -48,8 +49,7 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
v8::EscapableHandleScope handle_scope(isolate);
if (!function.IsAlive())
return v8::Null(isolate);
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
Expand All @@ -73,8 +73,7 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
v8::HandleScope handle_scope(isolate);
if (!function.IsAlive())
return;
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
Expand All @@ -97,8 +96,7 @@ struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
ReturnType ret = ReturnType();
if (!function.IsAlive())
return ret;
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate);
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
Expand Down
4 changes: 2 additions & 2 deletions shell/common/gin_helper/event_emitter_caller.cc
Expand Up @@ -5,6 +5,7 @@
#include "shell/common/gin_helper/event_emitter_caller.h"

#include "shell/common/gin_helper/locker.h"
#include "shell/common/gin_helper/microtasks_scope.h"
#include "shell/common/node_includes.h"

namespace gin_helper {
Expand All @@ -16,8 +17,7 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
const char* method,
ValueVector* args) {
// Perform microtask checkpoint after running JavaScript.
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate);
// Use node::MakeCallback to call the callback, and it will also run pending
// tasks in Node.js.
v8::MaybeLocal<v8::Value> ret = node::MakeCallback(
Expand Down
7 changes: 3 additions & 4 deletions shell/common/gin_helper/function_template.h
Expand Up @@ -14,6 +14,7 @@
#include "shell/common/gin_helper/arguments.h"
#include "shell/common/gin_helper/destroyable.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/gin_helper/microtasks_scope.h"

// This file is forked from gin/function_template.h with 2 differences:
// 1. Support for additional types of arguments.
Expand Down Expand Up @@ -214,8 +215,7 @@ class Invoker<IndicesHolder<indices...>, ArgTypes...>

template <typename ReturnType>
void DispatchToCallback(base::Callback<ReturnType(ArgTypes...)> callback) {
v8::MicrotasksScope script_scope(args_->isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(args_->isolate());
args_->Return(
callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...));
}
Expand All @@ -224,8 +224,7 @@ 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::Callback<void(ArgTypes...)> callback) {
v8::MicrotasksScope script_scope(args_->isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(args_->isolate());
callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...);
}

Expand Down
22 changes: 22 additions & 0 deletions shell/common/gin_helper/microtasks_scope.cc
@@ -0,0 +1,22 @@
// Copyright (c) 2020 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#include "shell/common/gin_helper/microtasks_scope.h"

#include "shell/common/gin_helper/locker.h"

namespace gin_helper {

MicrotasksScope::MicrotasksScope(v8::Isolate* isolate) {
if (Locker::IsBrowserProcess()) {
v8::MicrotasksScope::PerformCheckpoint(isolate);
} else {
v8_microtasks_scope_ = std::make_unique<v8::MicrotasksScope>(
isolate, v8::MicrotasksScope::kRunMicrotasks);
}
}

MicrotasksScope::~MicrotasksScope() = default;

} // namespace gin_helper
30 changes: 30 additions & 0 deletions shell/common/gin_helper/microtasks_scope.h
@@ -0,0 +1,30 @@
// Copyright (c) 2020 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#ifndef SHELL_COMMON_GIN_HELPER_MICROTASKS_SCOPE_H_
#define SHELL_COMMON_GIN_HELPER_MICROTASKS_SCOPE_H_

#include <memory>

#include "base/macros.h"
#include "v8/include/v8.h"

namespace gin_helper {

// In the browser process runs v8::MicrotasksScope::PerformCheckpoint
// In the render process creates a v8::MicrotasksScope.
class MicrotasksScope {
public:
explicit MicrotasksScope(v8::Isolate* isolate);
~MicrotasksScope();

private:
std::unique_ptr<v8::MicrotasksScope> v8_microtasks_scope_;

DISALLOW_COPY_AND_ASSIGN(MicrotasksScope);
};

} // namespace gin_helper

#endif // SHELL_COMMON_GIN_HELPER_MICROTASKS_SCOPE_H_
12 changes: 4 additions & 8 deletions shell/common/gin_helper/promise.cc
Expand Up @@ -26,8 +26,7 @@ PromiseBase& PromiseBase::operator=(PromiseBase&&) = default;
v8::Maybe<bool> PromiseBase::Reject() {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate());
v8::Context::Scope context_scope(GetContext());

return GetInner()->Reject(GetContext(), v8::Undefined(isolate()));
Expand All @@ -36,8 +35,7 @@ v8::Maybe<bool> PromiseBase::Reject() {
v8::Maybe<bool> PromiseBase::Reject(v8::Local<v8::Value> except) {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate());
v8::Context::Scope context_scope(GetContext());

return GetInner()->Reject(GetContext(), except);
Expand All @@ -46,8 +44,7 @@ v8::Maybe<bool> PromiseBase::Reject(v8::Local<v8::Value> except) {
v8::Maybe<bool> PromiseBase::RejectWithErrorMessage(base::StringPiece message) {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate());
v8::Context::Scope context_scope(GetContext());

v8::Local<v8::Value> error =
Expand Down Expand Up @@ -90,8 +87,7 @@ v8::Local<v8::Promise> Promise<void>::ResolvedPromise(v8::Isolate* isolate) {
v8::Maybe<bool> Promise<void>::Resolve() {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate());
v8::Context::Scope context_scope(GetContext());

return GetInner()->Resolve(GetContext(), v8::Undefined(isolate()));
Expand Down
4 changes: 2 additions & 2 deletions shell/common/gin_helper/promise.h
Expand Up @@ -16,6 +16,7 @@
#include "content/public/browser/browser_thread.h"
#include "shell/common/gin_converters/std_converter.h"
#include "shell/common/gin_helper/locker.h"
#include "shell/common/gin_helper/microtasks_scope.h"

namespace gin_helper {

Expand Down Expand Up @@ -110,8 +111,7 @@ class Promise : public PromiseBase {
v8::Maybe<bool> Resolve(const RT& value) {
gin_helper::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(isolate());
v8::Context::Scope context_scope(GetContext());

return GetInner()->Resolve(GetContext(),
Expand Down
4 changes: 2 additions & 2 deletions shell/common/node_bindings.cc
Expand Up @@ -29,6 +29,7 @@
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/event_emitter_caller.h"
#include "shell/common/gin_helper/locker.h"
#include "shell/common/gin_helper/microtasks_scope.h"
#include "shell/common/mac/main_application_bundle.h"
#include "shell/common/node_includes.h"

Expand Down Expand Up @@ -475,8 +476,7 @@ void NodeBindings::UvRunOnce() {
v8::Context::Scope context_scope(env->context());

// Perform microtask checkpoint after running JavaScript.
v8::MicrotasksScope script_scope(env->isolate(),
v8::MicrotasksScope::kRunMicrotasks);
gin_helper::MicrotasksScope microtasks_scope(env->isolate());

if (browser_env_ != BrowserEnvironment::BROWSER)
TRACE_EVENT_BEGIN0("devtools.timeline", "FunctionCall");
Expand Down

0 comments on commit 9b6ef9a

Please sign in to comment.