Fixed a fullscreen race condition on OSX

Fixed a fullscreen race condition on OSX

Ensure that |windowDidExitFullscreen| gets called after the fullscreen
transition is completed. Slightly speed up the animations on Yosemite.

BUG=593419
Committed: https://crrev.com/38ac6be27a783853f6a8fec136d2f4d3235a34d6
Cr-Commit-Position: refs/heads/master@{#383381}

[spqchan, 2016-03-17 21:24:48]

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

[spqchan, 2016-03-17 21:24:49]

PTAL

[Robert Sesek, 2016-03-18 16:41:13]

Can you explain the race condition here or on the bug? Is it that the animation
finishes after -windowDidExitFullscreen: is called?

https://codereview.chromium.org/1813693003/diff/1/chrome/browser/ui/cocoa/bro...
File chrome/browser/ui/cocoa/browser_window_fullscreen_transition.h (right):

https://codereview.chromium.org/1813693003/diff/1/chrome/browser/ui/cocoa/bro...
chrome/browser/ui/cocoa/browser_window_fullscreen_transition.h:12: @protocol
BrowserWindowFullscreenTransitionDelegate
Does this class always operate on a BrowserWindowController? Thoughts on passing
that to the initializer (in lieu of passing in tabStripBackgroundView and
FramedBrowserWindow)? That would mean not adding this protocol.

[spqchan, 2016-03-18 18:50:05]

On 2016/03/18 16:41:13, Robert Sesek wrote:
> Can you explain the race condition here or on the bug? Is it that the
animation
> finishes after -windowDidExitFullscreen: is called?
> 
>
https://codereview.chromium.org/1813693003/diff/1/chrome/browser/ui/cocoa/bro...
> File chrome/browser/ui/cocoa/browser_window_fullscreen_transition.h (right):
> 
>
https://codereview.chromium.org/1813693003/diff/1/chrome/browser/ui/cocoa/bro...
> chrome/browser/ui/cocoa/browser_window_fullscreen_transition.h:12: @protocol
> BrowserWindowFullscreenTransitionDelegate
> Does this class always operate on a BrowserWindowController? Thoughts on
passing
> that to the initializer (in lieu of passing in tabStripBackgroundView and
> FramedBrowserWindow)? That would mean not adding this protocol.

Sure thing, the race condition is that the "windowDidExitFullscreen:" can get
called before the animation finishes.
Usually that isn't the case, since we are running animations at a fraction of
the duration that AppKit gives us.
However, there's still a slim chance if the user's machine is really slow.

The reason why I passed tabStripBackgroundView and FramedBrowserWindow instead
of BrowserWindowController in the past is because I didn't want to expose too
much. However, considering how much params are being passed in now, may as well
just pass in the BrowserWindowController. I'll redo this and will let you know
when it's ready.

[spqchan, 2016-03-23 21:09:59]

PTAL

[Robert Sesek, 2016-03-24 18:45:37]

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

https://codereview.chromium.org/1813693003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm:213:
controller_.reset([controller retain]);
Does this have the potential to create a retain cycle?

[spqchan, 2016-03-24 19:59:30]

PTAL

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

https://codereview.chromium.org/1813693003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm:213:
controller_.reset([controller retain]);
On 2016/03/24 18:45:37, Robert Sesek wrote:
> Does this have the potential to create a retain cycle?

Good point. I changed it into a weak pointer

[Robert Sesek, 2016-03-25 20:18:26]

LGTM

[spqchan, 2016-03-25 21:31:22]

The CQ bit was checked by spqchan@chromium.org

[commit-bot: I haz the power, 2016-03-25 21:31:44]

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

[commit-bot: I haz the power, 2016-03-25 22:13:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-25 22:14:16]

Description was changed from

==========
Fixed a fullscreen race condition on OSX

Ensure that |windowDidExitFullscreen| gets called after the fullscreen
transition is completed. Slightly speed up the animations on Yosemite.

BUG=593419
==========

to

==========
Fixed a fullscreen race condition on OSX

Ensure that |windowDidExitFullscreen| gets called after the fullscreen
transition is completed. Slightly speed up the animations on Yosemite.

BUG=593419

Committed: https://crrev.com/38ac6be27a783853f6a8fec136d2f4d3235a34d6
Cr-Commit-Position: refs/heads/master@{#383381}
==========

[commit-bot: I haz the power, 2016-03-25 22:14:17]

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