fix: unsubscribe from observers when window is closing (#25586)

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
electron · Sep 23, 2020 · 6c3c9b5 · 6c3c9b5
1 parent a43d4ca
commit 6c3c9b5
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 10 deletions.
diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h
@@ -32,6 +32,11 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   NativeWindowMac(const gin_helper::Dictionary& options, NativeWindow* parent);
   ~NativeWindowMac() override;
 
+  // Cleanup observers when window is getting closed. Note that the destructor
+  // can be called much later after window gets closed, so we should not do
+  // cleanup in destructor.
+  void Cleanup();
+
   // NativeWindow:
   void SetContentView(views::View* view) override;
   void Close() override;

diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm
@@ -513,10 +513,12 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   original_level_ = [window_ level];
 }
 
-NativeWindowMac::~NativeWindowMac() {
+NativeWindowMac::~NativeWindowMac() {}
+
+void NativeWindowMac::Cleanup() {
+  DCHECK(!IsClosed());
   ui::NativeTheme::GetInstanceForNativeUi()->RemoveObserver(this);
-  if (wheel_event_monitor_)
-    [NSEvent removeMonitor:wheel_event_monitor_];
+  [NSEvent removeMonitor:wheel_event_monitor_];
 }
 
 void NativeWindowMac::RedrawTrafficLights() {
@@ -612,13 +614,6 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
 }
 
 void NativeWindowMac::CloseImmediately() {
-  // Remove event monitor before destroying window, otherwise the monitor may
-  // call its callback after window has been destroyed.
-  if (wheel_event_monitor_) {
-    [NSEvent removeMonitor:wheel_event_monitor_];
-    wheel_event_monitor_ = nil;
-  }
-
   // Retain the child window before closing it. If the last reference to the
   // NSWindow goes away inside -[NSWindow close], then bad stuff can happen.
   // See e.g. http://crbug.com/616701.

diff --git a/shell/browser/ui/cocoa/electron_ns_window_delegate.mm b/shell/browser/ui/cocoa/electron_ns_window_delegate.mm
@@ -283,6 +283,7 @@ - (void)windowDidExitFullScreen:(NSNotification*)notification {
 }
 
 - (void)windowWillClose:(NSNotification*)notification {
+  shell_->Cleanup();
   shell_->NotifyWindowClosed();
 
   // Something called -[NSWindow close] on a sheet rather than calling