fix: app.setLoginItemSettings error getting swallowed by gin conversion

filenames.gni

@@ -590,6 +590,8 @@ filenames = {
     "shell/common/gin_converters/hid_device_info_converter.h",
     "shell/common/gin_converters/image_converter.cc",
     "shell/common/gin_converters/image_converter.h",
+    "shell/common/gin_converters/login_item_settings_converter.cc",
+    "shell/common/gin_converters/login_item_settings_converter.h",
     "shell/common/gin_converters/media_converter.cc",
     "shell/common/gin_converters/media_converter.h",
     "shell/common/gin_converters/message_box_converter.cc",

shell/browser/api/electron_api_app.cc

@@ -89,7 +89,6 @@
 
 #if BUILDFLAG(IS_MAC)
 #include <CoreFoundation/CoreFoundation.h>
-#include "base/mac/mac_util.h"
 #include "shell/browser/ui/cocoa/electron_bundle_mover.h"
 #endif
 
@@ -324,80 +323,6 @@ struct Converter<JumpListResult> {
 };
 #endif
 
-#if BUILDFLAG(IS_WIN)
-template <>
-struct Converter<Browser::LaunchItem> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     Browser::LaunchItem* out) {
-    gin_helper::Dictionary dict;
-    if (!ConvertFromV8(isolate, val, &dict))
-      return false;
-
-    dict.Get("name", &(out->name));
-    dict.Get("path", &(out->path));
-    dict.Get("args", &(out->args));
-    dict.Get("scope", &(out->scope));
-    dict.Get("enabled", &(out->enabled));
-    return true;
-  }
-
-  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
-                                   Browser::LaunchItem val) {
-    auto dict = gin_helper::Dictionary::CreateEmpty(isolate);
-    dict.Set("name", val.name);
-    dict.Set("path", val.path);
-    dict.Set("args", val.args);
-    dict.Set("scope", val.scope);
-    dict.Set("enabled", val.enabled);
-    return dict.GetHandle();
-  }
-};
-#endif
-
-template <>
-struct Converter<Browser::LoginItemSettings> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     Browser::LoginItemSettings* out) {
-    gin_helper::Dictionary dict;
-    if (!ConvertFromV8(isolate, val, &dict))
-      return false;
-
-    dict.Get("openAtLogin", &(out->open_at_login));
-    dict.Get("openAsHidden", &(out->open_as_hidden));
-    dict.Get("path", &(out->path));
-    dict.Get("args", &(out->args));
-#if BUILDFLAG(IS_WIN)
-    dict.Get("name", &(out->name));
-    dict.Get("enabled", &(out->enabled));
-#elif BUILDFLAG(IS_MAC)
-    dict.Get("serviceName", &(out->service_name));
-    dict.Get("type", &(out->type));
-#endif
-    return true;
-  }
-
-  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
-                                   Browser::LoginItemSettings val) {
-    auto dict = gin_helper::Dictionary::CreateEmpty(isolate);
-#if BUILDFLAG(IS_WIN)
-    dict.Set("launchItems", val.launch_items);
-    dict.Set("executableWillLaunchAtLogin",
-             val.executable_will_launch_at_login);
-#elif BUILDFLAG(IS_MAC)
-    if (base::mac::MacOSMajorVersion() >= 13)
-      dict.Set("status", val.status);
-#endif
-    dict.Set("openAtLogin", val.open_at_login);
-    dict.Set("openAsHidden", val.open_as_hidden);
-    dict.Set("restoreState", val.restore_state);
-    dict.Set("wasOpenedAtLogin", val.opened_at_login);
-    dict.Set("wasOpenedAsHidden", val.opened_as_hidden);
-    return dict.GetHandle();
-  }
-};
-
 template <>
 struct Converter<content::CertificateRequestResultType> {
   static bool FromV8(v8::Isolate* isolate,
@@ -1216,8 +1141,8 @@ void App::SetAccessibilitySupportEnabled(gin_helper::ErrorThrower thrower,
   Browser::Get()->OnAccessibilitySupportChanged();
 }
 
-Browser::LoginItemSettings App::GetLoginItemSettings(gin::Arguments* args) {
-  Browser::LoginItemSettings options;
+v8::Local<v8::Value> App::GetLoginItemSettings(gin::Arguments* args) {
+  LoginItemSettings options;
   args->GetNext(&options);
   return Browser::Get()->GetLoginItemSettings(options);
 }

shell/browser/api/electron_api_app.h

@@ -206,7 +206,7 @@ class App : public ElectronBrowserClient::Delegate,
   bool IsAccessibilitySupportEnabled();
   void SetAccessibilitySupportEnabled(gin_helper::ErrorThrower thrower,
                                       bool enabled);
-  Browser::LoginItemSettings GetLoginItemSettings(gin::Arguments* args);
+  v8::Local<v8::Value> GetLoginItemSettings(gin::Arguments* args);
 #if BUILDFLAG(USE_NSS_CERTS)
   void ImportCertificate(gin_helper::ErrorThrower thrower,
                          base::Value options,

shell/browser/browser.cc

@@ -27,6 +27,16 @@
 
 namespace electron {
 
+LoginItemSettings::LoginItemSettings() = default;
+LoginItemSettings::~LoginItemSettings() = default;
+LoginItemSettings::LoginItemSettings(const LoginItemSettings& other) = default;
+
+#if BUILDFLAG(IS_WIN)
+LaunchItem::LaunchItem() = default;
+LaunchItem::~LaunchItem() = default;
+LaunchItem::LaunchItem(const LaunchItem& other) = default;
+#endif
+
 namespace {
 
 // Call |quit| after Chromium is fully started.
@@ -43,17 +53,6 @@ void RunQuitClosure(base::OnceClosure quit) {
 
 }  // namespace
 
-#if BUILDFLAG(IS_WIN)
-Browser::LaunchItem::LaunchItem() = default;
-Browser::LaunchItem::~LaunchItem() = default;
-Browser::LaunchItem::LaunchItem(const LaunchItem& other) = default;
-#endif
-
-Browser::LoginItemSettings::LoginItemSettings() = default;
-Browser::LoginItemSettings::~LoginItemSettings() = default;
-Browser::LoginItemSettings::LoginItemSettings(const LoginItemSettings& other) =
-    default;
-
 Browser::Browser() {
   WindowList::AddObserver(this);
 }

shell/browser/browser.h

@@ -17,6 +17,7 @@
 #include "gin/dictionary.h"
 #include "shell/browser/browser_observer.h"
 #include "shell/browser/window_list_observer.h"
+#include "shell/common/gin_converters/login_item_settings_converter.h"
 #include "shell/common/gin_helper/promise.h"
 
 #if BUILDFLAG(IS_WIN)
@@ -41,6 +42,48 @@ namespace electron {
 
 class ElectronMenuModel;
 
+#if BUILDFLAG(IS_WIN)
+struct LaunchItem {
+  std::wstring name;
+  std::wstring path;
+  std::wstring scope;
+  std::vector<std::wstring> args;
+  bool enabled = true;
+
+  LaunchItem();
+  ~LaunchItem();
+  LaunchItem(const LaunchItem&);
+};
+#endif
+
+struct LoginItemSettings {
+  bool open_at_login = false;
+  bool open_as_hidden = false;
+  bool restore_state = false;
+  bool opened_at_login = false;
+  bool opened_as_hidden = false;
+  std::u16string path;
+  std::vector<std::u16string> args;
+
+#if BUILDFLAG(IS_MAC)
+  std::string type = "mainAppService";
+  std::string service_name;
+  std::string status;
+#elif BUILDFLAG(IS_WIN)
+  // used in browser::setLoginItemSettings
+  bool enabled = true;
+  std::wstring name;
+
+  // used in browser::getLoginItemSettings
+  bool executable_will_launch_at_login = false;
+  std::vector<LaunchItem> launch_items;
+#endif
+
+  LoginItemSettings();
+  ~LoginItemSettings();
+  LoginItemSettings(const LoginItemSettings&);
+};
+
 // This class is used for control application-wide operations.
 class Browser : public WindowListObserver {
  public:
@@ -112,50 +155,8 @@ class Browser : public WindowListObserver {
   bool SetBadgeCount(std::optional<int> count);
   [[nodiscard]] int badge_count() const { return badge_count_; }
 
-#if BUILDFLAG(IS_WIN)
-  struct LaunchItem {
-    std::wstring name;
-    std::wstring path;
-    std::wstring scope;
-    std::vector<std::wstring> args;
-    bool enabled = true;
-
-    LaunchItem();
-    ~LaunchItem();
-    LaunchItem(const LaunchItem&);
-  };
-#endif
-
-  // Set/Get the login item settings of the app
-  struct LoginItemSettings {
-    bool open_at_login = false;
-    bool open_as_hidden = false;
-    bool restore_state = false;
-    bool opened_at_login = false;
-    bool opened_as_hidden = false;
-    std::u16string path;
-    std::vector<std::u16string> args;
-
-#if BUILDFLAG(IS_MAC)
-    std::string type = "mainAppService";
-    std::string service_name;
-    std::string status;
-#elif BUILDFLAG(IS_WIN)
-    // used in browser::setLoginItemSettings
-    bool enabled = true;
-    std::wstring name;
-
-    // used in browser::getLoginItemSettings
-    bool executable_will_launch_at_login = false;
-    std::vector<LaunchItem> launch_items;
-#endif
-
-    LoginItemSettings();
-    ~LoginItemSettings();
-    LoginItemSettings(const LoginItemSettings&);
-  };
   void SetLoginItemSettings(LoginItemSettings settings);
-  LoginItemSettings GetLoginItemSettings(const LoginItemSettings& options);
+  v8::Local<v8::Value> GetLoginItemSettings(const LoginItemSettings& options);
 
 #if BUILDFLAG(IS_MAC)
   // Set the handler which decides whether to shutdown.

shell/browser/browser_linux.cc

@@ -11,9 +11,11 @@
 #include "base/environment.h"
 #include "base/process/launch.h"
 #include "electron/electron_version.h"
+#include "shell/browser/javascript_environment.h"
 #include "shell/browser/native_window.h"
 #include "shell/browser/window_list.h"
 #include "shell/common/application_info.h"
+#include "shell/common/gin_converters/login_item_settings_converter.h"
 #include "shell/common/thread_restrictions.h"
 
 #if BUILDFLAG(IS_LINUX)
@@ -138,9 +140,10 @@ bool Browser::SetBadgeCount(std::optional<int> count) {
 
 void Browser::SetLoginItemSettings(LoginItemSettings settings) {}
 
-Browser::LoginItemSettings Browser::GetLoginItemSettings(
+v8::Local<v8::Value> Browser::GetLoginItemSettings(
     const LoginItemSettings& options) {
-  return LoginItemSettings();
+  LoginItemSettings settings;
+  return gin::ConvertToV8(JavascriptEnvironment::GetIsolate(), settings);
 }
 
 std::string Browser::GetExecutableFileVersion() const {

shell/browser/browser_mac.mm

@@ -29,6 +29,7 @@
 #include "shell/common/api/electron_api_native_image.h"
 #include "shell/common/application_info.h"
 #include "shell/common/gin_converters/image_converter.h"
+#include "shell/common/gin_converters/login_item_settings_converter.h"
 #include "shell/common/gin_helper/arguments.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/error_thrower.h"
@@ -88,8 +89,8 @@ bool CheckLoginItemStatus(bool* is_hidden) {
   return true;
 }
 
-Browser::LoginItemSettings GetLoginItemSettingsDeprecated() {
-  Browser::LoginItemSettings settings;
+LoginItemSettings GetLoginItemSettingsDeprecated() {
+  LoginItemSettings settings;
   settings.open_at_login = CheckLoginItemStatus(&settings.open_as_hidden);
   settings.restore_state = base::mac::WasLaunchedAsLoginItemRestoreState();
   settings.opened_at_login = base::mac::WasLaunchedAsLoginOrResumeItem();
@@ -375,13 +376,15 @@ bool CheckLoginItemStatus(bool* is_hidden) {
   }
 }
 
-Browser::LoginItemSettings Browser::GetLoginItemSettings(
+v8::Local<v8::Value> Browser::GetLoginItemSettings(
     const LoginItemSettings& options) {
   LoginItemSettings settings;
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+
   if (options.type != "mainAppService" && options.service_name.empty()) {
-    gin_helper::ErrorThrower(JavascriptEnvironment::GetIsolate())
-        .ThrowTypeError("'name' is required when type is not mainAppService");
-    return settings;
+    gin_helper::ErrorThrower(isolate).ThrowTypeError(
+        "'name' is required when type is not mainAppService");
+    return v8::Local<v8::Value>();
   }
 
 #if IS_MAS_BUILD()
@@ -408,7 +411,7 @@ bool CheckLoginItemStatus(bool* is_hidden) {
     settings = settings_deprecated;
   }
 #endif
-  return settings;
+  return gin::ConvertToV8(isolate, settings);
 }
 
 void Browser::SetLoginItemSettings(LoginItemSettings settings) {