Skip to content

Commit

Permalink
[M106] mac: patch only chrome-initiated -[NSWindow orderWindow:]
Browse files Browse the repository at this point in the history
Patching system-initiated -[NSWindow orderWindow:] calls has caused
issues in fullscreen. For example, this function will be called when a
sheet modal is created and the sheet will appear to be occluded by other
windows.

This CL patches only chrome-initiated calls instead.

I couldn't find out a reliable way to add regression test.
Neither -[NSWindow orderedIndex] or -[NSWindow childWindows] is
reliable for testing sheet window that is attached to a child window.

(cherry picked from commit bd812e5)

Bug: 1352634, 1324216
Change-Id: I3ee43bae3c248da433f486c6444fda55b5b7121b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3842864
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1037901}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3854876
Cr-Commit-Position: refs/branch-heads/5249@{#75}
Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}
  • Loading branch information
naeioi authored and Chromium LUCI CQ committed Aug 24, 2022
1 parent fda8f7f commit 64926cb
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 30 deletions.
5 changes: 5 additions & 0 deletions components/remote_cocoa/app_shim/native_widget_mac_nswindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ REMOTE_COCOA_APP_SHIM_EXPORT
// https://crbug.com/960904
- (void)enforceNeverMadeVisible;

// Order window for all cases, including for children windows that
// -[NSWindow orderWindow:] can't properly handle.
- (void)reallyOrderWindow:(NSWindowOrderingMode)place
relativeTo:(NSInteger)otherWin;

// Order the window to the front (space switch if necessary), and ensure that
// the window maintains its key state. A space switch will normally activate a
// window, so this function prevents that if the window is currently inactive.
Expand Down
48 changes: 20 additions & 28 deletions components/remote_cocoa/app_shim/native_widget_mac_nswindow.mm
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ @implementation NativeWidgetMacNSWindow {
BOOL _willUpdateRestorableState;
BOOL _isEnforcingNeverMadeVisible;
BOOL _preventKeyWindow;
BOOL _isAddingChildWindow;
BOOL _isTooltip;
}
@synthesize bridgedNativeWidgetId = _bridgedNativeWidgetId;
Expand All @@ -164,7 +163,6 @@ - (instancetype)initWithContentRect:(NSRect)contentRect
defer:deferCreation])) {
_commandDispatcher.reset([[CommandDispatcher alloc] initWithOwner:self]);
}
_isAddingChildWindow = NO;
return self;
}

Expand All @@ -181,7 +179,6 @@ - (void)dealloc {
}

- (void)addChildWindow:(NSWindow*)childWin ordered:(NSWindowOrderingMode)place {
base::AutoReset<BOOL> isAddingChildWindow(&_isAddingChildWindow, YES);
// Attaching a window to be a child window resets the window level, so
// restore the window level afterwards.
NSInteger level = childWin.level;
Expand Down Expand Up @@ -404,44 +401,39 @@ - (void)sendEvent:(NSEvent*)event {
[super sendEvent:event];
}

// Override window order functions to
// 1. Intercept other visibility changes
// This is needed in addition to the -[NSWindow display] override because
// Cocoa hardly ever calls display, and reports -[NSWindow isVisible]
// incorrectly when ordering in a window for the first time.
// 2. Handle child windows
// -[NSWindow orderWindow] does not work for child windows. To order
// children, we need to detach them from thier parent and re-attach them
// in our desire order.
- (void)orderWindow:(NSWindowOrderingMode)orderingMode
relativeTo:(NSInteger)otherWindowNumber {
- (void)reallyOrderWindow:(NSWindowOrderingMode)orderingMode
relativeTo:(NSInteger)otherWindowNumber {
NativeWidgetMacNSWindow* parent =
static_cast<NativeWidgetMacNSWindow*>([self parentWindow]);
// Cocoa will call -[NSWindow orderWindow] during -[NSWindow addChildWindow].
// To prevent re-entrancy, skip re-parenting if adding children.
if (parent && parent->_isAddingChildWindow) {
[super orderWindow:orderingMode relativeTo:otherWindowNumber];
[[self viewsNSWindowDelegate] onWindowOrderChanged:nil];

// This is not a child window. No need to patch.
if (!parent) {
[self orderWindow:orderingMode relativeTo:otherWindowNumber];
return;
}

// `otherWindow` is nil if `otherWindowNumber` is 0. In this case, place
// `self` at the top / bottom, depending on `orderingMode`.
NSWindow* otherWindow = [NSApp windowWithWindowNumber:otherWindowNumber];

// For unknown reason chrome will freeze during startup without this line.
[super orderWindow:orderingMode relativeTo:otherWindowNumber];

// During shutdown the parent may have changed at this point, so reacquire
// `parent`.
parent = static_cast<NativeWidgetMacNSWindow*>([self parentWindow]);
if (parent && (otherWindow == nullptr ||
parent == [otherWindow parentWindow] || parent == otherWindow))
if (otherWindow == nullptr || parent == [otherWindow parentWindow] ||
parent == otherWindow)
OrderChildWindow(self, otherWindow, orderingMode);

[[self viewsNSWindowDelegate] onWindowOrderChanged:nil];
}

// Override window order functions to intercept other visibility changes. This
// is needed in addition to the -[NSWindow display] override because Cocoa
// hardly ever calls display, and reports -[NSWindow isVisible] incorrectly
// when ordering in a window for the first time.
// Note that this methods has no effect for children windows. Use
// -reallyOrderWindow:relativeTo: instead.
- (void)orderWindow:(NSWindowOrderingMode)orderingMode
relativeTo:(NSInteger)otherWindowNumber {
[super orderWindow:orderingMode relativeTo:otherWindowNumber];
[[self viewsNSWindowDelegate] onWindowOrderChanged:nil];
}

// NSResponder implementation.

- (BOOL)performKeyEquivalent:(NSEvent*)event {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,11 @@ NSUInteger CountBridgedWindows(NSArray* child_windows) {
DCHECK(sibling_bridge);

NSInteger sibling = sibling_bridge->ns_window().windowNumber;
[window_ orderWindow:NSWindowAbove relativeTo:sibling];
[window_ reallyOrderWindow:NSWindowAbove relativeTo:sibling];
}

void NativeWidgetNSWindowBridge::StackAtTop() {
[window_ orderWindow:NSWindowAbove relativeTo:0];
[window_ reallyOrderWindow:NSWindowAbove relativeTo:0];
}

void NativeWidgetNSWindowBridge::ShowEmojiPanel() {
Expand Down

0 comments on commit 64926cb

Please sign in to comment.