[Mac] Fullscreen Toolbar Edge Cases

chrome/browser/ui/cocoa/fullscreen_toolbar_controller.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/fullscreen_toolbar_controller.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #import "base/mac/mac_util.h"
@@ skipping 249 matching lines @@
 }
 
 - (void)windowDidResignMain:(NSNotification*)notification {
   [self updateMenuBarAndDockVisibility];
 }
 
 - (CGFloat)floatingBarVerticalOffset {
   return kFloatingBarVerticalOffset;
 }
 
+- (void)lockBarVisibilityWithAnimation:(BOOL)animate {
+  isLockingBarVisibility_ = YES;
+  [self ensureOverlayShownWithAnimation:animate];
+  isLockingBarVisibility_ = NO;
+}
+
+- (void)releaseBarVisibilityWithAnimation:(BOOL)animate {
+  isReleasingBarVisibility_ = YES;
+  [self ensureOverlayHiddenWithAnimation:animate];
+  isReleasingBarVisibility_ = NO;
+}
+
 - (void)ensureOverlayShownWithAnimation:(BOOL)animate {

[erikchen, 2016/08/31 20:19:50]

I notice that this is also called by browser_window_controller_private.mm.
Should that also become lockBarVisiblity, and should this become a private
method?

[spqchan, 2016/09/07 19:46:48]

No, lockBarVisibility and releaseBarVisibility should only be called when the
barVisibilityLocks_ are actually being changed. Added comments to reflect that.

   if (!inFullscreenMode_)
     return;
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
     return;
 
   if (self.slidingStyle != fullscreen_mac::OMNIBOX_TABS_HIDDEN)
     return;
 
   [self cancelHideTimer];
   [self animateToolbarVisibility:YES];
 }
 
 - (void)ensureOverlayHiddenWithAnimation:(BOOL)animate {
   if (!inFullscreenMode_)
     return;
 
   if (self.slidingStyle != fullscreen_mac::OMNIBOX_TABS_HIDDEN)
     return;
 
+  if ([browserController_ isBarVisibilityLockedForOwner:nil] &&

[erikchen, 2016/08/31 20:19:50]

I don't see how this condition could ever return true.
-isBarVisibilityLockedForOwner: should always return false.

[spqchan, 2016/09/07 19:46:48]

ensureOverlayHiddenWithAnimation can be called when the bar visibility is
locked. !isReleasingBarVisibility_ is unnecessary though. Removed it

+      !isReleasingBarVisibility_) {
+    return;
+  }
+
+  if ([self mouseInsideTrackingArea] || menubarFraction_ == kShowFraction)
+    return;
+
   [self cancelHideTimer];
   [self animateToolbarVisibility:NO];
 }
 
 - (void)cancelAnimationAndTimer {
   [self cancelHideTimer];
   [currentAnimation_ stopAnimation];
   currentAnimation_.reset();
 }
 
@@ skipping 60 matching lines @@
 - (CGFloat)menubarOffset {
   if ([browserController_ isInAppKitFullscreen])
     return -std::floor(menubarFraction_ * [self floatingBarVerticalOffset]);
 
   return [self shouldShowMenubarInImmersiveFullscreen]
              ? -[self floatingBarVerticalOffset]
              : 0;
 }
 
 - (CGFloat)toolbarFraction {
-  if ([browserController_ isBarVisibilityLockedForOwner:nil])
-    return kShowFraction;
-
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
     return kHideFraction;
 
   switch (slidingStyle_) {
     case fullscreen_mac::OMNIBOX_TABS_PRESENT:
       return kShowFraction;
     case fullscreen_mac::OMNIBOX_TABS_NONE:
       return kHideFraction;
     case fullscreen_mac::OMNIBOX_TABS_HIDDEN:
+      if (IsCGFloatEqual(menubarFraction_, kShowFraction))
+        return kShowFraction;
+
       if (currentAnimation_.get())
         return [currentAnimation_ toolbarFraction];
 
       if (hideTimer_.get() || shouldAnimateToolbarOut_)
         return kShowFraction;
 
+      if (isLockingBarVisibility_)
+        return kHideFraction;

[erikchen, 2016/08/31 20:19:50]

I see what you're doing here, but I think this will have the wrong behavior if
the browser tries to lock the bar visiblity while the NSMenuBar is in the middle
of an animation.

[spqchan, 2016/09/07 19:46:48]

True. Updated the logic so that if the menubar is shown or in the progress of
showing itself, it will take priority over the animation

+      else if (isReleasingBarVisibility_)
+        return kShowFraction;
+      else if ([browserController_ isBarVisibilityLockedForOwner:nil])
+        return kShowFraction;
+
       return toolbarFractionFromMenuProgress_;
   }
 }
 
 - (BOOL)isFullscreenTransitionInProgress {
   return [browserController_ isFullscreenTransitionInProgress];
 }
 
 - (BOOL)isInFullscreen {
   return inFullscreenMode_;
@@ skipping 201 matching lines @@
 
   // No more calls back up to the BWC.
   browserController_ = nil;
 }
 
 - (BOOL)shouldShowMenubarInImmersiveFullscreen {
   return [self doesScreenHaveMenuBar] && [self toolbarFraction] > 0.99;
 }
 
 @end