mac: Fix a bug where a fullscreened window on Mavericks would be too short.

mac: Fix a bug where a fullscreened window on Mavericks is too short.

There is a bug in Mavericks for applications linked against OSX 10.8 and
earlier. It requires Screens Have Separate Spaces to be enabled, and for the
window to be on a secondary screen. When AppKit Fullscreen is invoked on the
window, its final frame is 22pt too short.

This CL detects when these specific conditions occur so that the window's size
can be fixed.

Note: This is a bug buried in AppKit. Playing with methods like
-[NSWindowDelegate window:willUseFullScreenContentSize:] will not fix the
problem.

BUG=396980
Committed: https://crrev.com/3b432982d316ebdb553ff137a166521cbccc8b77
Cr-Commit-Position: refs/heads/master@{#300166}

[erikchen, 2014-10-16 00:44:44]

erikchen@chromium.org changed reviewers:
+ rsesek@chromium.org

[erikchen, 2014-10-16 00:44:44]

rsesek: Please review.

[Robert Sesek, 2014-10-17 19:13:16]

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.h:187: BOOL
enteringAppKitFullscreenOnPrimaryScreen_;
Move this to be after enteringPresentationMode_ instead.

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:161: // earlier. It
requires Screens Have Separate Spaces to be enabled, and for the
"It" is a vague reference here -- the bug requires?

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1082: if
(!base::mac::IsOSMavericks())
This issue is resolved on Yosemite?

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1085: ![NSScreen
screensHaveSeparateSpaces])
nit: any time the condition is multiple lines, the body needs braces {}

[erikchen, 2014-10-17 21:02:33]

rsesek: PTAL

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.h:187: BOOL
enteringAppKitFullscreenOnPrimaryScreen_;
On 2014/10/17 19:13:16, Robert Sesek wrote:
> Move this to be after enteringPresentationMode_ instead.

Done.

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:161: // earlier. It
requires Screens Have Separate Spaces to be enabled, and for the
On 2014/10/17 19:13:16, Robert Sesek wrote:
> "It" is a vague reference here -- the bug requires?

Yes. I replaced "It" with "The bug".

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1082: if
(!base::mac::IsOSMavericks())
On 2014/10/17 19:13:16, Robert Sesek wrote:
> This issue is resolved on Yosemite?

Yes. The bug only affects Mavericks.

https://codereview.chromium.org/655393002/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1085: ![NSScreen
screensHaveSeparateSpaces])
On 2014/10/17 19:13:16, Robert Sesek wrote:
> nit: any time the condition is multiple lines, the body needs braces {}

Done.

One day, in a universe far-far away, clang format will add the braces after it
changes a 1-line conditional to a 2-line conditional.

[Robert Sesek, 2014-10-17 21:05:19]

LGTM

[erikchen, 2014-10-17 21:05:43]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-10-17 21:07:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/655393002/60001

[commit-bot: I haz the power, 2014-10-17 21:39:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-17 21:39:47]

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