mac: Add basic layout unit test for fullscreen transition.

mac: Add basic layout unit tests for AppKit Fullscreen.

Previous tests were flaky and keep getting disabled.

BUG=

Committed: https://crrev.com/52ec3e50337f44c70651f80bf934c39f85c37abc
Cr-Commit-Position: refs/heads/master@{#293759}

[erikchen, 2014-09-04 22:30:21]

Patchset #1 (id:1) has been deleted

[erikchen, 2014-09-04 22:35:24]

Patchset #1 (id:20001) has been deleted

[erikchen, 2014-09-04 22:37:51]

Patchset #1 (id:40001) has been deleted

[erikchen, 2014-09-04 22:39:43]

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

[erikchen, 2014-09-04 22:39:43]

andresantoso: Please review.

[Andre, 2014-09-05 00:13:01]

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/presentation_mode_controller.h (right):

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller.h:150: -
(void)setMenuBarRevealProgress:(CGFloat)progress;
Why are these 2 exposed here, but changeToolbarFraction is not (the test makes a
category for it).
How about  we move them to a category declared in
presentation_mode_controller_private.h which the test can include?

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm (right):

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:40: return
[bookmarkBarController_ view];
infoBarContainerController, toolbarController, and bookmarkBarController have
public getters, we should call it instead of accessing the ivars.
Also, BrowserWindowController declares the same methods in a category. It's not
a good idea to have multiple categories with the same methods, it is
unpredictable which one will win.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:131: NSRect
FrameInWindow(NSView* view) {
Put helper functions in anonymous namespace.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:134: return
[root convertRect:[view frame] fromView:[view superview]];
A simpler way to convert to window coordinates:
  [[view superview] convertRect:[view frame] toView:nil]
or maybe (note frame -> bounds):
  [view convertRect:[view bounds] toView:nil]

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:179:
NSHeight([[[[controller_ window] contentView] superview] bounds]);
How about NSHeight([[controller_ window] frame])

[erikchen, 2014-09-05 17:57:22]

Patchset #2 (id:80001) has been deleted

[erikchen, 2014-09-05 17:57:49]

andresantoso: PTAL

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/presentation_mode_controller.h (right):

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller.h:150: -
(void)setMenuBarRevealProgress:(CGFloat)progress;
On 2014/09/05 00:13:00, Andre wrote:
> Why are these 2 exposed here, but changeToolbarFraction is not (the test makes
a
> category for it).
> How about  we move them to a category declared in
> presentation_mode_controller_private.h which the test can include?

Done.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm (right):

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:40: return
[bookmarkBarController_ view];
On 2014/09/05 00:13:00, Andre wrote:
> infoBarContainerController, toolbarController, and bookmarkBarController have
> public getters, we should call it instead of accessing the ivars.
> Also, BrowserWindowController declares the same methods in a category. It's
not
> a good idea to have multiple categories with the same methods, it is
> unpredictable which one will win.

These were copied from another test. I removed them.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:131: NSRect
FrameInWindow(NSView* view) {
On 2014/09/05 00:13:01, Andre wrote:
> Put helper functions in anonymous namespace.

Done.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:134: return
[root convertRect:[view frame] fromView:[view superview]];
On 2014/09/05 00:13:00, Andre wrote:
> A simpler way to convert to window coordinates:
>   [[view superview] convertRect:[view frame] toView:nil]
> or maybe (note frame -> bounds):
>   [view convertRect:[view bounds] toView:nil]

I used the latter.

https://codereview.chromium.org/545643002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:179:
NSHeight([[[[controller_ window] contentView] superview] bounds]);
On 2014/09/05 00:13:00, Andre wrote:
> How about NSHeight([[controller_ window] frame])

Done. Also updated other test.

[Andre, 2014-09-05 18:02:21]

lgtm

[erikchen, 2014-09-05 18:04:22]

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

[erikchen, 2014-09-05 18:04:22]

rsesek: Looking for an OWNER review of chrome/browser/ui/cocoa/*

[Robert Sesek, 2014-09-05 19:53:40]

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

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller_private.h:162: // Exposed for
testing.
This needs an actual declaration comment, too.

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/presentation_mode_controller_private.h (right):

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_private.h:13: @interface
PresentationModeController (Private)
BWC has a separate file for its private methods because it's so large. This
should just go in an (ExposedForTesting) category at the end of the main .h
file.

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm (right):

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:120: virtual
void SetUp() {
OVERRIDE

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:131: virtual
void TearDown() {
OVERRIDE

[erikchen, 2014-09-05 20:47:02]

rsesek: PTAL

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

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller_private.h:162: // Exposed for
testing.
On 2014/09/05 19:53:40, rsesek wrote:
> This needs an actual declaration comment, too.

Done.

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/presentation_mode_controller_private.h (right):

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_private.h:13: @interface
PresentationModeController (Private)
On 2014/09/05 19:53:40, rsesek wrote:
> BWC has a separate file for its private methods because it's so large. This
> should just go in an (ExposedForTesting) category at the end of the main .h
> file.

Done.

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm (right):

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:120: virtual
void SetUp() {
On 2014/09/05 19:53:40, rsesek wrote:
> OVERRIDE

Done.

https://codereview.chromium.org/545643002/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm:131: virtual
void TearDown() {
On 2014/09/05 19:53:40, rsesek wrote:
> OVERRIDE

Done.

[Robert Sesek, 2014-09-05 21:09:47]

LGTM

[erikchen, 2014-09-05 21:09:56]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-09-05 21:11:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/545643002/120001

[commit-bot: I haz the power, 2014-09-06 03:15:33]

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

[commit-bot: I haz the power, 2014-09-06 03:15:35]

Exceeded time limit waiting for builds to trigger.

[erikchen, 2014-09-08 18:23:15]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-09-08 18:27:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/545643002/120001

[commit-bot: I haz the power, 2014-09-08 18:31:50]

Committed patchset #3 (id:120001) as a489b5be9b8f64ae674d0ff757ab78cb14923532

[commit-bot: I haz the power, 2014-09-10 03:47:18]

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