fix: reparenting UAF crash on macOS (#38603)
This commit is contained in:
parent
9a9d8ae5ea
commit
5ee890fb6f
3 changed files with 36 additions and 10 deletions
|
@ -157,6 +157,7 @@ class NativeWindowMac : public NativeWindow,
|
||||||
bool IsActive() const override;
|
bool IsActive() const override;
|
||||||
// Remove the specified child window without closing it.
|
// Remove the specified child window without closing it.
|
||||||
void RemoveChildWindow(NativeWindow* child) override;
|
void RemoveChildWindow(NativeWindow* child) override;
|
||||||
|
void RemoveChildFromParentWindow(NativeWindow* child);
|
||||||
// Attach child windows, if the window is visible.
|
// Attach child windows, if the window is visible.
|
||||||
void AttachChildren() override;
|
void AttachChildren() override;
|
||||||
// Detach window from parent without destroying it.
|
// Detach window from parent without destroying it.
|
||||||
|
|
|
@ -388,6 +388,9 @@ void NativeWindowMac::Close() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Ensure we're detached from the parent window before closing.
|
||||||
|
RemoveChildFromParentWindow(this);
|
||||||
|
|
||||||
// If a sheet is attached to the window when we call
|
// If a sheet is attached to the window when we call
|
||||||
// [window_ performClose:nil], the window won't close properly
|
// [window_ performClose:nil], the window won't close properly
|
||||||
// even after the user has ended the sheet.
|
// even after the user has ended the sheet.
|
||||||
|
@ -405,6 +408,8 @@ void NativeWindowMac::Close() {
|
||||||
}
|
}
|
||||||
|
|
||||||
void NativeWindowMac::CloseImmediately() {
|
void NativeWindowMac::CloseImmediately() {
|
||||||
|
RemoveChildFromParentWindow(this);
|
||||||
|
|
||||||
// Retain the child window before closing it. If the last reference to the
|
// Retain the child window before closing it. If the last reference to the
|
||||||
// NSWindow goes away inside -[NSWindow close], then bad stuff can happen.
|
// NSWindow goes away inside -[NSWindow close], then bad stuff can happen.
|
||||||
// See e.g. http://crbug.com/616701.
|
// See e.g. http://crbug.com/616701.
|
||||||
|
@ -598,6 +603,11 @@ void NativeWindowMac::RemoveChildWindow(NativeWindow* child) {
|
||||||
[window_ removeChildWindow:child->GetNativeWindow().GetNativeNSWindow()];
|
[window_ removeChildWindow:child->GetNativeWindow().GetNativeNSWindow()];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void NativeWindowMac::RemoveChildFromParentWindow(NativeWindow* child) {
|
||||||
|
if (parent())
|
||||||
|
parent()->RemoveChildWindow(child);
|
||||||
|
}
|
||||||
|
|
||||||
void NativeWindowMac::AttachChildren() {
|
void NativeWindowMac::AttachChildren() {
|
||||||
for (auto* child : child_windows_) {
|
for (auto* child : child_windows_) {
|
||||||
auto* child_nswindow = child->GetNativeWindow().GetNativeNSWindow();
|
auto* child_nswindow = child->GetNativeWindow().GetNativeNSWindow();
|
||||||
|
@ -1781,27 +1791,27 @@ void NativeWindowMac::InternalSetWindowButtonVisibility(bool visible) {
|
||||||
[[window_ standardWindowButton:NSWindowZoomButton] setHidden:!visible];
|
[[window_ standardWindowButton:NSWindowZoomButton] setHidden:!visible];
|
||||||
}
|
}
|
||||||
|
|
||||||
void NativeWindowMac::InternalSetParentWindow(NativeWindow* parent,
|
void NativeWindowMac::InternalSetParentWindow(NativeWindow* new_parent,
|
||||||
bool attach) {
|
bool attach) {
|
||||||
if (is_modal())
|
if (is_modal())
|
||||||
return;
|
return;
|
||||||
|
|
||||||
NativeWindow::SetParentWindow(parent);
|
|
||||||
|
|
||||||
// Do not remove/add if we are already properly attached.
|
// Do not remove/add if we are already properly attached.
|
||||||
if (attach && parent &&
|
if (attach && new_parent &&
|
||||||
[window_ parentWindow] == parent->GetNativeWindow().GetNativeNSWindow())
|
[window_ parentWindow] ==
|
||||||
|
new_parent->GetNativeWindow().GetNativeNSWindow())
|
||||||
return;
|
return;
|
||||||
|
|
||||||
// Remove current parent window.
|
// Remove current parent window.
|
||||||
if ([window_ parentWindow])
|
RemoveChildFromParentWindow(this);
|
||||||
parent->RemoveChildWindow(this);
|
|
||||||
|
|
||||||
// Set new parent window.
|
// Set new parent window.
|
||||||
if (parent && attach) {
|
if (new_parent && attach) {
|
||||||
parent->add_child_window(this);
|
new_parent->add_child_window(this);
|
||||||
parent->AttachChildren();
|
new_parent->AttachChildren();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
NativeWindow::SetParentWindow(new_parent);
|
||||||
}
|
}
|
||||||
|
|
||||||
void NativeWindowMac::SetForwardMouseMessages(bool forward) {
|
void NativeWindowMac::SetForwardMouseMessages(bool forward) {
|
||||||
|
|
|
@ -4677,6 +4677,21 @@ describe('BrowserWindow module', () => {
|
||||||
expect(w.getChildWindows().length).to.equal(0);
|
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 () => {
|
ifit(process.platform === 'darwin')('child window matches visibility when visibility changes', async () => {
|
||||||
const w = new BrowserWindow({ show: false });
|
const w = new BrowserWindow({ show: false });
|
||||||
const c = new BrowserWindow({ show: false, parent: w });
|
const c = new BrowserWindow({ show: false, parent: w });
|
||||||
|
|
Loading…
Reference in a new issue