fix: check the result when calling js function (#17443)

* fix: check the result when calling js function

* test: should not crash when callback returns nothing

atom/common/native_mate_converters/callback.h

@@ -55,11 +55,12 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
     std::vector<v8::Local<v8::Value>> args{ConvertToV8(isolate, raw)...};
-    v8::Local<v8::Value> ret(holder
+    v8::MaybeLocal<v8::Value> ret = holder->Call(
-                                 ->Call(context, holder, args.size(),
+        context, holder, args.size(), args.empty() ? nullptr : &args.front());
-                                        args.empty() ? nullptr : &args.front())
+    if (ret.IsEmpty())
-                                 .ToLocalChecked());
+      return v8::Undefined(isolate);
-    return handle_scope.Escape(ret);
+    else
+      return handle_scope.Escape(ret.ToLocalChecked());
   }
 };
 
@@ -81,7 +82,7 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
     holder
         ->Call(context, holder, args.size(),
                args.empty() ? nullptr : &args.front())
-        .ToLocalChecked();
+        .IsEmpty();
   }
 };
 

atom/common/native_mate_converters/v8_value_converter.cc

@@ -334,11 +334,10 @@ std::unique_ptr<base::Value> V8ValueConverter::FromV8ValueImpl(
                                           v8::NewStringType::kNormal)
                       .ToLocalChecked());
     if (toISOString->IsFunction()) {
-      v8::Local<v8::Value> result = toISOString.As<v8::Function>()
+      v8::MaybeLocal<v8::Value> result =
-                                        ->Call(context, val, 0, nullptr)
+          toISOString.As<v8::Function>()->Call(context, val, 0, nullptr);
-                                        .ToLocalChecked();
       if (!result.IsEmpty()) {
-        v8::String::Utf8Value utf8(isolate, result);
+        v8::String::Utf8Value utf8(isolate, result.ToLocalChecked());
         return std::make_unique<base::Value>(std::string(*utf8, utf8.length()));
       }
     }

atom/renderer/api/atom_api_spell_check_client.cc

@@ -221,7 +221,7 @@ void SpellCheckClient::SpellCheckWords(
   v8::Local<v8::Value> args[] = {mate::ConvertToV8(isolate_, words),
                                  templ->GetFunction(context).ToLocalChecked()};
   // Call javascript with the words and the callback function
-  scope.spell_check_->Call(context, scope.provider_, 2, args).ToLocalChecked();
+  scope.spell_check_->Call(context, scope.provider_, 2, args).IsEmpty();
 }
 
 // Returns whether or not the given string is a contraction.

native_mate/native_mate/wrappable.cc

@@ -39,8 +39,7 @@ void WrappableBase::InitWith(v8::Isolate* isolate,
   // Call object._init if we have one.
   v8::Local<v8::Function> init;
   if (Dictionary(isolate, wrapper).Get("_init", &init))
-    init->Call(isolate->GetCurrentContext(), wrapper, 0, nullptr)
+    init->Call(isolate->GetCurrentContext(), wrapper, 0, nullptr).IsEmpty();
-        .ToLocalChecked();
 
   AfterInit(isolate);
 }

spec/api-browser-window-spec.js

@@ -2525,17 +2525,26 @@ describe('BrowserWindow module', () => {
 
   describe('beginFrameSubscription method', () => {
     before(function () {
-      // This test is too slow, only test it on CI.
-      if (!isCI) {
-        this.skip()
-      }
-
       // FIXME These specs crash on Linux when run in a docker container
       if (isCI && process.platform === 'linux') {
         this.skip()
       }
     })
 
+    it('does not crash when callback returns nothing', (done) => {
+      w.loadFile(path.join(fixtures, 'api', 'frame-subscriber.html'))
+      w.webContents.on('dom-ready', () => {
+        w.webContents.beginFrameSubscription(function (data) {
+          // Pending endFrameSubscription to next tick can reliably reproduce
+          // a crash which happens when nothing is returned in the callback.
+          setTimeout(() => {
+            w.webContents.endFrameSubscription()
+            done()
+          })
+        })
+      })
+    })
+
     it('subscribes to frame updates', (done) => {
       let called = false
       w.loadFile(path.join(fixtures, 'api', 'frame-subscriber.html'))