Mac: Fix window ordering for certificate viewer/selector sheets and overlays.

Mac: Fix window ordering for certificate viewer/selector sheets and overlays.

Currently, the macOS-provided native dialogs for the certificate viewer and the
certificate chooser both use an invisible overlay window to make them tab-modal
instead of window modal (which is the default). If two of these dialogs with
invisible overlays are open at the same time, the overlay window order can get
mixed up and the overlay window ends up on top of the native dialog.

For the MacViews certificate viewer especially, the dialog itself and its
corresponding overlay can get incorrect window ordering, where the overlay ends
up in front of the dialog. Both these situations will render the native dialog
unclickable because the overlay is intercepting mouse events.

To fix, adjust the window order manually every time the overlay and dialog are
hidden/shown.

BUG=671150, 650898

[Patti Lor, 2016-12-09 02:44:32]

Description was changed from

==========
Mac: Fix window ordering for certificate viewer/selector sheets and overlays.

Currently, the macOS-provided native dialogs for the certificate viewer and the
certificate chooser both use an invisible overlay window to make them tab-modal
instead of window modal (which is the default). If two of these dialogs with
invisible overlays are open at the same time, the overlay window order can get
mixed up and the overlay window ends up on top of the native dialog.

For the MacViews certificate viewer especially, the dialog itself and its
corresponding overlay can get incorrect window ordering, where the overlay ends
up in front of the dialog. Both these situations will render the native dialog
unclickable because the overlay is intercepting mouse events.

To fix, adjust the window order manually every time the overlay and dialog are
hidden/shown.

TODO: Try all the repro steps in the first/last bug.

BUG=671150, 650898, 630225
==========

to

==========
Mac: Fix window ordering for certificate viewer/selector sheets and overlays.

Currently, the macOS-provided native dialogs for the certificate viewer and the
certificate chooser both use an invisible overlay window to make them tab-modal
instead of window modal (which is the default). If two of these dialogs with
invisible overlays are open at the same time, the overlay window order can get
mixed up and the overlay window ends up on top of the native dialog.

For the MacViews certificate viewer especially, the dialog itself and its
corresponding overlay can get incorrect window ordering, where the overlay ends
up in front of the dialog. Both these situations will render the native dialog
unclickable because the overlay is intercepting mouse events.

To fix, adjust the window order manually every time the overlay and dialog are
hidden/shown.

BUG=671150, 650898
==========

[Patti Lor, 2016-12-09 02:46:03]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 02:46:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 03:30:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 03:30:59]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2016-12-09 03:45:43]

patricialor@chromium.org changed reviewers:
+ tapted@chromium.org

[Patti Lor, 2016-12-09 03:45:44]

Hi Trent, PTAL. Thanks!

[tapted, 2016-12-09 04:02:00]

https://codereview.chromium.org/2562653002/diff/1/chrome/browser/ui/cocoa/ssl...
File chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm (right):

https://codereview.chromium.org/2562653002/diff/1/chrome/browser/ui/cocoa/ssl...
chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm:291:
[overlayWindow_ setLevel:0];
We should be passing an NSFooWindowLevel to setLevel. E.g. NSNormalWindowLevel,
NSFloatingWindowLevel, etc. There's a `CGWindowLevel.h` which uses the same
constants -- I don't know whether 0/1 will do the right thing  (NSFooWindowLevel
is actually a macro that results in a function call returning some unknown
integer). But! I don't think setLevel is right.

Can we use NSWindow's

- (void)orderWindow:(NSWindowOrderingMode)place 
         relativeTo:(NSInteger)otherWin;

instead?

[Patti Lor, 2016-12-09 05:02:49]

https://codereview.chromium.org/2562653002/diff/1/chrome/browser/ui/cocoa/ssl...
File chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm (right):

https://codereview.chromium.org/2562653002/diff/1/chrome/browser/ui/cocoa/ssl...
chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm:291:
[overlayWindow_ setLevel:0];
On 2016/12/09 04:02:00, tapted wrote:
> We should be passing an NSFooWindowLevel to setLevel. E.g.
NSNormalWindowLevel,
> NSFloatingWindowLevel, etc. There's a `CGWindowLevel.h` which uses the same
> constants -- I don't know whether 0/1 will do the right thing 
(NSFooWindowLevel
> is actually a macro that results in a function call returning some unknown
> integer). But! I don't think setLevel is right.
> 
> Can we use NSWindow's
> 
> - (void)orderWindow:(NSWindowOrderingMode)place 
>          relativeTo:(NSInteger)otherWin;
> 
> instead?

Yeah, I considered this. It adds some complexity because the child windows will
need to be retrieved via the parent window, and there always seems to be 1
mysterious child window (the count is always # of overlay windows + 1). I don't
think it works either, I tried just looping through |overlayWindow_|'s parent's
child windows (excluding overlayWindow_) and calling orderWindow:relativeTo: on
them (passing in [child {level,orderedLevel,windowNumber}], not sure which is
the right one), and it still has the buggy behaviour.

[Patti Lor, 2016-12-09 06:29:43]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 06:30:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 06:47:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 06:47:45]

Dry run: This issue passed the CQ dry run.

[tapted, 2016-12-11 23:28:20]

https://codereview.chromium.org/2562653002/diff/20001/components/constrained_...
File
components/constrained_window/native_web_contents_modal_dialog_manager_views_mac.mm
(right):

https://codereview.chromium.org/2562653002/diff/20001/components/constrained_...
components/constrained_window/native_web_contents_modal_dialog_manager_views_mac.mm:35:
[overlay setLevel:visible ? NSFloatingWindowLevel : NSNormalWindowLevel];
I think NSFloatingWindowLevel would mean that dragging another window over the
overlay would cause the overlay window to remain floating above it. (maybe
sheets and parent/child relationships alter this behaviour, but that's normally
what setLevel is for, so I don't think we want it here).

I have a theory about what's happening. When there are two overlay windows
attached, AppKit has multiple childWindows to keep "on top" of the browser
window. But it might be using the order that they were added to childWindows to
decide how they should be ordered amongst themselves.

When showing, can you try something like

NSWindow* parent = [overlay parentWindow];
[parent removeChildWindow:parent];
[parent addChildWindow:overlay];

That might insert the window at the end, to ensure AppKit always puts the most
recently shown one on top.

[Patti Lor, 2016-12-12 00:32:56]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 00:33:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-12 01:00:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 01:00:37]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2016-12-12 01:29:04]

Thanks Trent, PTAL! Your theory was correct :)

https://codereview.chromium.org/2562653002/diff/20001/components/constrained_...
File
components/constrained_window/native_web_contents_modal_dialog_manager_views_mac.mm
(right):

https://codereview.chromium.org/2562653002/diff/20001/components/constrained_...
components/constrained_window/native_web_contents_modal_dialog_manager_views_mac.mm:35:
[overlay setLevel:visible ? NSFloatingWindowLevel : NSNormalWindowLevel];
On 2016/12/11 23:28:20, tapted wrote:
> I think NSFloatingWindowLevel would mean that dragging another window over the
> overlay would cause the overlay window to remain floating above it. (maybe
> sheets and parent/child relationships alter this behaviour, but that's
normally
> what setLevel is for, so I don't think we want it here).
> 
> I have a theory about what's happening. When there are two overlay windows
> attached, AppKit has multiple childWindows to keep "on top" of the browser
> window. But it might be using the order that they were added to childWindows
to
> decide how they should be ordered amongst themselves.
> 
> When showing, can you try something like
> 
> NSWindow* parent = [overlay parentWindow];
> [parent removeChildWindow:parent];
> [parent addChildWindow:overlay];
> 
> That might insert the window at the end, to ensure AppKit always puts the most
> recently shown one on top.

That works! Have done this for all three cases. All of them also required the
sheet to be reparented as well.