chore: add error throwing utility (#19803)

* chore: add error throwing utility

* feedback from review

* DRY out repeated isolate calls

filenames.gni

@@ -490,6 +490,8 @@ filenames = {
     "shell/common/keyboard_util.h",
     "shell/common/deprecate_util.cc",
     "shell/common/deprecate_util.h",
+    "shell/common/error_util.cc",
+    "shell/common/error_util.h",
     "shell/common/mouse_util.cc",
     "shell/common/mouse_util.h",
     "shell/common/mac/main_application_bundle.h",

native_mate/native_mate/function_template.h

@@ -5,6 +5,7 @@
 #ifndef NATIVE_MATE_NATIVE_MATE_FUNCTION_TEMPLATE_H_
 #define NATIVE_MATE_NATIVE_MATE_FUNCTION_TEMPLATE_H_
 
+#include "../shell/common/error_util.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "native_mate/arguments.h"
@@ -128,6 +129,16 @@ inline bool GetNextArgument(Arguments* args,
   return true;
 }
 
+// Allow clients to pass a util::Error to throw errors if they
+// don't need the full mate::Arguments
+inline bool GetNextArgument(Arguments* args,
+                            int create_flags,
+                            bool is_first,
+                            electron::util::ErrorThrower* result) {
+  *result = electron::util::ErrorThrower(args->isolate());
+  return true;
+}
+
 // Classes for generating and storing an argument pack of integer indices
 // (based on well-known "indices trick", see: http://goo.gl/bKKojn):
 template <size_t... indices>

shell/browser/api/atom_api_system_preferences.h

@@ -12,6 +12,7 @@
 #include "base/values.h"
 #include "native_mate/handle.h"
 #include "shell/browser/api/event_emitter.h"
+#include "shell/common/error_util.h"
 #include "shell/common/promise_util.h"
 
 #if defined(OS_WIN)
@@ -95,7 +96,8 @@ class SystemPreferences : public mate::EventEmitter<SystemPreferences>
   void RemoveUserDefault(const std::string& name);
   bool IsSwipeTrackingFromScrollEventsEnabled();
 
-  std::string GetSystemColor(const std::string& color, mate::Arguments* args);
+  std::string GetSystemColor(util::ErrorThrower thrower,
+                             const std::string& color);
 
   bool CanPromptTouchID();
   v8::Local<v8::Promise> PromptTouchID(v8::Isolate* isolate,

shell/browser/api/atom_api_system_preferences_mac.mm

@@ -405,8 +405,8 @@ std::string SystemPreferences::GetAccentColor() {
   return base::SysNSStringToUTF8([sysColor RGBAValue]);
 }
 
-std::string SystemPreferences::GetSystemColor(const std::string& color,
+std::string SystemPreferences::GetSystemColor(util::ErrorThrower thrower,
-                                              mate::Arguments* args) {
+                                              const std::string& color) {
   NSColor* sysColor = nil;
   if (color == "blue") {
     sysColor = [NSColor systemBlueColor];
@@ -427,7 +427,7 @@ std::string SystemPreferences::GetSystemColor(const std::string& color,
   } else if (color == "yellow") {
     sysColor = [NSColor systemYellowColor];
   } else {
-    args->ThrowError("Unknown system color: " + color);
+    thrower.ThrowError("Unknown system color: " + color);
     return "";
   }
 

shell/common/error_util.cc

@@ -0,0 +1,45 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include <string>
+
+#include "native_mate/converter.h"
+#include "shell/common/error_util.h"
+
+namespace electron {
+
+namespace util {
+
+ErrorThrower::ErrorThrower(v8::Isolate* isolate) : isolate_(isolate) {}
+
+// This constructor should be rarely if ever used, since
+// v8::Isolate::GetCurrent() uses atomic loads and is thus a bit
+// costly to invoke
+ErrorThrower::ErrorThrower() : isolate_(v8::Isolate::GetCurrent()) {}
+
+ErrorThrower::~ErrorThrower() = default;
+
+void ErrorThrower::ThrowError(const std::string& err_msg) {
+  Throw(v8::Exception::Error, err_msg);
+}
+
+void ErrorThrower::ThrowTypeError(const std::string& err_msg) {
+  Throw(v8::Exception::TypeError, err_msg);
+}
+
+void ErrorThrower::ThrowRangeError(const std::string& err_msg) {
+  Throw(v8::Exception::RangeError, err_msg);
+}
+
+void ErrorThrower::ThrowReferenceError(const std::string& err_msg) {
+  Throw(v8::Exception::ReferenceError, err_msg);
+}
+
+void ErrorThrower::ThrowSyntaxError(const std::string& err_msg) {
+  Throw(v8::Exception::SyntaxError, err_msg);
+}
+
+}  // namespace util
+
+}  // namespace electron

shell/common/error_util.h

@@ -0,0 +1,47 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_ERROR_UTIL_H_
+#define SHELL_COMMON_ERROR_UTIL_H_
+
+#include <string>
+
+#include "native_mate/converter.h"
+
+namespace electron {
+
+namespace util {
+
+class ErrorThrower {
+ public:
+  explicit ErrorThrower(v8::Isolate* isolate);
+  ErrorThrower();
+
+  ~ErrorThrower();
+
+  void ThrowError(const std::string& err_msg);
+  void ThrowTypeError(const std::string& err_msg);
+  void ThrowRangeError(const std::string& err_msg);
+  void ThrowReferenceError(const std::string& err_msg);
+  void ThrowSyntaxError(const std::string& err_msg);
+
+ private:
+  v8::Isolate* isolate() const { return isolate_; }
+
+  using ErrorGenerator =
+      v8::Local<v8::Value> (*)(v8::Local<v8::String> err_msg);
+  void Throw(ErrorGenerator gen, const std::string& err_msg) {
+    v8::Local<v8::Value> exception = gen(mate::StringToV8(isolate_, err_msg));
+    if (!isolate_->IsExecutionTerminating())
+      isolate_->ThrowException(exception);
+  }
+
+  v8::Isolate* isolate_;
+};
+
+}  // namespace util
+
+}  // namespace electron
+
+#endif  // SHELL_COMMON_ERROR_UTIL_H_

spec-main/api-system-preferences-spec.ts

@@ -117,6 +117,13 @@ describe('systemPreferences module', () => {
   })
 
   ifdescribe(process.platform === 'darwin')('systemPreferences.getSystemColor(color)', () => {
+    it('throws on invalid system colors', () => {
+      const color = 'bad-color'
+      expect(() => {
+        systemPreferences.getSystemColor(color as any)
+      }).to.throw(`Unknown system color: ${color}`)
+    })
+  
     it('returns a valid system color', () => {
       const colors = ['blue', 'brown', 'gray', 'green', 'orange', 'pink', 'purple', 'red', 'yellow']