refactor: move IsClosed() and IsClosable() tests into NativeWindow::Close() (#46907)

refactor: devirtualize NativeWindow::IsClosed()

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>

shell/browser/api/electron_api_base_window.cc

@@ -362,8 +362,7 @@ void BaseWindow::SetContentView(gin::Handle<View> view) {
 }
 
 void BaseWindow::CloseImmediately() {
-  if (!window_->IsClosed())
+  window_->CloseImmediately();
-    window_->CloseImmediately();
 }
 
 void BaseWindow::Close() {

shell/browser/native_window.cc

@@ -284,10 +284,6 @@ void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
     Show();
 }
 
-bool NativeWindow::IsClosed() const {
-  return is_closed_;
-}
-
 void NativeWindow::SetSize(const gfx::Size& size, bool animate) {
   SetBounds(gfx::Rect(GetPosition(), size), animate);
 }
@@ -525,8 +521,23 @@ void NativeWindow::NotifyWindowCloseButtonClicked() {
   CloseImmediately();
 }
 
+void NativeWindow::Close() {
+  if (!IsClosable()) {
+    WindowList::WindowCloseCancelled(this);
+    return;
+  }
+
+  if (!is_closed())
+    CloseImpl();
+}
+
+void NativeWindow::CloseImmediately() {
+  if (!is_closed())
+    CloseImmediatelyImpl();
+}
+
 void NativeWindow::NotifyWindowClosed() {
-  if (is_closed_)
+  if (is_closed())
     return;
 
   is_closed_ = true;

shell/browser/native_window.h

@@ -82,9 +82,10 @@ class NativeWindow : public base::SupportsUserData,
 
   virtual void SetContentView(views::View* view) = 0;
 
-  virtual void Close() = 0;
+  // wrapper around CloseImpl that checks that window_ can be closed
-  virtual void CloseImmediately() = 0;
+  void Close();
-  virtual bool IsClosed() const;
+  // wrapper around CloseImmediatelyImpl that checks that window_ can be closed
+  void CloseImmediately();
   virtual void Focus(bool focus) = 0;
   virtual bool IsFocused() const = 0;
   virtual void Show() = 0;
@@ -435,6 +436,8 @@ class NativeWindow : public base::SupportsUserData,
  protected:
   friend class api::BrowserView;
 
+  [[nodiscard]] constexpr bool is_closed() const { return is_closed_; }
+
   NativeWindow(const gin_helper::Dictionary& options, NativeWindow* parent);
 
   // views::WidgetDelegate:
@@ -444,6 +447,9 @@ class NativeWindow : public base::SupportsUserData,
 
   void set_content_view(views::View* view) { content_view_ = view; }
 
+  virtual void CloseImpl() = 0;
+  virtual void CloseImmediatelyImpl() = 0;
+
   // The boolean parsing of the "titleBarOverlay" option
   bool titlebar_overlay_ = false;
 

shell/browser/native_window_mac.h

@@ -38,8 +38,8 @@ class NativeWindowMac : public NativeWindow,
 
   // NativeWindow:
   void SetContentView(views::View* view) override;
-  void Close() override;
+  void CloseImpl() override;
-  void CloseImmediately() override;
+  void CloseImmediatelyImpl() override;
   void Focus(bool focus) override;
   bool IsFocused() const override;
   void Show() override;

shell/browser/native_window_mac.mm

@@ -33,7 +33,6 @@
 #include "shell/browser/ui/cocoa/root_view_mac.h"
 #include "shell/browser/ui/cocoa/window_buttons_proxy.h"
 #include "shell/browser/ui/drag_util.h"
-#include "shell/browser/window_list.h"
 #include "shell/common/gin_converters/gfx_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_util.h"
@@ -331,12 +330,7 @@ void NativeWindowMac::SetContentView(views::View* view) {
   root_view->DeprecatedLayoutImmediately();
 }
 
-void NativeWindowMac::Close() {
+void NativeWindowMac::CloseImpl() {
-  if (!IsClosable()) {
-    WindowList::WindowCloseCancelled(this);
-    return;
-  }
-
   if (fullscreen_transition_state() != FullScreenTransitionState::kNone) {
     SetHasDeferredWindowClose(true);
     return;
@@ -372,7 +366,7 @@ void NativeWindowMac::Close() {
   }
 }
 
-void NativeWindowMac::CloseImmediately() {
+void NativeWindowMac::CloseImmediatelyImpl() {
   // Ensure we're detached from the parent window before closing.
   RemoveChildFromParentWindow();
 
@@ -1692,7 +1686,7 @@ bool NativeWindowMac::IsActive() const {
 }
 
 void NativeWindowMac::Cleanup() {
-  DCHECK(!IsClosed());
+  DCHECK(!is_closed());
   ui::NativeTheme::GetInstanceForNativeUi()->RemoveObserver(this);
   display::Screen::GetScreen()->RemoveObserver(this);
   [window_ cleanup];

shell/browser/native_window_views.cc

@@ -31,7 +31,6 @@
 #include "shell/browser/ui/views/root_view.h"
 #include "shell/browser/web_contents_preferences.h"
 #include "shell/browser/web_view_manager.h"
-#include "shell/browser/window_list.h"
 #include "shell/common/electron_constants.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
@@ -473,16 +472,11 @@ void NativeWindowViews::SetContentView(views::View* view) {
   root_view_.GetMainView()->DeprecatedLayoutImmediately();
 }
 
-void NativeWindowViews::Close() {
+void NativeWindowViews::CloseImpl() {
-  if (!IsClosable()) {
-    WindowList::WindowCloseCancelled(this);
-    return;
-  }
-
   widget()->Close();
 }
 
-void NativeWindowViews::CloseImmediately() {
+void NativeWindowViews::CloseImmediatelyImpl() {
   widget()->CloseNow();
 }
 

shell/browser/native_window_views.h

@@ -48,8 +48,8 @@ class NativeWindowViews : public NativeWindow,
 
   // NativeWindow:
   void SetContentView(views::View* view) override;
-  void Close() override;
+  void CloseImpl() override;
-  void CloseImmediately() override;
+  void CloseImmediatelyImpl() override;
   void Focus(bool focus) override;
   bool IsFocused() const override;
   void Show() override;

shell/browser/window_list.cc

@@ -84,7 +84,7 @@ void WindowList::CloseAllWindows() {
   std::ranges::reverse(weak_windows);
 #endif
   for (const auto& window : weak_windows) {
-    if (window && !window->IsClosed())
+    if (window)
       window->Close();
   }
 }
@@ -95,7 +95,7 @@ void WindowList::DestroyAllWindows() {
       ConvertToWeakPtrVector(GetInstance()->windows_);
 
   for (const auto& window : weak_windows) {
-    if (window && !window->IsClosed())
+    if (window)
       window->CloseImmediately();
   }
 }