feat: add BrowserWindow.set/getWindowButtonPosition APIs

docs/api/browser-window.md

@@ -1842,16 +1842,17 @@ will remove the vibrancy effect on the window.
 Note that `appearance-based`, `light`, `dark`, `medium-light`, and `ultra-dark` have been
 deprecated and will be removed in an upcoming version of macOS.
 
-#### `win.setTrafficLightPosition(position)` _macOS_
+#### `win.setWindowButtonPosition(position)` _macOS_
 
-* `position` [Point](structures/point.md)
+* `position` [Point](structures/point.md) | null
 
 Set a custom position for the traffic light buttons in frameless window.
+Passing `null` will reset the position to default.
 
-#### `win.getTrafficLightPosition()` _macOS_
+#### `win.getWindowButtonPosition()` _macOS_
 
-Returns `Point` - The custom position for the traffic light buttons in
-frameless window.
+Returns `Point | null` - The custom position for the traffic light buttons in
+frameless window, `null` will be returned when there is no custom position.
 
 #### `win.setTouchBar(touchBar)` _macOS_
 

filenames.gni

@@ -586,6 +586,7 @@ filenames = {
     "shell/common/gin_converters/native_window_converter.h",
     "shell/common/gin_converters/net_converter.cc",
     "shell/common/gin_converters/net_converter.h",
+    "shell/common/gin_converters/optional_converter.h",
     "shell/common/gin_converters/serial_port_info_converter.h",
     "shell/common/gin_converters/std_converter.h",
     "shell/common/gin_converters/time_converter.cc",

shell/browser/api/electron_api_app.cc

@@ -64,6 +64,7 @@
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_converters/net_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"

shell/browser/api/electron_api_base_window.cc

@@ -21,6 +21,7 @@
 #include "shell/common/gin_converters/gfx_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_converters/native_window_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
@@ -873,17 +874,25 @@ bool BaseWindow::GetWindowButtonVisibility() const {
   return window_->GetWindowButtonVisibility();
 }
 
+void BaseWindow::SetWindowButtonPosition(absl::optional<gfx::Point> position) {
+  window_->SetWindowButtonPosition(std::move(position));
+}
+
+absl::optional<gfx::Point> BaseWindow::GetWindowButtonPosition() const {
+  return window_->GetWindowButtonPosition();
+}
+
 void BaseWindow::SetTrafficLightPosition(const gfx::Point& position) {
   // For backward compatibility we treat (0, 0) as resetting to default.
   if (position.IsOrigin())
-    window_->SetTrafficLightPosition(absl::nullopt);
+    SetWindowButtonPosition(absl::nullopt);
   else
-    window_->SetTrafficLightPosition(position);
+    SetWindowButtonPosition(position);
 }
 
 gfx::Point BaseWindow::GetTrafficLightPosition() const {
   // For backward compatibility we treat default value as (0, 0).
-  return window_->GetTrafficLightPosition().value_or(gfx::Point());
+  return GetWindowButtonPosition().value_or(gfx::Point());
 }
 #endif
 
@@ -1296,6 +1305,10 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate,
                  &BaseWindow::SetWindowButtonVisibility)
       .SetMethod("_getWindowButtonVisibility",
                  &BaseWindow::GetWindowButtonVisibility)
+      .SetMethod("setWindowButtonPosition",
+                 &BaseWindow::SetWindowButtonPosition)
+      .SetMethod("getWindowButtonPosition",
+                 &BaseWindow::GetWindowButtonPosition)
       .SetProperty("excludedFromShownWindowsMenu",
                    &BaseWindow::IsExcludedFromShownWindowsMenu,
                    &BaseWindow::SetExcludedFromShownWindowsMenu)

shell/browser/api/electron_api_base_window.h

@@ -195,6 +195,9 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
   std::string GetAlwaysOnTopLevel();
   void SetWindowButtonVisibility(bool visible);
   bool GetWindowButtonVisibility() const;
+  void SetWindowButtonPosition(absl::optional<gfx::Point> position);
+  absl::optional<gfx::Point> GetWindowButtonPosition() const;
+  // Deprecated.
   void SetTrafficLightPosition(const gfx::Point& position);
   gfx::Point GetTrafficLightPosition() const;
 #endif

shell/browser/api/electron_api_menu.cc

@@ -14,6 +14,7 @@
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"
@@ -33,9 +34,9 @@ struct Converter<SharingItem> {
     gin_helper::Dictionary dict;
     if (!ConvertFromV8(isolate, val, &dict))
       return false;
-    dict.GetOptional("texts", &(out->texts));
-    dict.GetOptional("filePaths", &(out->file_paths));
-    dict.GetOptional("urls", &(out->urls));
+    dict.Get("texts", &(out->texts));
+    dict.Get("filePaths", &(out->file_paths));
+    dict.Get("urls", &(out->urls));
     return true;
   }
 };

shell/browser/api/electron_api_tray.cc

@@ -18,6 +18,7 @@
 #include "shell/common/gin_converters/gfx_converter.h"
 #include "shell/common/gin_converters/guid_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/function_template_extensions.h"
 #include "shell/common/node_includes.h"

shell/browser/native_window.h

@@ -219,8 +219,8 @@ class NativeWindow : public base::SupportsUserData,
 #if BUILDFLAG(IS_MAC)
   virtual void SetWindowButtonVisibility(bool visible) = 0;
   virtual bool GetWindowButtonVisibility() const = 0;
-  virtual void SetTrafficLightPosition(absl::optional<gfx::Point> position) = 0;
-  virtual absl::optional<gfx::Point> GetTrafficLightPosition() const = 0;
+  virtual void SetWindowButtonPosition(absl::optional<gfx::Point> position) = 0;
+  virtual absl::optional<gfx::Point> GetWindowButtonPosition() const = 0;
   virtual void RedrawTrafficLights() = 0;
   virtual void UpdateFrame() = 0;
 #endif

shell/browser/native_window_mac.h

@@ -129,8 +129,8 @@ class NativeWindowMac : public NativeWindow,
   void SetVibrancy(const std::string& type) override;
   void SetWindowButtonVisibility(bool visible) override;
   bool GetWindowButtonVisibility() const override;
-  void SetTrafficLightPosition(absl::optional<gfx::Point> position) override;
-  absl::optional<gfx::Point> GetTrafficLightPosition() const override;
+  void SetWindowButtonPosition(absl::optional<gfx::Point> position) override;
+  absl::optional<gfx::Point> GetWindowButtonPosition() const override;
   void RedrawTrafficLights() override;
   void UpdateFrame() override;
   void SetTouchBar(

shell/browser/native_window_mac.mm

@@ -36,6 +36,7 @@
 #include "shell/browser/ui/inspectable_web_contents.h"
 #include "shell/browser/window_list.h"
 #include "shell/common/gin_converters/gfx_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/options_switches.h"
@@ -183,7 +184,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   options.Get(options::kResizable, &resizable);
   options.Get(options::kZoomToPageWidth, &zoom_to_page_width_);
   options.Get(options::kSimpleFullScreen, &always_simple_fullscreen_);
-  options.GetOptional(options::kTrafficLightPosition, &traffic_light_position_);
+  options.Get(options::kTrafficLightPosition, &traffic_light_position_);
   options.Get(options::kVisualEffectState, &visual_effect_state_);
 
   if (options.Has(options::kFullscreenWindowTitle)) {
@@ -1474,7 +1475,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
          ![window_ standardWindowButton:NSWindowCloseButton].hidden;
 }
 
-void NativeWindowMac::SetTrafficLightPosition(
+void NativeWindowMac::SetWindowButtonPosition(
     absl::optional<gfx::Point> position) {
   traffic_light_position_ = std::move(position);
   if (buttons_proxy_) {
@@ -1483,7 +1484,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   }
 }
 
-absl::optional<gfx::Point> NativeWindowMac::GetTrafficLightPosition() const {
+absl::optional<gfx::Point> NativeWindowMac::GetWindowButtonPosition() const {
   return traffic_light_position_;
 }
 

shell/common/api/electron_api_native_image.cc

@@ -24,6 +24,7 @@
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/gfx_converter.h"
 #include "shell/common/gin_converters/gurl_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/function_template_extensions.h"

shell/common/gin_converters/message_box_converter.cc

@@ -6,6 +6,7 @@
 
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_converters/native_window_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 
 namespace gin {
@@ -22,7 +23,7 @@ bool Converter<electron::MessageBoxSettings>::FromV8(
   dict.Get("messageBoxType", &type);
   out->type = static_cast<electron::MessageBoxType>(type);
   dict.Get("buttons", &out->buttons);
-  dict.GetOptional("id", &out->id);
+  dict.Get("id", &out->id);
   dict.Get("defaultId", &out->default_id);
   dict.Get("cancelId", &out->cancel_id);
   dict.Get("title", &out->title);

shell/common/gin_converters/optional_converter.h

@@ -0,0 +1,36 @@
+// Copyright (c) 2023 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ELECTRON_SHELL_COMMON_GIN_CONVERTERS_OPTIONAL_CONVERTER_H_
+#define ELECTRON_SHELL_COMMON_GIN_CONVERTERS_OPTIONAL_CONVERTER_H_
+
+#include <utility>
+
+#include "gin/converter.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
+
+namespace gin {
+
+template <typename T>
+struct Converter<absl::optional<T>> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const absl::optional<T>& val) {
+    if (val)
+      return Converter<T>::ToV8(isolate, val.value());
+    else
+      return v8::Null(isolate);
+  }
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     absl::optional<T>* out) {
+    T converted;
+    if (Converter<T>::FromV8(isolate, val, &converted))
+      out->emplace(std::move(converted));
+    return true;
+  }
+};
+
+}  // namespace gin
+
+#endif  // ELECTRON_SHELL_COMMON_GIN_CONVERTERS_OPTIONAL_CONVERTER_H_

shell/common/gin_helper/dictionary.h

@@ -62,18 +62,6 @@ class Dictionary : public gin::Dictionary {
     return !result.IsNothing() && result.FromJust();
   }
 
-  // Like normal Get but put result in an absl::optional.
-  template <typename T>
-  bool GetOptional(base::StringPiece key, absl::optional<T>* out) const {
-    T ret;
-    if (Get(key, &ret)) {
-      out->emplace(std::move(ret));
-      return true;
-    } else {
-      return false;
-    }
-  }
-
   template <typename T>
   bool GetHidden(base::StringPiece key, T* out) const {
     v8::Local<v8::Context> context = isolate()->GetCurrentContext();

shell/common/gin_helper/function_template_extensions.h

@@ -16,19 +16,6 @@
 // in the gin_helper namespace.
 namespace gin {
 
-// Support absl::optional as an argument.
-template <typename T>
-bool GetNextArgument(Arguments* args,
-                     const InvokerOptions& invoker_options,
-                     bool is_first,
-                     absl::optional<T>* result) {
-  T converted;
-  // Use gin::Arguments::GetNext which always advances |next| counter.
-  if (args->GetNext(&converted))
-    result->emplace(std::move(converted));
-  return true;
-}
-
 inline bool GetNextArgument(Arguments* args,
                             const InvokerOptions& invoker_options,
                             bool is_first,

shell/renderer/api/electron_api_ipc_renderer.cc

@@ -14,6 +14,7 @@
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "shell/common/api/api.mojom.h"
 #include "shell/common/gin_converters/blink_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/function_template_extensions.h"

spec/api-browser-window-spec.ts

@@ -2095,31 +2095,42 @@ describe('BrowserWindow module', () => {
     const pos = { x: 10, y: 10 };
     afterEach(closeAllWindows);
 
-    describe('BrowserWindow.getTrafficLightPosition(pos)', () => {
+    describe('BrowserWindow.getWindowButtonPosition(pos)', () => {
+      it('returns null when there is no custom position', () => {
+        const w = new BrowserWindow({ show: false });
+        expect(w.getWindowButtonPosition()).to.be.null('getWindowButtonPosition');
+      });
+
       it('gets position property for "hidden" titleBarStyle', () => {
         const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
-        expect(w.getTrafficLightPosition()).to.deep.equal(pos);
+        expect(w.getWindowButtonPosition()).to.deep.equal(pos);
       });
 
       it('gets position property for "customButtonsOnHover" titleBarStyle', () => {
         const w = new BrowserWindow({ show: false, titleBarStyle: 'customButtonsOnHover', trafficLightPosition: pos });
-        expect(w.getTrafficLightPosition()).to.deep.equal(pos);
+        expect(w.getWindowButtonPosition()).to.deep.equal(pos);
       });
     });
 
-    describe('BrowserWindow.setTrafficLightPosition(pos)', () => {
+    describe('BrowserWindow.setWindowButtonPosition(pos)', () => {
+      it('resets the position when null is passed', () => {
+        const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
+        w.setWindowButtonPosition(null);
+        expect(w.getWindowButtonPosition()).to.be.null('setWindowButtonPosition');
+      });
+
       it('sets position property for "hidden" titleBarStyle', () => {
         const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
         const newPos = { x: 20, y: 20 };
-        w.setTrafficLightPosition(newPos);
-        expect(w.getTrafficLightPosition()).to.deep.equal(newPos);
+        w.setWindowButtonPosition(newPos);
+        expect(w.getWindowButtonPosition()).to.deep.equal(newPos);
       });
 
       it('sets position property for "customButtonsOnHover" titleBarStyle', () => {
         const w = new BrowserWindow({ show: false, titleBarStyle: 'customButtonsOnHover', trafficLightPosition: pos });
         const newPos = { x: 20, y: 20 };
-        w.setTrafficLightPosition(newPos);
-        expect(w.getTrafficLightPosition()).to.deep.equal(newPos);
+        w.setWindowButtonPosition(newPos);
+        expect(w.getWindowButtonPosition()).to.deep.equal(newPos);
       });
     });
   });