[Mac] Implement LocationBarViewMac::UpdateLocationBarVisibility()

chrome/browser/ui/cocoa/toolbar/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/toolbar/toolbar_controller.h"
 
 #include <algorithm>
 
 #include "base/mac/bundle_locations.h"
 #include "base/mac/foundation_util.h"
@@ skipping 52 matching lines @@
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image.h"
 
 using content::OpenURLParams;
 using content::Referrer;
 using content::WebContents;
 
 namespace {
 
+// Duration of the toolbar animation.
+const NSTimeInterval kToolBarAnimationDuration = 0.12;

[tapted, 2015/10/20 06:31:09]

Can this be shared with something?  (just asking... not sure if that's a good
change)

[dominickn, 2015/10/26 02:53:04]

I set this value to be the same as the bookmark bar animation, but I don't know
if it's worth sharing the value between the two as they both currently use an
anonymous constant for it.

[tapted, 2015/10/26 04:40:55]

So probably it should move to animatable_view.mm and the `duration` argument
removed, since everything passes 0.12. Something for another CL though.

[dominickn, 2015/10/26 06:42:42]

Acknowledged.

+
 // Height of the toolbar in pixels when the bookmark bar is closed.
 const CGFloat kBaseToolbarHeightNormal = 35.0;
 
 // The padding above the toolbar elements. This is calculated from the values
 // in Toolbar.xib: the height of the toolbar (35) minus the height of the child
 // elements (29) minus the y-origin of the elements (4).
 const CGFloat kToolbarElementTopPadding = 2.0;
 
 // The minimum width of the location bar in pixels.
 const CGFloat kMinimumLocationBarWidth = 100.0;
 
+// The padding to the left and right of the location bar when it is shown
+// by itself in the toolbar.
+const CGFloat kLocationBarOnlyHorizontalPadding = 3.0;
+
 // The amount of left padding that the wrench menu should have.
 const CGFloat kWrenchMenuLeftPadding = 3.0;
 
 class BrowserActionsContainerDelegate :
     public BrowserActionsContainerViewSizeDelegate {
  public:
   BrowserActionsContainerDelegate(
       AutocompleteTextField* location_bar,
       BrowserActionsContainerView* browser_actions_container_view);
   ~BrowserActionsContainerDelegate() override;
@@ skipping 27 matching lines @@
 }
 
 }  // namespace
 
 @interface ToolbarController()
 @property(assign, nonatomic) Browser* browser;
 - (void)cleanUp;
 - (void)addAccessibilityDescriptions;
 - (void)initCommandStatus:(CommandUpdater*)commands;
 - (void)prefChanged:(const std::string&)prefName;
-- (BackgroundGradientView*)backgroundGradientView;
+- (ToolbarView*)toolbarView;
 - (void)toolbarFrameChanged;
 - (void)pinLocationBarToLeftOfBrowserActionsContainerAndAnimate:(BOOL)animate;
 - (void)maintainMinimumLocationBarWidth;
 - (void)adjustBrowserActionsContainerForNewWindow:(NSNotification*)notification;
 - (void)browserActionsContainerDragged:(NSNotification*)notification;
 - (void)browserActionsVisibilityChanged:(NSNotification*)notification;
 - (void)browserActionsContainerWillAnimate:(NSNotification*)notification;
 - (void)adjustLocationSizeBy:(CGFloat)dX animate:(BOOL)animate;
 - (void)updateWrenchButtonSeverity:(WrenchIconPainter::Severity)severity
                            animate:(BOOL)animate;
@@ skipping 60 matching lines @@
 
 }  // namespace ToolbarControllerInternal
 
 @implementation ToolbarController
 
 @synthesize browser = browser_;
 
 - (id)initWithCommands:(CommandUpdater*)commands
                profile:(Profile*)profile
                browser:(Browser*)browser
+        resizeDelegate:(id<ViewResizer>)resizeDelegate
           nibFileNamed:(NSString*)nibName {
   DCHECK(commands && profile && [nibName length]);
   if ((self = [super initWithNibName:nibName
                               bundle:base::mac::FrameworkBundle()])) {
     commands_ = commands;
     profile_ = profile;
     browser_ = browser;
     hasToolbar_ = YES;
     hasLocationBar_ = YES;
 
     // Register for notifications about state changes for the toolbar buttons
     commandObserver_.reset(
         new ToolbarControllerInternal::CommandObserverBridge(self));
 
     commands->AddCommandObserver(IDC_BACK, commandObserver_.get());
     commands->AddCommandObserver(IDC_FORWARD, commandObserver_.get());
     commands->AddCommandObserver(IDC_RELOAD, commandObserver_.get());
     commands->AddCommandObserver(IDC_HOME, commandObserver_.get());
     commands->AddCommandObserver(IDC_BOOKMARK_PAGE, commandObserver_.get());
     // NOTE: Don't remove the command observers. ToolbarController is
     // autoreleased at about the same time as the CommandUpdater (owned by the
     // Browser), so |commands_| may not be valid any more.
+
+    [[self toolbarView] setResizeDelegate:resizeDelegate];
   }
   return self;
 }
 
 - (id)initWithCommands:(CommandUpdater*)commands
                profile:(Profile*)profile
-               browser:(Browser*)browser {
+               browser:(Browser*)browser
+        resizeDelegate:(id<ViewResizer>)resizeDelegate {
   if ((self = [self initWithCommands:commands
                              profile:profile
                              browser:browser
+                      resizeDelegate:resizeDelegate
                         nibFileNamed:@"Toolbar"])) {
     // Start global error services now so we badge the menu correctly.
     SyncGlobalErrorFactory::GetForProfile(profile);
   }
   return self;
 }
 
 // Called after the view is done loading and the outlets have been hooked up.
 // Now we can hook up bridges that rely on UI objects such as the location bar
 // and button state. -viewDidLoad is the recommended way to do this in 10.10
@@ skipping 88 matching lines @@
   // Create the controllers for the back/forward menus.
   backMenuController_.reset([[BackForwardMenuController alloc]
           initWithBrowser:browser_
                 modelType:BACK_FORWARD_MENU_TYPE_BACK
                    button:backButton_]);
   forwardMenuController_.reset([[BackForwardMenuController alloc]
           initWithBrowser:browser_
                 modelType:BACK_FORWARD_MENU_TYPE_FORWARD
                    button:forwardButton_]);
 
-  // For a popup window, the toolbar is really just a location bar
-  // (see override for [ToolbarController view], below).  When going
-  // fullscreen, we remove the toolbar controller's view from the view
-  // hierarchy.  Calling [locationBar_ removeFromSuperview] when going
-  // fullscreen causes it to get released, making us unhappy
-  // (http://crbug.com/18551).  We avoid the problem by incrementing
-  // the retain count of the location bar; use of the scoped object
-  // helps us remember to release it.
-  locationBarRetainer_.reset([locationBar_ retain]);
   trackingArea_.reset(
       [[CrTrackingArea alloc] initWithRect:NSZeroRect // Ignored
                                    options:NSTrackingMouseMoved |
                                            NSTrackingInVisibleRect |
                                            NSTrackingMouseEnteredAndExited |
                                            NSTrackingActiveAlways
                                      owner:self
                                   userInfo:nil]);
   NSView* toolbarView = [self view];
   [toolbarView addTrackingArea:trackingArea_.get()];
@@ skipping 17 matching lines @@
 
   [self addAccessibilityDescriptions];
 }
 
 - (void)dealloc {
   [self cleanUp];
   [super dealloc];
 }
 
 - (void)browserWillBeDestroyed {
+  // Clear resize delegate so it doesn't get called during stopAnimation, and
+  // stop any in-flight animation.
+  [[self toolbarView] setResizeDelegate:nil];
+  [[self toolbarView] stopAnimation];
+
   // Pass this call onto other reference counted objects.
   [backMenuController_ browserWillBeDestroyed];
   [forwardMenuController_ browserWillBeDestroyed];
   [browserActionsController_ browserWillBeDestroyed];
   [wrenchMenuController_ browserWillBeDestroyed];
 
   [self cleanUp];
 }
 
 - (void)cleanUp {
@@ skipping 172 matching lines @@
 
   hasToolbar_ = toolbar;
 
   // If there's a toolbar, there must be a location bar.
   DCHECK((toolbar && locBar) || !toolbar);
   hasLocationBar_ = toolbar ? YES : locBar;
 
   // Decide whether to hide/show based on whether there's a location bar.
   [[self view] setHidden:!hasLocationBar_];
 
-  // Make location bar not editable when in a pop-up.
+  // Make location bar not editable when in a pop-up or an app window.
   locationBarView_->SetEditable(toolbar);
+
+  // Hide the toolbar buttons and only display the location bar in the view
+  // if we do not have a toolbar but have a location bar.
+  if (!toolbar && locBar)
+    [self showLocationBarOnly];
 }
 
-- (NSView*)view {
-  if (hasToolbar_)
-    return [super view];
-  return locationBar_;
-}
-
-// (Private) Returns the backdrop to the toolbar.
-- (BackgroundGradientView*)backgroundGradientView {
-  // We really do mean |[super view]|; see our override of |-view|.
-  DCHECK([[super view] isKindOfClass:[BackgroundGradientView class]]);
-  return (BackgroundGradientView*)[super view];
+// (Private) Returns the backdrop to the toolbar as a ToolbarView.
+- (ToolbarView*)toolbarView{
+  return base::mac::ObjCCastStrict<ToolbarView>([self view]);
 }
 
 - (id)customFieldEditorForObject:(id)obj {
   if (obj == locationBar_) {
     // Lazilly construct Field editor, Cocoa UI code always runs on the
     // same thread, so there shoudn't be a race condition here.
     if (autocompleteTextFieldEditor_.get() == nil) {
       autocompleteTextFieldEditor_.reset(
           [[AutocompleteTextFieldEditor alloc] init]);
     }
@@ skipping 104 matching lines @@
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(adjustBrowserActionsContainerForNewWindow:)
                name:NSWindowDidBecomeKeyNotification
              object:[[self view] window]];
   }
   if (![browserActionsContainerView_ isHidden])
     [self pinLocationBarToLeftOfBrowserActionsContainerAndAnimate:NO];
 }
 
+- (void)updateVisibility:(BOOL)visible withAnimation:(BOOL)animate {
+  // |visible| being YES is equivalent to [[toolbarController_ view] isHidden]
+  // being NO, and vice-versa. That is, when |visible| and |isHidden| have the
+  // same value, a state change is needed.
+  ToolbarView* view = [self toolbarView];
+  if ([view isHidden] == visible) {
+    // Send showLocationBarOnly if there is no toolbar and there is a location
+    // bar. This displays the toolbar with only the location bar inside it.
+    [self setHasToolbar:hasToolbar_ hasLocationBar:visible];

[tapted, 2015/10/20 06:31:09]

This is in the wrong place. But.. I'm wondering if it can simply be removed...

The constructor of a Browser calls
HostedAppBrowserController::UpdateLocationBarVisibility(false).

Presumably that's because something has already called setHasToolbar:NO
hasLocationBar:YES -- maybe this is only (effectively) happening on views at the
moment, and there's a missing piece for Cocoa where that should be happening
during construction.

[dominickn, 2015/10/26 02:53:04]

HostedAppBrowserController::UpdateLocationBarVisibility(false) eventually calls
this function via the LocationBarViewMac, and that call is the same as when the
visibility is changed. I've changed how showLocationOnly is called and moved it
explicitly here to resolve the pop up window visual differences.

[tapted, 2015/10/26 04:40:55]

The point is that, currently, browser_window_controller.mm is the only place
that calls setHasToolbar:hasLocationBar and it does it exactly once, immediately
after the ToolbarController is initialized.

What breaks if this line is removed?

And (assuming something breaks), can it be resolved by having the current call
site always pass YES for hasLocationBar? (perhaps, only when there is a
hosted_app_controller_). That is, the case where there *is* a
hosted_app_controller but the location bar is hidden should be _always_ handled
by setting the height to 0. If hasLocationBar starts off as `NO` then the
toolbar is in a strange state until the first time it's shown (sure, it's
hidden, so you can't _see_ the strange state, but it's there..)

[dominickn, 2015/10/26 06:42:42]

Removing the call breaks the animation for bookmark apps because it's using the
isHidden property of the location bar view to toggle the visible/not visible
state. hasLocationBar is always YES for bookmark apps anyway, so that doesn't
fix the issue. I've added a call to [view setHidden:!visible] to fix this.

+
+    // updateVisibility is only called when in a hosted app. The bookmark bar is
+    // never visible in these apps, so the desired height will always be
+    // kBaseToolbarHeightNormal.
+    CGFloat newHeight = visible ? kBaseToolbarHeightNormal : 0;
+
+    // Perform the animation, which will cause the BrowserWindowController to
+    // resize this view in the browser layout as required.
+    if (animate) {
+      [view animateToNewHeight:newHeight
+                      duration:kToolBarAnimationDuration];
+    } else {
+      [view setHeight:newHeight];
+    }
+  }
+}
+
+- (void)showLocationBarOnly {
+  // Ignore this message if the toolbar is shown or the location bar is hidden.
+  if (hasToolbar_ || !hasLocationBar_)

[tapted, 2015/10/20 06:31:09]

Can this be a DCHECK now? (with the reverse logic)

[dominickn, 2015/10/26 02:53:04]

Now that the callsite for this method has changed, this can only be a DCHECK if
the callsite is guarded by an if (!hasToolbar_ && hasLocationBar_) (since
setHasToolbar will update the location bar visibility).

+    return;
+
+  // If the toolbar buttons are already hidden, return. Assumes that the back,
+  // forward, reload, and wrench buttons are either shown or hidden together.
+  if ([backButton_ isHidden]) {
+    DCHECK_EQ(YES, [backButton_ isHidden] && [forwardButton_ isHidden] &&
+                       [reloadButton_ isHidden] && [wrenchButton_ isHidden]);
+    return;
+  }
+
+  // Adjust the location bar frame to stretch the full width of the toolbar,
+  // with padding on either side.
+  NSRect locationFrame = [locationBar_ frame];
+  locationFrame.origin.x = kLocationBarOnlyHorizontalPadding;
+  locationFrame.size.width =
+      NSWidth([[self view] frame]) - 2 * kLocationBarOnlyHorizontalPadding;
+  [locationBar_ setFrame:locationFrame];
+
+  [backButton_ setHidden:YES];
+  [forwardButton_ setHidden:YES];
+  [reloadButton_ setHidden:YES];
+  [wrenchButton_ setHidden:YES];
+
+  if (!showHomeButton_.GetValue())

[tapted, 2015/10/20 06:31:09]

This check isn't needed

[dominickn, 2015/10/26 02:53:04]

Done.

+    [homeButton_ setHidden:YES];
+}
+
 - (void)adjustBrowserActionsContainerForNewWindow:
     (NSNotification*)notification {
   [self toolbarFrameChanged];
   [[NSNotificationCenter defaultCenter]
       removeObserver:self
                 name:NSWindowDidBecomeKeyNotification
               object:[[self view] window]];
 }
 
 - (void)browserActionsContainerDragged:(NSNotification*)notification {
@@ skipping 125 matching lines @@
 
 - (CGFloat)desiredHeightForCompression:(CGFloat)compressByHeight {
   // With no toolbar, just ignore the compression.
   if (!hasToolbar_)
     return NSHeight([locationBar_ frame]);
 
   return kBaseToolbarHeightNormal - compressByHeight;
 }
 
 - (void)setDividerOpacity:(CGFloat)opacity {
-  BackgroundGradientView* view = [self backgroundGradientView];
+  ToolbarView* view = [self toolbarView];

[tapted, 2015/10/20 06:31:09]

view -> toolbarView

[dominickn, 2015/10/26 02:53:04]

Done.

   [view setShowsDivider:(opacity > 0 ? YES : NO)];
 
-  // We may not have a toolbar view (e.g., popup windows only have a location
-  // bar).
-  if ([view isKindOfClass:[ToolbarView class]]) {
-    ToolbarView* toolbarView = (ToolbarView*)view;
-    [toolbarView setDividerOpacity:opacity];
-  }
+  ToolbarView* toolbarView = (ToolbarView*)view;
+  [toolbarView setDividerOpacity:opacity];
 
   [view setNeedsDisplay:YES];
 }
 
 - (BrowserActionsController*)browserActionsController {
   return browserActionsController_.get();
 }
 
 - (NSView*)wrenchButton {
   return wrenchButton_;
@@ skipping 57 matching lines @@
 - (void)hideDropURLsIndicatorInView:(NSView*)view {
   // Do nothing.
 }
 
 // (URLDropTargetController protocol)
 - (BOOL)isUnsupportedDropData:(id<NSDraggingInfo>)info {
   return drag_util::IsUnsupportedDropData(profile_, info);
 }
 
 @end