Window levels don't work in MacViews |
||
Issue descriptionWorking on bug 865287, I tracked down a window level issue I was facing to NativeWidgetMac::InitNativeWidget(). If the |keep_on_top| param is set, then SetNSWindowAlwaysOnTop is called. https://cs.chromium.org/search/?q=SetNSWindowAlwaysOnTop That sets the window level to NSFloatingWindowLevel (3). OK. But if we look at BridgedNativeWidget::SetVisibilityState(), and add logging to the ui::SHOW_STATE_INACTIVE case: NSLog(@"showing inactive window %@ with level %ld", window_.get(), [window_ level]); [window_ orderWindow:NSWindowAbove relativeTo:parent_window_number]; NSLog(@"did show inactive window %@ with level %ld", window_.get(), [window_ level]); we see as log output: 2018-07-31 13:21:28.842 Chromium[56148:11388635] showing inactive window <NativeWidgetMacNSWindow: 0x7fbac59911b0> with level 3 2018-07-31 13:21:28.842 Chromium[56148:11388635] did show inactive window <NativeWidgetMacNSWindow: 0x7fbac59911b0> with level 0 If we drop a breakpoint in lldb on AppKit`-[NSWindow setLevel:] we get: * frame #0: 0x00007fff87540fb6 AppKit`-[NSWindow setLevel:] frame #1: 0x00007fff87540d44 AppKit`-[NSWindow addChildWindow:ordered:] + 649 frame #2: 0x000000013c27f3be libviews.dylib`views::BridgedNativeWidget::OnVisibilityChanged(this=0x0000000171a02860) + 270 at bridged_native_widget.mm:800 This seems to hit a fundamental conflict between how Views manages windows (oops, widgets) and Cocoa. Views uses window parent relationships to handle ownership, but in Cocoa, child windows must have the same window level as their parent window. I'm not sure how to reconcile this.
,
Jul 31
To clarify that last backtrace:
* frame #0: 0x00007fff87540fb6 AppKit`-[NSWindow setLevel:]
frame #1: 0x00007fff87540d44 AppKit`-[NSWindow addChildWindow:ordered:] + 649
frame #2: 0x000000013c27f3be libviews.dylib`views::BridgedNativeWidget::OnVisibilityChanged(this=0x0000000171a02860) + 270 at bridged_native_widget.mm:800
frame #3: 0x000000013c297b98 libviews.dylib`::-[ViewsNSWindowDelegate onWindowOrderChanged:](self=0x000000016260bfc0, _cmd="onWindowOrderChanged:", notification=0x0000000000000000) + 40 at views_nswindow_delegate.mm:74
frame #4: 0x000000013c29652b libviews.dylib`::-[NativeWidgetMacNSWindow orderWindow:relativeTo:](self=0x0000000171a02ab0, _cmd="orderWindow:relativeTo:", orderingMode=NSWindowAbove, otherWindowNumber=5518) + 123 at native_widget_mac_nswindow.mm:229
frame #5: 0x000000013c27c424 libviews.dylib`views::BridgedNativeWidget::SetVisibilityState(this=0x0000000171a02860, new_state=SHOW_INACTIVE) + 1364 at bridged_native_widget.mm:547
frame #6: 0x000000013c4109d6 libviews.dylib`views::NativeWidgetMac::ShowWithWindowState(this=0x0000000162668a20, state=SHOW_STATE_INACTIVE) + 790 at native_widget_mac.mm:470
BridgedNativeWidget::SetVisibilityState() → -[NativeWidgetMacNSWindow orderWindow:relativeTo:] → -[ViewsNSWindowDelegate onWindowOrderChanged:] → BridgedNativeWidget::OnVisibilityChanged() → -[NSWindow addChildWindow:ordered:] → -[NSWindow setLevel:]
The same issue with child windows vs layers remains.
,
Aug 1
Yeah.. I encountered this in https://chromium-review.googlesource.com/c/chromium/src/+/923227 too.. But note that MacViews doesn't use the native parent/child window properties for lifetime management (it can't coz it breaks when stuff is ordered out). So simply skipping the addChildWindow step if something has a higher window level than the parent is probably Just Fine and has the same window-ordering guarantees. The caveat is that is does _not_ have the same moves-with-parent-window guarantees. For menus that shouldn't matter. For other bubbles that care, they likely already have redundant "watch parent-move and mimc" logic since: - resizing the top corners of a window need to move these bubbles anyway - moves done by programatic serFrame calls need to be watched regardless - other platforms don't have this mimic-move stuff Where it might break down is when a window moves to a new space. I'm pretty sure the parentWindow goop is what makes persistent bubbles (e.g. permissions requests) teleport with the window into the new space.
,
Aug 1
Cutting the parent/child relationship works for menus. Can we establish rules of when this happens?
,
Aug 1
BTW, re SetAlwaysOnTop, that's such a disaster. It's called for menu and drag window types, but baking the Windows notion of only having two window levels into Views when on the Mac there are UINT64_MAX window levels is just ... ugh.
,
Aug 1
Aura exposes an entire layer tree.. see subclasses of WindowStackingClient. That results in fun bugs like Issue 848327 (/me hides). So a "level" is actually a pointer into the tree that has an array of other windows at your level, which you can be at the top of or the bottom of. But aura is a window manager. Likely the macOS WindowServer process has something equivalent internally that it presents a simplified API to via window levels. I think a map from WidgetDelegate/Widget::InitParams::Type to 'bool' => "should I maintain [parent addChildWindow]" is reasonable. E.g. window modal sheets already opt out. For now, I'd say just those and menus until there's a strong use case for something else.
,
Aug 1
That makes sense. I'm still trying to work out what to do with SetAlwaysOnTop though. Widget::InitParams::InitParams has keep_on_top(type == TYPE_MENU || type == TYPE_DRAG) and by the time our code processes SetAlwaysOnTop it has no idea what window level to use. Menu? Drag? Generic floating window? The only way I can see out of this is to make keep_on_top an enum for a few different window levels for our map, including a generic one for things like extensions (this is exposed to extensions!) and changing things so that on other platforms any of them is "on top" but we have the data on the Mac to pick the right value. This seems a bit down my priority list tho.
,
Aug 3
Cross-posting from email: it looks like window levels can work for child windows, you just need to set the level *after* adding the window as a child. Fun stuff.
,
Aug 24
|
||
►
Sign in to add a comment |
||
Comment 1 by a...@chromium.org
, Jul 31