Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unsubscribe from observers when window is closing #25584

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions shell/browser/native_window_mac.h
Expand Up @@ -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;
Expand Down
15 changes: 5 additions & 10 deletions shell/browser/native_window_mac.mm
Expand Up @@ -536,10 +536,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() {
Expand Down Expand Up @@ -635,13 +637,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.
Expand Down
1 change: 1 addition & 0 deletions shell/browser/ui/cocoa/electron_ns_window_delegate.mm
Expand Up @@ -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
Expand Down