refactor: promise_util promise creation (#16401)

* refactor: promise_util creation

* enter correct contexts on resolve/reject

* return Local in helper

* set context correctly

* forgot one

atom/browser/api/atom_api_app.cc

@@ -533,12 +533,8 @@ int ImportIntoCertStore(CertificateManagerModel* model,
 }
 #endif
 
-void OnIconDataAvailable(v8::Isolate* isolate,
+void OnIconDataAvailable(scoped_refptr<util::Promise> promise,
-                         scoped_refptr<util::Promise> promise,
                          gfx::Image* icon) {
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-
   if (icon && !icon->IsEmpty()) {
     promise->Resolve(*icon);
   } else {
@@ -1130,9 +1126,6 @@ JumpListResult App::SetJumpList(v8::Local<v8::Value> val,
 
 v8::Local<v8::Promise> App::GetFileIcon(const base::FilePath& path,
                                         mate::Arguments* args) {
-  v8::Locker locker(isolate());
-  v8::HandleScope handle_scope(isolate());
-
   scoped_refptr<util::Promise> promise = new util::Promise(isolate());
   base::FilePath normalized_path = path.NormalizePathSeparators();
 
@@ -1153,7 +1146,7 @@ v8::Local<v8::Promise> App::GetFileIcon(const base::FilePath& path,
     promise->Resolve(*icon);
   } else {
     icon_manager->LoadIcon(normalized_path, icon_size,
-                           base::Bind(&OnIconDataAvailable, isolate(), promise),
+                           base::Bind(&OnIconDataAvailable, promise),
                            &cancelable_task_tracker_);
   }
   return promise->GetHandle();

atom/common/api/atom_api_shell.cc

@@ -44,17 +44,8 @@ struct Converter<base::win::ShortcutOperation> {
 
 namespace {
 
-void OnOpenExternalFinished(const v8::Global<v8::Context>& context,
+void OnOpenExternalFinished(scoped_refptr<atom::util::Promise> promise,
-                            scoped_refptr<atom::util::Promise> promise,
                             const std::string& error) {
-  v8::Isolate* isolate = promise->isolate();
-  mate::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  v8::MicrotasksScope script_scope(isolate,
-                                   v8::MicrotasksScope::kRunMicrotasks);
-  v8::Context::Scope context_scope(
-      v8::Local<v8::Context>::New(isolate, context));
-
   if (error.empty())
     promise->Resolve();
   else
@@ -99,11 +90,8 @@ v8::Local<v8::Promise> OpenExternal(
     }
   }
 
-  v8::Global<v8::Context> context(args->isolate(),
+  platform_util::OpenExternal(url, options,
-                                  args->isolate()->GetCurrentContext());
+                              base::Bind(&OnOpenExternalFinished, promise));
-  platform_util::OpenExternal(
-      url, options,
-      base::Bind(&OnOpenExternalFinished, std::move(context), promise));
 
   return promise->GetHandle();
 }

atom/common/promise_util.cc

@@ -2,10 +2,11 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "atom/common/promise_util.h"
-
 #include <string>
 
+#include "atom/common/api/locker.h"
+#include "atom/common/promise_util.h"
+
 namespace atom {
 
 namespace util {
@@ -14,12 +15,20 @@ Promise::Promise(v8::Isolate* isolate) {
   auto context = isolate->GetCurrentContext();
   auto resolver = v8::Promise::Resolver::New(context).ToLocalChecked();
   isolate_ = isolate;
+
+  context_.Reset(isolate, context);
   resolver_.Reset(isolate, resolver);
 }
 
 Promise::~Promise() = default;
 
 v8::Maybe<bool> Promise::RejectWithErrorMessage(const std::string& string) {
+  v8::HandleScope handle_scope(isolate());
+  v8::MicrotasksScope script_scope(isolate(),
+                                   v8::MicrotasksScope::kRunMicrotasks);
+  v8::Context::Scope context_scope(
+      v8::Local<v8::Context>::New(isolate(), GetContext()));
+
   v8::Local<v8::String> error_message =
       v8::String::NewFromUtf8(isolate(), string.c_str());
   v8::Local<v8::Value> error = v8::Exception::Error(error_message);

atom/common/promise_util.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "atom/common/api/locker.h"
 #include "content/public/browser/browser_thread.h"
 #include "native_mate/converter.h"
 
@@ -19,30 +20,55 @@ class Promise : public base::RefCounted<Promise> {
   explicit Promise(v8::Isolate* isolate);
 
   v8::Isolate* isolate() const { return isolate_; }
+  v8::Local<v8::Context> GetContext() {
+    return v8::Local<v8::Context>::New(isolate_, context_);
+  }
 
   virtual v8::Local<v8::Promise> GetHandle() const;
 
   v8::Maybe<bool> Resolve() {
-    return GetInner()->Resolve(isolate()->GetCurrentContext(),
+    v8::HandleScope handle_scope(isolate());
-                               v8::Undefined(isolate()));
+    v8::MicrotasksScope script_scope(isolate(),
+                                     v8::MicrotasksScope::kRunMicrotasks);
+    v8::Context::Scope context_scope(
+        v8::Local<v8::Context>::New(isolate(), GetContext()));
+
+    return GetInner()->Resolve(GetContext(), v8::Undefined(isolate()));
   }
 
   v8::Maybe<bool> Reject() {
-    return GetInner()->Reject(isolate()->GetCurrentContext(),
+    v8::HandleScope handle_scope(isolate());
-                              v8::Undefined(isolate()));
+    v8::MicrotasksScope script_scope(isolate(),
+                                     v8::MicrotasksScope::kRunMicrotasks);
+    v8::Context::Scope context_scope(
+        v8::Local<v8::Context>::New(isolate(), GetContext()));
+
+    return GetInner()->Reject(GetContext(), v8::Undefined(isolate()));
   }
 
   // Promise resolution is a microtask
   // We use the MicrotasksRunner to trigger the running of pending microtasks
   template <typename T>
   v8::Maybe<bool> Resolve(const T& value) {
-    return GetInner()->Resolve(isolate()->GetCurrentContext(),
+    v8::HandleScope handle_scope(isolate());
+    v8::MicrotasksScope script_scope(isolate(),
+                                     v8::MicrotasksScope::kRunMicrotasks);
+    v8::Context::Scope context_scope(
+        v8::Local<v8::Context>::New(isolate(), GetContext()));
+
+    return GetInner()->Resolve(GetContext(),
                                mate::ConvertToV8(isolate(), value));
   }
 
   template <typename T>
   v8::Maybe<bool> Reject(const T& value) {
-    return GetInner()->Reject(isolate()->GetCurrentContext(),
+    v8::HandleScope handle_scope(isolate());
+    v8::MicrotasksScope script_scope(isolate(),
+                                     v8::MicrotasksScope::kRunMicrotasks);
+    v8::Context::Scope context_scope(
+        v8::Local<v8::Context>::New(isolate(), GetContext()));
+
+    return GetInner()->Reject(GetContext(),
                               mate::ConvertToV8(isolate(), value));
   }
 
@@ -52,6 +78,7 @@ class Promise : public base::RefCounted<Promise> {
   virtual ~Promise();
   friend class base::RefCounted<Promise>;
   v8::Isolate* isolate_;
+  v8::Global<v8::Context> context_;
 
  private:
   v8::Local<v8::Promise::Resolver> GetInner() const {