Fixed a fullscreen race condition on OSX

chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm

Index: chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm
diff --git a/chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm b/chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm
index 409c4222dacb9594279d7079241c83589e82102b..513942ed2c055ff81b840e7072564eb3ef2f9566 100644
--- a/chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm
+++ b/chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm
@@ -12,6 +12,7 @@
 #import "base/mac/sdk_forward_declarations.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/framed_browser_window.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_background_view.h"
 
@@ -22,8 +23,10 @@ NSString* const kSnapshotWindowAnimationID = @"SnapshotWindowAnimationID";
 NSString* const kAnimationIDKey = @"AnimationIDKey";
 
 // The fraction of the duration from AppKit's startCustomAnimation methods
-// that we want our animation to run in.
+// that we want our animation to run in. Yosemite's fraction is smaller
+// since its fullscreen transition is significantly slower.
 CGFloat const kAnimationDurationFraction = 0.5;
+CGFloat const kAnimationDurationFractionYosemite = 0.3;
 
 // This class has two simultaneous animations to resize and reposition layers.
 // These animations must use the same timing function, otherwise there will be
@@ -91,6 +94,9 @@ class FrameAndStyleLock {
   // The window which is undergoing the fullscreen transition.
   base::scoped_nsobject<FramedBrowserWindow> primaryWindow_;
 
+  // The window which is undergoing the fullscreen transition.
+  base::scoped_nsobject<BrowserWindowController> controller_;
+
   // A layer that holds a snapshot of the original state of |primaryWindow_|.
   base::scoped_nsobject<CALayer> snapshotLayer_;
 
@@ -131,6 +137,10 @@ class FrameAndStyleLock {
 
   // Locks and unlocks the FullSizeContentWindow.
   scoped_ptr<FrameAndStyleLock> lock_;
+
+  // Flag that indicates if the animation was completed. Sets to true at the
+  // end of the animation.
+  BOOL completedTransition_;
 }
 
 // Takes a snapshot of |primaryWindow_| and puts it in |snapshotLayer_|.
@@ -180,11 +190,14 @@ class FrameAndStyleLock {
 
 // -------------------------Public Methods----------------------------
 
-- (instancetype)initEnterWithWindow:(FramedBrowserWindow*)window {
-  DCHECK(window);
-  DCHECK([self rootLayerOfWindow:window]);
+- (instancetype)initEnterWithController:(BrowserWindowController*)controller {
+  DCHECK(controller);
+  DCHECK([self rootLayerOfWindow:[controller window]]);
   if ((self = [super init])) {
-    primaryWindow_.reset([window retain]);
+    controller_.reset([controller retain]);
+    FramedBrowserWindow* framedBrowserWindow =
+        base::mac::ObjCCast<FramedBrowserWindow>([controller window]);
+    primaryWindow_.reset([framedBrowserWindow retain]);
 
     isEnteringFullscreen_ = YES;
     initialFrame_ = [primaryWindow_ frame];
@@ -193,19 +206,21 @@ class FrameAndStyleLock {
   return self;
 }
 
-- (instancetype)initExitWithWindow:(FramedBrowserWindow*)window
-                             frame:(NSRect)frame
-            tabStripBackgroundView:(NSView*)view {
-  DCHECK(window);
-  DCHECK([self rootLayerOfWindow:window]);
+- (instancetype)initExitWithController:(BrowserWindowController*)controller {
+  DCHECK(controller);
+  DCHECK([self rootLayerOfWindow:[controller window]]);
   if ((self = [super init])) {
-    primaryWindow_.reset([window retain]);
-    tabStripBackgroundView_.reset([view retain]);
+    controller_.reset([controller retain]);

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

Does this have the potential to create a retain cycle?

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

Good point. I changed it into a weak pointer

+    FramedBrowserWindow* framedBrowserWindow =
+        base::mac::ObjCCast<FramedBrowserWindow>([controller window]);
+    primaryWindow_.reset([framedBrowserWindow retain]);
+
     isEnteringFullscreen_ = NO;
-    finalFrame_ = frame;
     initialFrame_ = [[primaryWindow_ screen] frame];
+    finalFrame_ = [controller savedRegularWindowFrame];
+    tabStripBackgroundView_.reset([[controller tabStripBackgroundView] retain]);
 
-    lock_.reset(new FrameAndStyleLock(window));
+    lock_.reset(new FrameAndStyleLock(framedBrowserWindow));
   }
   return self;
 }
@@ -216,8 +231,15 @@ class FrameAndStyleLock {
   return @[ primaryWindow_.get(), snapshotWindow_.get() ];
 }
 
+- (BOOL)isTransitionCompleted {
+  return completedTransition_;
+}
+
 - (void)startCustomFullScreenAnimationWithDuration:(NSTimeInterval)duration {
-  CGFloat animationDuration = duration * kAnimationDurationFraction;
+  CGFloat durationFraction = base::mac::IsOSYosemite()
+                                 ? kAnimationDurationFractionYosemite
+                                 : kAnimationDurationFraction;
+  CGFloat animationDuration = duration * durationFraction;
   [self preparePrimaryWindowForAnimation];
   [self animatePrimaryWindowWithDuration:animationDuration];
   [self animateSnapshotWindowWithDuration:animationDuration];
@@ -501,7 +523,12 @@ class FrameAndStyleLock {
     CALayer* root = [self rootLayerOfWindow:primaryWindow_];
     [root removeAnimationForKey:kPrimaryWindowAnimationID];
     root.opacity = 1;
+
+    if (!isEnteringFullscreen_)
+      [controller_ exitFullscreenAnimationFinished];
   }
+
+  completedTransition_ = YES;
 }
 
 - (CALayer*)rootLayerOfWindow:(NSWindow*)window {