[MacViews] Use NSTitledWindowMask for window modal dialogs.

[MacViews] Use NSTitledWindowMask for window modal dialogs.

-[NSApplication beginSheet:modalForWindow:...] doesn't seem to make the
sheet modal if it has NSBorderlessWindowMask.

Use NSTitledWindowMask for window modal widgets. The title is not drawn
on sheets anyway.

BUG=522359
Committed: https://crrev.com/4fd831b9c04682e0df42d4e6dba2155f0982312b
Cr-Commit-Position: refs/heads/master@{#344421}

[jackhou1, 2015-08-19 07:17:14]

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

[jackhou1, 2015-08-19 07:17:14]

tapted, PTAL

[tapted, 2015-08-19 09:02:14]

Nice find! I think we just need to check for null.. (hopefully an existing test
picks that up :p)

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:41: // If the widget is modal, it will be
displayed as a sheet. This only works
nit: widget -> Widget
(and perhaps "only works" -> "works best")

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:42: // if it has NSTitledWindowMask.
nit: perhaps worth adding "(e.g. with NSBorderlessWindowMask. the parent window
still accepts input)."

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:43: if (params.delegate->GetModalType() ==
ui::MODAL_TYPE_WINDOW)
params.delegate && ... 

(params.delegate can be null)

[jackhou1, 2015-08-20 00:01:36]

> Nice find! I think we just need to check for null.. (hopefully an existing
test
> picks that up :p)

Ah of course. (Yeah about 245 views_unittests =P).

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:41: // If the widget is modal, it will be
displayed as a sheet. This only works
On 2015/08/19 09:02:14, tapted wrote:
> nit: widget -> Widget
> (and perhaps "only works" -> "works best")

Done.

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:42: // if it has NSTitledWindowMask.
On 2015/08/19 09:02:14, tapted wrote:
> nit: perhaps worth adding "(e.g. with NSBorderlessWindowMask. the parent
window
> still accepts input)."

Done.

https://codereview.chromium.org/1291763005/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:43: if (params.delegate->GetModalType() ==
ui::MODAL_TYPE_WINDOW)
On 2015/08/19 09:02:14, tapted wrote:
> params.delegate && ... 
> 
> (params.delegate can be null)

Done.

[tapted, 2015-08-20 00:23:10]

lgtm

(+ a description nit: "modal" -> "window modal" in the summary lines)

Thanks!

https://codereview.chromium.org/1291763005/diff/20001/ui/views/widget/native_...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1291763005/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_mac.mm:42: // it has NSTitledWindowMask. (e.g.
with NSBorderlessWindowMask. the parent
nit (sorry - I gave you bad text :p): Grammar pedant says this should be one of:

...it has NSTitledWindowMask (e.g., with NSBorderlessWindowMask, the parent
window still accepts input).

or

...it has NSTitledWindowMask. For example, with NSBorderlessWindowMask, the
parent window still accepts input.

[jackhou1, 2015-08-20 00:37:17]

https://codereview.chromium.org/1291763005/diff/20001/ui/views/widget/native_...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/1291763005/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_mac.mm:42: // it has NSTitledWindowMask. (e.g.
with NSBorderlessWindowMask. the parent
On 2015/08/20 00:23:09, tapted wrote:
> nit (sorry - I gave you bad text :p): Grammar pedant says this should be one
of:
> 
> ...it has NSTitledWindowMask (e.g., with NSBorderlessWindowMask, the parent
> window still accepts input).
> 
> or
> 
> ...it has NSTitledWindowMask. For example, with NSBorderlessWindowMask, the
> parent window still accepts input.

Done. (Sorry I should have checked it).

[jackhou1, 2015-08-20 03:45:04]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2015-08-20 03:45:05]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/1291763005/#ps40001
(title: "Fix comment.")

[commit-bot: I haz the power, 2015-08-20 03:45:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291763005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291763005/40001

[commit-bot: I haz the power, 2015-08-20 03:49:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-20 03:50:40]

Patchset 3 (id:??) landed as
https://crrev.com/4fd831b9c04682e0df42d4e6dba2155f0982312b
Cr-Commit-Position: refs/heads/master@{#344421}