fix: crash when invalid titleBarStyle set

docs/api/structures/base-window-options.md

@@ -80,14 +80,14 @@
   * `followWindow` - The backdrop should automatically appear active when the window is active, and inactive when it is not. This is the default.
   * `active` - The backdrop should always appear active.
   * `inactive` - The backdrop should always appear inactive.
-* `titleBarStyle` string (optional) _macOS_ _Windows_ - The style of window title bar.
+* `titleBarStyle` string (optional) - The style of window title bar.
   Default is `default`. Possible values are:
   * `default` - Results in the standard title bar for macOS or Windows respectively.
   * `hidden` - Results in a hidden title bar and a full size content window. On macOS, the window still has the standard window controls (“traffic lights”) in the top left. On Windows and Linux, when combined with `titleBarOverlay: true` it will activate the Window Controls Overlay (see `titleBarOverlay` for more information), otherwise no window controls will be shown.
-  * `hiddenInset` _macOS_ - Only on macOS, results in a hidden title bar
+  * `hiddenInset` _macOS_ - Results in a hidden title bar
     with an alternative look where the traffic light buttons are slightly
     more inset from the window edge.
-  * `customButtonsOnHover` _macOS_ - Only on macOS, results in a hidden
+  * `customButtonsOnHover` _macOS_ -Results in a hidden
     title bar and a full size content window, the traffic light buttons will
     display when being hovered over in the top left of the window.
     **Note:** This option is currently experimental.

shell/browser/api/electron_api_browser_window.cc

@@ -301,7 +301,7 @@ v8::Local<v8::Value> BrowserWindow::GetWebContents(v8::Isolate* isolate) {
 void BrowserWindow::SetTitleBarOverlay(const gin_helper::Dictionary& options,
                                        gin_helper::Arguments* args) {
   // Ensure WCO is already enabled on this window
-  if (!window_->titlebar_overlay_enabled()) {
+  if (!window_->IsWindowControlsOverlayEnabled()) {
     args->ThrowError("Titlebar overlay is not enabled");
     return;
   }
@@ -324,40 +324,31 @@ void BrowserWindow::SetTitleBarOverlay(const gin_helper::Dictionary& options,
     updated = true;
   }
 
-  // Check and update the symbol color
+  // Check and update the symbol color.
   std::string symbol_color;
   if (options.Get(options::kOverlaySymbolColor, &symbol_color)) {
-    // Parse the string as a CSS color
+    // Parse the string as a CSS color.
     SkColor color;
     if (!content::ParseCssColorString(symbol_color, &color)) {
       args->ThrowError("Could not parse symbol color as CSS color");
       return;
     }
-
-    // Update the view
     window->set_overlay_symbol_color(color);
     updated = true;
   }
 
-  // Check and update the height
+  // Check and update the height.
   int height = 0;
   if (options.Get(options::kOverlayHeight, &height)) {
     window->set_titlebar_overlay_height(height);
     updated = true;
   }
 
-  if (!updated)
-    return;
-
-    // If anything was updated, ensure the overlay is repainted.
-#if BUILDFLAG(IS_WIN)
-  auto* frame_view = static_cast<WinFrameView*>(
-      window->widget()->non_client_view()->frame_view());
-#else
-  auto* frame_view = static_cast<OpaqueFrameView*>(
-      window->widget()->non_client_view()->frame_view());
-#endif
-  frame_view->InvalidateCaptionButtons();
+  if (updated) {
+    auto* frame_view = static_cast<FramelessView*>(
+        window->widget()->non_client_view()->frame_view());
+    frame_view->InvalidateCaptionButtons();
+  }
 }
 #endif
 

shell/browser/native_window.h

@@ -372,12 +372,23 @@ class NativeWindow : public base::SupportsUserData,
     kHiddenInset,
     kCustomButtonsOnHover,
   };
+
   TitleBarStyle title_bar_style() const { return title_bar_style_; }
+
+  bool IsWindowControlsOverlayEnabled() const {
+    bool valid_titlebar_style = title_bar_style() == TitleBarStyle::kHidden
+#if BUILDFLAG(IS_MAC)
+                                ||
+                                title_bar_style() == TitleBarStyle::kHiddenInset
+#endif
+        ;
+    return valid_titlebar_style && titlebar_overlay_;
+  }
+
   int titlebar_overlay_height() const { return titlebar_overlay_height_; }
   void set_titlebar_overlay_height(int height) {
     titlebar_overlay_height_ = height;
   }
-  bool titlebar_overlay_enabled() const { return titlebar_overlay_; }
 
   bool has_frame() const { return has_frame_; }
   void set_has_frame(bool has_frame) { has_frame_ = has_frame; }

shell/browser/native_window_views.cc

@@ -245,7 +245,8 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
     }
   }
 
-  if (title_bar_style_ != TitleBarStyle::kNormal)
+  // |hidden| is the only non-default titleBarStyle valid on Windows and Linux.
+  if (title_bar_style_ == TitleBarStyle::kHidden)
     set_has_frame(false);
 
 #if BUILDFLAG(IS_WIN)

shell/browser/native_window_views.h

@@ -170,10 +170,6 @@ class NativeWindowViews : public NativeWindow,
   void UpdateThickFrame();
 #endif
 
-  bool IsWindowControlsOverlayEnabled() const {
-    return (title_bar_style_ == NativeWindowViews::TitleBarStyle::kHidden) &&
-           titlebar_overlay_;
-  }
   SkColor overlay_button_color() const { return overlay_button_color_; }
   void set_overlay_button_color(SkColor color) {
     overlay_button_color_ = color;

shell/browser/ui/views/frameless_view.cc

@@ -95,14 +95,12 @@ void FramelessView::ResetWindowControls() {}
 
 void FramelessView::UpdateWindowIcon() {}
 
+void FramelessView::InvalidateCaptionButtons() {}
+
 void FramelessView::UpdateWindowTitle() {}
 
 void FramelessView::SizeConstraintsChanged() {}
 
-bool FramelessView::ShouldCustomDrawSystemTitlebar() const {
-  return window()->IsWindowControlsOverlayEnabled();
-}
-
 views::View* FramelessView::TargetForRect(views::View* root,
                                           const gfx::Rect& rect) {
   CHECK_EQ(root, this);

shell/browser/ui/views/frameless_view.h

@@ -33,6 +33,10 @@ class FramelessView : public views::NonClientFrameView {
   // Returns whether the |point| is on frameless window's resizing border.
   virtual int ResizingBorderHitTest(const gfx::Point& point);
 
+  // Tells the NonClientView to invalidate caption buttons
+  // and forces a re-layout and re-paint.
+  virtual void InvalidateCaptionButtons();
+
   NativeWindowViews* window() const { return window_; }
   views::Widget* frame() const { return frame_; }
 
@@ -42,8 +46,6 @@ class FramelessView : public views::NonClientFrameView {
   int ResizingBorderHitTestImpl(const gfx::Point& point,
                                 const gfx::Insets& resize_border);
 
-  bool ShouldCustomDrawSystemTitlebar() const;
-
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;
   gfx::Rect GetWindowBoundsForClientBounds(

shell/browser/ui/views/opaque_frame_view.cc

@@ -60,7 +60,7 @@ OpaqueFrameView::~OpaqueFrameView() = default;
 void OpaqueFrameView::Init(NativeWindowViews* window, views::Widget* frame) {
   FramelessView::Init(window, frame);
 
-  if (!ShouldCustomDrawSystemTitlebar())
+  if (!window->IsWindowControlsOverlayEnabled())
     return;
 
   caption_button_placeholder_container_ =
@@ -194,7 +194,7 @@ int OpaqueFrameView::FrameTopBorderThickness(bool restored) const {
 }
 
 gfx::Rect OpaqueFrameView::GetBoundsForClientView() const {
-  if (ShouldCustomDrawSystemTitlebar()) {
+  if (window()->IsWindowControlsOverlayEnabled()) {
     auto border_thickness = FrameBorderInsets(false);
     int top_height = border_thickness.top();
     return gfx::Rect(
@@ -208,7 +208,7 @@ gfx::Rect OpaqueFrameView::GetBoundsForClientView() const {
 
 gfx::Rect OpaqueFrameView::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
-  if (ShouldCustomDrawSystemTitlebar()) {
+  if (window()->IsWindowControlsOverlayEnabled()) {
     int top_height = NonClientTopHeight(false);
     auto border_insets = FrameBorderInsets(false);
     return gfx::Rect(
@@ -222,7 +222,7 @@ gfx::Rect OpaqueFrameView::GetWindowBoundsForClientBounds(
 }
 
 int OpaqueFrameView::NonClientHitTest(const gfx::Point& point) {
-  if (ShouldCustomDrawSystemTitlebar()) {
+  if (window()->IsWindowControlsOverlayEnabled()) {
     if (HitTestCaptionButton(close_button_, point))
       return HTCLOSE;
     if (HitTestCaptionButton(restore_button_, point))
@@ -238,14 +238,16 @@ int OpaqueFrameView::NonClientHitTest(const gfx::Point& point) {
     }
   }
 
-  // Use the parent class's hittest last.
   return FramelessView::NonClientHitTest(point);
 }
 
 // views::View:
 void OpaqueFrameView::OnPaint(gfx::Canvas* canvas) {
+  if (!window()->IsWindowControlsOverlayEnabled())
+    return;
+
   if (frame()->IsFullscreen())
-    return;  // Nothing is visible, so don't bother to paint.
+    return;
 
   const bool active = ShouldPaintAsActive();
   const SkColor symbol_color = window()->overlay_button_color();
@@ -283,27 +285,28 @@ views::View* OpaqueFrameView::TargetForRect(views::View* root,
 void OpaqueFrameView::Layout(PassKey) {
   LayoutSuperclass<FramelessView>(this);
 
-  if (ShouldCustomDrawSystemTitlebar()) {
-    // Reset all our data so that everything is invisible.
-    TopAreaPadding top_area_padding = GetTopAreaPadding();
-    available_space_leading_x_ = top_area_padding.leading;
-    available_space_trailing_x_ = width() - top_area_padding.trailing;
-    minimum_size_for_buttons_ =
-        available_space_leading_x_ + width() - available_space_trailing_x_;
-    placed_leading_button_ = false;
-    placed_trailing_button_ = false;
-
-    LayoutWindowControls();
-
-    int height = NonClientTopHeight(false);
-    auto insets = FrameBorderInsets(false);
-    int container_x = placed_trailing_button_ ? available_space_trailing_x_ : 0;
-    caption_button_placeholder_container_->SetBounds(
-        container_x, insets.top(), minimum_size_for_buttons_ - insets.width(),
-        height - insets.top());
-
-    LayoutWindowControlsOverlay();
-  }
+  if (!window()->IsWindowControlsOverlayEnabled())
+    return;
+
+  // Reset all our data so that everything is invisible.
+  TopAreaPadding top_area_padding = GetTopAreaPadding();
+  available_space_leading_x_ = top_area_padding.leading;
+  available_space_trailing_x_ = width() - top_area_padding.trailing;
+  minimum_size_for_buttons_ =
+      available_space_leading_x_ + width() - available_space_trailing_x_;
+  placed_leading_button_ = false;
+  placed_trailing_button_ = false;
+
+  LayoutWindowControls();
+
+  int height = NonClientTopHeight(false);
+  auto insets = FrameBorderInsets(false);
+  int container_x = placed_trailing_button_ ? available_space_trailing_x_ : 0;
+  caption_button_placeholder_container_->SetBounds(
+      container_x, insets.top(), minimum_size_for_buttons_ - insets.width(),
+      height - insets.top());
+
+  LayoutWindowControlsOverlay();
 }
 
 OpaqueFrameView::TopAreaPadding OpaqueFrameView::GetTopAreaPadding() const {
@@ -336,16 +339,14 @@ void OpaqueFrameView::LayoutWindowControls() {
   buttons_not_shown.push_back(views::FrameButton::kMinimize);
   buttons_not_shown.push_back(views::FrameButton::kClose);
 
-  if (ShouldCustomDrawSystemTitlebar()) {
-    for (const auto& button : leading_buttons_) {
-      ConfigureButton(button, ALIGN_LEADING);
-      std::erase(buttons_not_shown, button);
-    }
+  for (const auto& button : leading_buttons_) {
+    ConfigureButton(button, ALIGN_LEADING);
+    std::erase(buttons_not_shown, button);
+  }
 
-    for (const auto& button : base::Reversed(trailing_buttons_)) {
-      ConfigureButton(button, ALIGN_TRAILING);
-      std::erase(buttons_not_shown, button);
-    }
+  for (const auto& button : base::Reversed(trailing_buttons_)) {
+    ConfigureButton(button, ALIGN_TRAILING);
+    std::erase(buttons_not_shown, button);
   }
 
   for (const auto& button_id : buttons_not_shown)

shell/browser/ui/views/opaque_frame_view.h

@@ -42,8 +42,10 @@ class OpaqueFrameView : public FramelessView {
   OpaqueFrameView();
   ~OpaqueFrameView() override;
 
+  // FramelessView:
   void Init(NativeWindowViews* window, views::Widget* frame) override;
   int ResizingBorderHitTest(const gfx::Point& point) override;
+  void InvalidateCaptionButtons() override;
 
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;
@@ -57,9 +59,6 @@ class OpaqueFrameView : public FramelessView {
   void Layout(PassKey) override;
   void OnPaint(gfx::Canvas* canvas) override;
 
-  // Needed by BrowserWindow::SetTitleBarOverlay.
-  void InvalidateCaptionButtons();
-
  private:
   enum ButtonAlignment { ALIGN_LEADING, ALIGN_TRAILING };
 

shell/browser/ui/views/win_frame_view.cc

@@ -80,24 +80,23 @@ views::View* WinFrameView::TargetForRect(views::View* root,
     // Custom system titlebar returns non HTCLIENT value, however event should
     // be handled by the view, not by the system, because there are no system
     // buttons underneath.
-    if (!ShouldCustomDrawSystemTitlebar()) {
+    if (!window()->IsWindowControlsOverlayEnabled())
       return this;
-    }
+
     auto local_point = rect.origin();
     ConvertPointToTarget(parent(), caption_button_container_, &local_point);
-    if (!caption_button_container_->HitTestPoint(local_point)) {
+    if (!caption_button_container_->HitTestPoint(local_point))
       return this;
-    }
   }
 
   return NonClientFrameView::TargetForRect(root, rect);
 }
 
 int WinFrameView::NonClientHitTest(const gfx::Point& point) {
-  if (window_->has_frame())
+  if (window()->has_frame())
     return frame_->client_view()->NonClientHitTest(point);
 
-  if (ShouldCustomDrawSystemTitlebar()) {
+  if (window()->IsWindowControlsOverlayEnabled()) {
     // See if the point is within any of the window controls.
     if (caption_button_container_) {
       gfx::Point local_point = point;
@@ -191,7 +190,7 @@ int WinFrameView::FrameTopBorderThicknessPx(bool restored) const {
 
   // See comments in BrowserDesktopWindowTreeHostWin::GetClientAreaInsets().
   const bool needs_no_border =
-      (ShouldCustomDrawSystemTitlebar() && frame()->IsMaximized()) ||
+      (window()->IsWindowControlsOverlayEnabled() && frame()->IsMaximized()) ||
       frame()->IsFullscreen();
   if (needs_no_border && !restored)
     return 0;
@@ -239,7 +238,7 @@ void WinFrameView::LayoutCaptionButtons() {
     return;
 
   // Non-custom system titlebar already contains caption buttons.
-  if (!ShouldCustomDrawSystemTitlebar()) {
+  if (!window()->IsWindowControlsOverlayEnabled()) {
     caption_button_container_->SetVisible(false);
     return;
   }

shell/browser/ui/views/win_frame_view.h

@@ -26,16 +26,14 @@ class WinFrameView : public FramelessView {
   ~WinFrameView() override;
 
   void Init(NativeWindowViews* window, views::Widget* frame) override;
+  void InvalidateCaptionButtons() override;
 
   // Alpha to use for features in the titlebar (the window title and caption
   // buttons) when the window is inactive. They are opaque when active.
   static constexpr SkAlpha kInactiveTitlebarFeatureAlpha = 0x66;
 
   SkColor GetReadableFeatureColor(SkColor background_color);
 
-  // Tells the NonClientView to invalidate the WinFrameView's caption buttons.
-  void InvalidateCaptionButtons();
-
   // views::NonClientFrameView:
   gfx::Rect GetWindowBoundsForClientBounds(
       const gfx::Rect& client_bounds) const override;

spec/api-browser-window-spec.ts

@@ -3132,7 +3132,7 @@ describe('BrowserWindow module', () => {
       ipcMain.removeAllListeners('geometrychange');
     });
 
-    it('does not crash when an invalid titleBarStyle was initially set', () => {
+    it('throws when an invalid titleBarStyle is initially set', () => {
       const win = new BrowserWindow({
         show: false,
         webPreferences: {
@@ -3150,7 +3150,7 @@ describe('BrowserWindow module', () => {
         win.setTitleBarOverlay({
           color: '#000000'
         });
-      }).to.not.throw();
+      }).to.throw('Titlebar overlay is not enabled');
     });
 
     it('correctly updates the height of the overlay', async () => {