[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"
 #include "base/mac/sdk_forward_declarations.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #include "chrome/common/chrome_switches.h"
 #import "third_party/google_toolbox_for_mac/src/AppKit/GTMNSAnimation+Duration.h"
 #import "ui/base/cocoa/nsview_additions.h"
 #import "ui/base/cocoa/tracking_area.h"
 
 namespace {
 
 // The duration of the toolbar show/hide animation.
-const NSTimeInterval kDropdownAnimationDuration = 0.20;
+const NSTimeInterval kDropdownAnimationDuration = 0.17;

[erikchen, 2016/08/30 23:46:48]

Should this vary by OS version?

 
 // If the fullscreen toolbar is hidden, it is difficult for the user to see
 // changes in the tabstrip. As a result, if a tab is inserted or the current
 // tab switched to a new one, the toolbar must animate in and out to display
 // the tabstrip changes to the user. The animation drops down the toolbar and
 // then wait for 0.75 seconds before it hides the toolbar.
 const NSTimeInterval kTabStripChangesDelay = 0.75;
 
 // Additional height threshold added at the toolbar's bottom. This is to mimic
 // threshold the mouse position needs to be at before the menubar automatically
@@ skipping 116 matching lines @@
 - (void)setupTrackingArea;
 - (void)removeTrackingAreaIfNecessary;
 
 // Returns YES if the mouse is inside the tracking area.
 - (BOOL)mouseInsideTrackingArea;
 
 // Whether the current screen is expected to have a menu bar, regardless of
 // current visibility of the menu bar.
 - (BOOL)doesScreenHaveMenuBar;
 
+// Returns YES if the toolbar's visibility is locked or if the cursor is
+// interacting with it.
+- (BOOL)isToolbarFocused;
+
 // Returns YES if the window is on the primary screen.
 - (BOOL)isWindowOnPrimaryScreen;
 
 // Returns |kFullScreenModeHideAll| when the overlay is hidden and
 // |kFullScreenModeHideDock| when the overlay is shown.
 - (base::mac::FullScreenMode)desiredSystemFullscreenMode;
 
 // Animate the overlay to the given visibility with animation. If |visible|
 // is true, animate the toolbar to a fraction of 1.0. Otherwise it's 0.0.
 - (void)animateToolbarVisibility:(BOOL)visible;
@@ skipping 16 matching lines @@
 @implementation FullscreenToolbarController
 
 @synthesize slidingStyle = slidingStyle_;
 
 - (id)initWithBrowserController:(BrowserWindowController*)controller
                           style:(fullscreen_mac::SlidingStyle)style {
   if ((self = [super init])) {
     browserController_ = controller;
     systemFullscreenMode_ = base::mac::kFullScreenModeNormal;
     slidingStyle_ = style;
+    hiddenToolbarState_ = fullscreen_mac::TOOLBAR_HIDDEN;
   }
 
   // Install the Carbon event handler for the menubar show, hide and
   // undocumented reveal event.
   EventTypeSpec eventSpecs[3];
 
   eventSpecs[0].eventClass = kEventClassMenu;
   eventSpecs[0].eventKind = kMenuBarRevealEventKind;
 
   eventSpecs[1].eventClass = kEventClassMenu;
@@ skipping 56 matching lines @@
 }
 
 - (void)windowDidResignMain:(NSNotification*)notification {
   [self updateMenuBarAndDockVisibility];
 }
 
 - (CGFloat)floatingBarVerticalOffset {
   return kFloatingBarVerticalOffset;
 }
 
-- (void)ensureOverlayShownWithAnimation:(BOOL)animate {
+- (void)showToolbarIfPossibleWithAnimation:(BOOL)animate {
   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 {
+- (void)hideToolbarIfPossibleWithAnimation:(BOOL)animate {
   if (!inFullscreenMode_)
     return;
 
   if (self.slidingStyle != fullscreen_mac::OMNIBOX_TABS_HIDDEN)
     return;
 
+  if ([self isToolbarFocused])
+    return;
+
   [self cancelHideTimer];
   [self animateToolbarVisibility:NO];
+  shouldShowToolbarWithoutMenu_ = NO;
 }
 
 - (void)cancelAnimationAndTimer {
   [self cancelHideTimer];
   [currentAnimation_ stopAnimation];
   currentAnimation_.reset();
 }
 
 - (void)revealToolbarForTabStripChanges {
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableFullscreenToolbarReveal)) {
     return;
   }
 
+  if (self.slidingStyle != fullscreen_mac::OMNIBOX_TABS_HIDDEN)
+    return;
+
   // Reveal the toolbar for tabstrip changes if the toolbar is hidden.
-  if (IsCGFloatEqual([self toolbarFraction], kHideFraction)) {
+  if (hiddenToolbarState_ == fullscreen_mac::TOOLBAR_HIDDEN) {
     isRevealingToolbarForTabStripChanges_ = YES;
-    [self ensureOverlayShownWithAnimation:YES];
+    [self showToolbarIfPossibleWithAnimation:YES];
   }
 }
 
 - (void)setSystemFullscreenModeTo:(base::mac::FullScreenMode)mode {
   if (mode == systemFullscreenMode_)
     return;
   if (systemFullscreenMode_ == base::mac::kFullScreenModeNormal)
     base::mac::RequestFullScreen(mode);
   else if (mode == base::mac::kFullScreenModeNormal)
     base::mac::ReleaseFullScreen(systemFullscreenMode_);
   else
     base::mac::SwitchFullScreenModes(systemFullscreenMode_, mode);
   systemFullscreenMode_ = mode;
 }
 
 - (void)mouseEntered:(NSEvent*)event {
   // Empty implementation. Required for CrTrackingArea.
 }
 
 - (void)mouseExited:(NSEvent*)event {
   DCHECK(inFullscreenMode_);
   DCHECK_EQ([event trackingArea], trackingArea_.get());
 
   // If the menubar is gone, animate the toolbar out.
   if (IsCGFloatEqual(menubarFraction_, kHideFraction))
-    [self ensureOverlayHiddenWithAnimation:YES];
+    [self hideToolbarIfPossibleWithAnimation:YES];
 
-  [self removeTrackingAreaIfNecessary];
+  // We no longer need to track it if it's not focused.
+  if (![self isToolbarFocused])
+    [self removeTrackingAreaIfNecessary];
 }
 
 - (void)updateToolbar {
   [browserController_ layoutSubviews];
 
   // In AppKit fullscreen, moving the mouse to the top of the screen toggles
   // menu visibility. Replicate the same effect for immersive fullscreen.
   if ([browserController_ isInImmersiveFullscreen])
     [self updateMenuBarAndDockVisibility];
 }
@@ skipping 12 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 (hiddenToolbarState_ == fullscreen_mac::TOOLBAR_SHOWN)
+        return kShowFraction;
+
+      if (hiddenToolbarState_ == fullscreen_mac::TOOLBAR_HIDDEN)
+        return kHideFraction;
+
       if (currentAnimation_.get())
         return [currentAnimation_ toolbarFraction];
 
-      if (hideTimer_.get() || shouldAnimateToolbarOut_)
-        return kShowFraction;
-
-      return toolbarFractionFromMenuProgress_;
+      return shouldShowToolbarWithoutMenu_ ? kShowFraction : menubarFraction_;
   }
 }
 
 - (BOOL)isFullscreenTransitionInProgress {
   return [browserController_ isFullscreenTransitionInProgress];
 }
 
 - (BOOL)isInFullscreen {
   return inFullscreenMode_;
 }
 
 - (BOOL)isMouseOnScreen {
   return NSMouseInRect([NSEvent mouseLocation],
                        [[browserController_ window] screen].frame, false);
 }
 
 - (void)setTrackingAreaFromOverlayFrame:(NSRect)frame {
   NSRect contentBounds = [contentView_ bounds];
+  CGFloat yOffset =
+      kFloatingBarVerticalOffset - kTrackingAreaAdditionalThreshold;
+
   trackingAreaFrame_ = frame;
-  trackingAreaFrame_.origin.y -= kTrackingAreaAdditionalThreshold;
+  trackingAreaFrame_.origin.y += yOffset;
   trackingAreaFrame_.size.height =
       NSMaxY(contentBounds) - trackingAreaFrame_.origin.y;
 }
 
 - (void)animationDidStop:(NSAnimation*)animation {
+  CGFloat toolbarFraction = [self toolbarFraction];
+  if (IsCGFloatEqual(toolbarFraction, 1.0))
+    hiddenToolbarState_ = fullscreen_mac::TOOLBAR_SHOWN;
+  else if (IsCGFloatEqual(toolbarFraction, 0.0))
+    hiddenToolbarState_ = fullscreen_mac::TOOLBAR_HIDDEN;
+  else
+    hiddenToolbarState_ = fullscreen_mac::TOOLBAR_TRANSITIONING;
+
   if (isRevealingToolbarForTabStripChanges_) {
-    if ([self toolbarFraction] > 0.0) {
+    if (hiddenToolbarState_ != fullscreen_mac::TOOLBAR_HIDDEN) {
       // Set the timer to hide the toolbar.
       [hideTimer_ invalidate];
       hideTimer_.reset(
           [[NSTimer scheduledTimerWithTimeInterval:kTabStripChangesDelay
                                             target:self
                                           selector:@selector(hideTimerFire:)
                                           userInfo:nil
                                            repeats:NO] retain]);
     } else {
       isRevealingToolbarForTabStripChanges_ = NO;
     }
   }
 
   // Reset the |currentAnimation_| pointer now that the animation is over.
   currentAnimation_.reset();
 }
 
 - (void)animationDidEnd:(NSAnimation*)animation {
   [self animationDidStop:animation];
   [self setupTrackingArea];
 }
 
 - (void)setMenuBarRevealProgress:(CGFloat)progress {
   // If the menubarFraction increases, check if we are in the right screen
   // so that the toolbar is not revealed on the wrong screen.
   if (![self isMouseOnScreen] && progress > menubarFraction_)
     return;
 
+  BOOL isMenuBarDisappearing = progress < menubarFraction_;
   menubarFraction_ = progress;
 
   if (self.slidingStyle == fullscreen_mac::OMNIBOX_TABS_HIDDEN) {
-    if (IsCGFloatEqual(menubarFraction_, kShowFraction))
+    shouldShowToolbarWithoutMenu_ =
+        shouldShowToolbarWithoutMenu_ ||
+        (isMenuBarDisappearing && [self isToolbarFocused]);
+
+    if (IsCGFloatEqual(menubarFraction_, kShowFraction)) {
       [self setupTrackingArea];
+      hiddenToolbarState_ = fullscreen_mac::TOOLBAR_SHOWN;

[erikchen, 2016/08/30 23:46:48]

This logic confuses me. Can we please move more logic into -toolbarFraction, and
less logic in the callbacks from AppKit?

It seems like we want something along the lines of:

1) If an NSMenuBar animation is in progress, use a state based on the progress
of the menubar.
2) Otherwise, if a toolbar animation is in progress, use a state based on the
progress of the animation.
3) Otherwise, use shouldShowToolbarWithoutMenu_.

Ditto for shouldShowToolbarWithoutMenu_. It's really not clear to me what the
ivar means. From the logic, it looks like it means:

If the NSMenuBar is in animation, and there is no toolbar animation in progress,
should we show the full height of the toolbar, or just part of the height.

But this can be simplified:
Let's say that we have the state: enum NSMenuBarState { HIDDEN, SHOWING,
TRANSITIONING}. And if state = TRANSITIONING, there are two further states:
CGFloat menubarFraction_
enum NSMenuBarAnimationDirection { IN, OUT}

Then 
"""
return shouldShowToolbarWithoutMenu_ ? kShowFraction : menubarFraction_;
"""

becomes:
"""
return direction == OUT ? kShowFraction : menubarFraction_
"""

AFAICT, that will have the same behavior, but without needing this intermediate
variable of shouldShowToolbarWithoutMenu_. 

Looking at your code, right now the question I'm having trouble answering is:
What happens when there's a Chrome animation in progress, and the NSMenuBar is
animating? hiddenToolbarState_ is changed by both of those processes.

+    }
 
-    // If the menubar is disappearing from the screen, check if the mouse
-    // is still interacting with the toolbar. If it is, don't set
-    // |toolbarFractionFromMenuProgress_| so that the the toolbar will remain
-    // on the screen.
-    BOOL isMenuBarDisappearing =
-        menubarFraction_ < toolbarFractionFromMenuProgress_;
-    if (!(isMenuBarDisappearing && [self mouseInsideTrackingArea]))
-      toolbarFractionFromMenuProgress_ = progress;
+    // If |shouldShowToolbarWithoutMenu_| is true, then |hiddenToolbarState_|
+    // should remain fullscreen_mac::TOOLBAR_SHOWN.
+    if (!shouldShowToolbarWithoutMenu_) {
+      hiddenToolbarState_ = IsCGFloatEqual(menubarFraction_, kHideFraction)
+                                ? fullscreen_mac::TOOLBAR_HIDDEN
+                                : fullscreen_mac::TOOLBAR_TRANSITIONING;
+    }
   }
 
   // If an animation is not running, then -layoutSubviews will not be called
   // for each tick of the menu bar reveal. Do that manually.
   // TODO(erikchen): The animation is janky. layoutSubviews need a refactor so
   // that it calls setFrameOffset: instead of setFrame: if the frame's size has
   // not changed.
   if (!currentAnimation_.get())
     [browserController_ layoutSubviews];
 }
@@ skipping 56 matching lines @@
 
 - (BOOL)doesScreenHaveMenuBar {
   if (![[NSScreen class]
           respondsToSelector:@selector(screensHaveSeparateSpaces)])
     return [self isWindowOnPrimaryScreen];
 
   BOOL eachScreenShouldHaveMenuBar = [NSScreen screensHaveSeparateSpaces];
   return eachScreenShouldHaveMenuBar ?: [self isWindowOnPrimaryScreen];
 }
 
+- (BOOL)isToolbarFocused {
+  if ([browserController_ isBarVisibilityLockedForOwner:nil])
+    return YES;
+
+  return [self mouseInsideTrackingArea] || menubarFraction_ == kShowFraction;
+}
+
 - (BOOL)isWindowOnPrimaryScreen {
   NSScreen* screen = [[browserController_ window] screen];
   NSScreen* primaryScreen = [[NSScreen screens] firstObject];
   return (screen == primaryScreen);
 }
 
 - (base::mac::FullScreenMode)desiredSystemFullscreenMode {
   if ([self shouldShowMenubarInImmersiveFullscreen])
     return base::mac::kFullScreenModeHideDock;
   return base::mac::kFullScreenModeHideAll;
@@ skipping 19 matching lines @@
             controller:self]);
   DCHECK(currentAnimation_);
   [currentAnimation_ setAnimationBlockingMode:NSAnimationNonblocking];
   [currentAnimation_ setDelegate:self];
 
   // If there is an existing tracking area, remove it. We do not track mouse
   // movements during animations (see class comment in the header file).
   [self removeTrackingAreaIfNecessary];
 
   [currentAnimation_ startAnimation];
+  hiddenToolbarState_ = fullscreen_mac::TOOLBAR_TRANSITIONING;
 }
 
 - (void)cancelHideTimer {
   [hideTimer_ invalidate];
   hideTimer_.reset();
 }
 
 - (void)hideTimerFire:(NSTimer*)timer {
   DCHECK_EQ(hideTimer_, timer);  // This better be our hide timer.
   [hideTimer_ invalidate];       // Make sure it doesn't repeat.
   hideTimer_.reset();            // And get rid of it.
-  shouldAnimateToolbarOut_ = YES;
   [self animateToolbarVisibility:NO];
-  shouldAnimateToolbarOut_ = NO;
 }
 
 - (void)cleanup {
   [self cancelAnimationAndTimer];
   [[NSNotificationCenter defaultCenter] removeObserver:self];
 
   [self removeTrackingAreaIfNecessary];
 
   // Call the main status resignation code to perform the associated cleanup,
   // since we will no longer be receiving actual status resignation
   // notifications.
   [self setSystemFullscreenModeTo:base::mac::kFullScreenModeNormal];
 
   // No more calls back up to the BWC.
   browserController_ = nil;
 }
 
 - (BOOL)shouldShowMenubarInImmersiveFullscreen {
   return [self doesScreenHaveMenuBar] && [self toolbarFraction] > 0.99;
 }
 
 @end