diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index e8237460dfbf..15e00717bf57 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -157,6 +157,7 @@ class NativeWindowMac : public NativeWindow, bool IsActive() const override; // Remove the specified child window without closing it. void RemoveChildWindow(NativeWindow* child) override; + void RemoveChildFromParentWindow(NativeWindow* child); // Attach child windows, if the window is visible. void AttachChildren() override; // Detach window from parent without destroying it. diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 86f8188c5f70..5dc6bf0db7e3 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -388,6 +388,9 @@ void NativeWindowMac::Close() { return; } + // Ensure we're detached from the parent window before closing. + RemoveChildFromParentWindow(this); + // If a sheet is attached to the window when we call // [window_ performClose:nil], the window won't close properly // even after the user has ended the sheet. @@ -405,6 +408,8 @@ void NativeWindowMac::Close() { } void NativeWindowMac::CloseImmediately() { + RemoveChildFromParentWindow(this); + // 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. @@ -598,6 +603,11 @@ void NativeWindowMac::RemoveChildWindow(NativeWindow* child) { [window_ removeChildWindow:child->GetNativeWindow().GetNativeNSWindow()]; } +void NativeWindowMac::RemoveChildFromParentWindow(NativeWindow* child) { + if (parent()) + parent()->RemoveChildWindow(child); +} + void NativeWindowMac::AttachChildren() { for (auto* child : child_windows_) { auto* child_nswindow = child->GetNativeWindow().GetNativeNSWindow(); @@ -1781,27 +1791,27 @@ void NativeWindowMac::InternalSetWindowButtonVisibility(bool visible) { [[window_ standardWindowButton:NSWindowZoomButton] setHidden:!visible]; } -void NativeWindowMac::InternalSetParentWindow(NativeWindow* parent, +void NativeWindowMac::InternalSetParentWindow(NativeWindow* new_parent, bool attach) { if (is_modal()) return; - NativeWindow::SetParentWindow(parent); - // Do not remove/add if we are already properly attached. - if (attach && parent && - [window_ parentWindow] == parent->GetNativeWindow().GetNativeNSWindow()) + if (attach && new_parent && + [window_ parentWindow] == + new_parent->GetNativeWindow().GetNativeNSWindow()) return; // Remove current parent window. - if ([window_ parentWindow]) - parent->RemoveChildWindow(this); + RemoveChildFromParentWindow(this); // Set new parent window. - if (parent && attach) { - parent->add_child_window(this); - parent->AttachChildren(); + if (new_parent && attach) { + new_parent->add_child_window(this); + new_parent->AttachChildren(); } + + NativeWindow::SetParentWindow(new_parent); } void NativeWindowMac::SetForwardMouseMessages(bool forward) { diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts index 450e503f5c58..8fb8b50598c2 100644 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -4677,6 +4677,21 @@ describe('BrowserWindow module', () => { expect(w.getChildWindows().length).to.equal(0); }); + it('can handle child window close and reparent multiple times', async () => { + const w = new BrowserWindow({ show: false }); + let c: BrowserWindow | null; + + for (let i = 0; i < 5; i++) { + c = new BrowserWindow({ show: false, parent: w }); + const closed = once(c, 'closed'); + c.close(); + await closed; + } + + await setTimeout(); + expect(w.getChildWindows().length).to.equal(0); + }); + ifit(process.platform === 'darwin')('child window matches visibility when visibility changes', async () => { const w = new BrowserWindow({ show: false }); const c = new BrowserWindow({ show: false, parent: w });