fix: let Node.js perform microtask checkpoint in the main process (#24131)

* fix: let Node.js perform microtask checkpoint in the main process

* fix: don't specify v8::MicrotasksScope for explicit policy

* fix: remove checkpoint from some call-sites

We already perform checkpoint at the end of a task,
either through MicrotaskRunner or through NodeBindings.
There isn't a need to add them again when calling into JS
except when dealing with promises.

* fix: remove checkpoint from some call-sites

We already perform checkpoint at the end of a task,
either through MicrotaskRunner or through NodeBindings.
There isn't a need to add them again when calling into JS
except when dealing with promises.

* fix incorrect specs

* default constructor arguments are considered for explicit mark

* add regression spec

shell/common/api/electron_bindings.cc

@@ -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"
@@ -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, true);
   v8::Context::Scope context_scope(
       v8::Local<v8::Context>::New(isolate, context));
 

shell/common/gin_helper/callback.h

@@ -13,7 +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.
 
 namespace gin_helper {
@@ -48,8 +48,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, true);
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
@@ -73,8 +72,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, true);
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
@@ -97,8 +95,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, true);
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);

shell/common/gin_helper/event_emitter_caller.cc

@@ -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 {
@@ -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, 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(

shell/common/gin_helper/function_template.h

@@ -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.
@@ -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(), true);
     args_->Return(
         callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...));
   }
@@ -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(), true);
     callback.Run(std::move(ArgumentHolder<indices, ArgTypes>::value)...);
   }
 

shell/common/gin_helper/microtasks_scope.cc

@@ -0,0 +1,24 @@
+// 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,
+                                 bool ignore_browser_checkpoint) {
+  if (Locker::IsBrowserProcess()) {
+    if (!ignore_browser_checkpoint)
+      v8::MicrotasksScope::PerformCheckpoint(isolate);
+  } else {
+    v8_microtasks_scope_ = std::make_unique<v8::MicrotasksScope>(
+        isolate, v8::MicrotasksScope::kRunMicrotasks);
+  }
+}
+
+MicrotasksScope::~MicrotasksScope() = default;
+
+}  // namespace gin_helper

shell/common/gin_helper/microtasks_scope.h

@@ -0,0 +1,31 @@
+// 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,
+                           bool ignore_browser_checkpoint = false);
+  ~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_

shell/common/gin_helper/promise.cc

@@ -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()));
@@ -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);
@@ -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 =
@@ -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()));

shell/common/gin_helper/promise.h

@@ -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 {
 
@@ -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(),

shell/common/node_bindings.cc

@@ -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"
 
@@ -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");