[Mac] Implement LocationBarViewMac::UpdateLocationBarVisibility()

chrome/browser/ui/cocoa/toolbar/toolbar_controller_unittest.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 <Cocoa/Cocoa.h>
 
 #import "base/mac/scoped_nsobject.h"
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/cocoa/cocoa_profile_test.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
+#import "chrome/browser/ui/cocoa/view_resizer_pong.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
 
 // An NSView that fakes out hitTest:.
 @interface HitView : NSView {
   id hitTestReturn_;
 }
@@ skipping 32 matching lines @@
 }
 @end
 
 namespace {
 
 class ToolbarControllerTest : public CocoaProfileTest {
  public:
 
   // Indexes that match the ordering returned by the private ToolbarController
   // |-toolbarViews| method.
-  enum {
+  enum SubviewIndex {
     kBackIndex, kForwardIndex, kReloadIndex, kHomeIndex,
     kWrenchIndex, kLocationIndex, kBrowserActionContainerViewIndex
   };
 
   void SetUp() override {
     CocoaProfileTest::SetUp();
     ASSERT_TRUE(browser());
 
+    resizeDelegate_.reset([[ViewResizerPong alloc] init]);
+
     CommandUpdater* updater =
         browser()->command_controller()->command_updater();
     // The default state for the commands is true, set a couple to false to
     // ensure they get picked up correct on initialization
     updater->UpdateCommandEnabled(IDC_BACK, false);
     updater->UpdateCommandEnabled(IDC_FORWARD, false);
     bar_.reset([[TestToolbarController alloc]
         initWithCommands:browser()->command_controller()->command_updater()
                  profile:profile()
-                 browser:browser()]);
+                 browser:browser()
+          resizeDelegate:resizeDelegate_.get()]);
     EXPECT_TRUE([bar_ view]);
     NSView* parent = [test_window() contentView];
     [parent addSubview:[bar_ view]];
   }
 
   void TearDown() override {
     bar_.reset();  // browser() must outlive the ToolbarController.
     CocoaProfileTest::TearDown();
   }
 
   // Make sure the enabled state of the view is the same as the corresponding
   // command in the updater. The views are in the declaration order of outlets.
   void CompareState(CommandUpdater* updater, NSArray* views) {
     EXPECT_EQ(updater->IsCommandEnabled(IDC_BACK),
               [[views objectAtIndex:kBackIndex] isEnabled] ? true : false);
     EXPECT_EQ(updater->IsCommandEnabled(IDC_FORWARD),
               [[views objectAtIndex:kForwardIndex] isEnabled] ? true : false);
     EXPECT_EQ(updater->IsCommandEnabled(IDC_RELOAD),
               [[views objectAtIndex:kReloadIndex] isEnabled] ? true : false);
     EXPECT_EQ(updater->IsCommandEnabled(IDC_HOME),
               [[views objectAtIndex:kHomeIndex] isEnabled] ? true : false);
   }
 
+  NSView* GetSubviewAt(SubviewIndex index) {
+    return [[bar_ toolbarViews] objectAtIndex:index];
+  }
+
+  base::scoped_nsobject<ViewResizerPong> resizeDelegate_;
   base::scoped_nsobject<TestToolbarController> bar_;
 };
 
 TEST_VIEW(ToolbarControllerTest, [bar_ view])
 
 // Test the initial state that everything is sync'd up
 TEST_F(ToolbarControllerTest, InitialState) {
   CommandUpdater* updater = browser()->command_controller()->command_updater();
   CompareState(updater, [bar_ toolbarViews]);
 }
 
 // Make sure a "titlebar only" toolbar with location bar works.
 TEST_F(ToolbarControllerTest, TitlebarOnly) {
   NSView* view = [bar_ view];
 
   [bar_ setHasToolbar:NO hasLocationBar:YES];
-  EXPECT_NE(view, [bar_ view]);
+  EXPECT_EQ(view, [bar_ view]);
 
   // Simulate a popup going fullscreen and back by performing the reparenting
   // that happens during fullscreen transitions
   NSView* superview = [view superview];
   [view removeFromSuperview];
   [superview addSubview:view];
 
   [bar_ setHasToolbar:YES hasLocationBar:YES];

[tapted, 2015/10/26 07:55:34]

Hm - does this make sense? I think we say that the state set above is
inescapable. Can you see if this test went in to test something specific? Maybe
it is not testing anything interesting, since this will presumably fail if you
try my `DCHECK(![backButton_ isHidden]);` suggestion

[dominickn, 2015/10/26 23:31:32]

From the blame and log, it doesn't look like it's testing any specific
situation. The call was added when the setHasToolbar was refactored from one to
two arguments. I've removed this call.

This particular test and a few others look a bit odd (not really any EXPECT
calls); I might have a look in another CL to make them more comprehensive.

   EXPECT_EQ(view, [bar_ view]);
 
   // Leave it off to make sure that's fine
   [bar_ setHasToolbar:NO hasLocationBar:YES];
 }
 
+// Test updateVisibility with location bar only; this method is used by bookmark
+// apps, and should never be called when the toolbar is enabled. Ensure that the
+// buttons remain in the correct state.
+TEST_F(ToolbarControllerTest, UpdateVisibility) {
+  NSView* view = [bar_ view];
+
+  // Test the escapable states first.
+  [bar_ setHasToolbar:YES hasLocationBar:YES];
+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kLocationIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kBackIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kForwardIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kReloadIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kWrenchIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+
+  // For NO/NO, only the top level toolbar view is hidden.
+  [bar_ setHasToolbar:NO hasLocationBar:NO];
+  EXPECT_TRUE([view isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kLocationIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kBackIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kForwardIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kReloadIndex) isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kWrenchIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+
+  [bar_ setHasToolbar:NO hasLocationBar:YES];
+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kLocationIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBackIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kForwardIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kReloadIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kWrenchIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+
+  // Maintain visible state.
+  [bar_ updateVisibility:YES withAnimation:NO];
+  EXPECT_GT([[bar_ view] frame].size.height, 0);
+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kLocationIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBackIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kForwardIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kReloadIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kWrenchIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+
+  // Hide the toolbar and ensure it is not visible.
+  [bar_ updateVisibility:NO withAnimation:NO];
+  EXPECT_TRUE([view isHidden]);
+  EXPECT_EQ(0, [resizeDelegate_ height]);
+  EXPECT_EQ(0, [[bar_ view] frame].size.height);
+
+  // Try to show the home button.
+  [bar_ showOptionalHomeButton];
+
+  // Re-show the bar. Buttons should remain hidden, including the home button.
+  [bar_ updateVisibility:YES withAnimation:NO];
+  EXPECT_GT([resizeDelegate_ height], 0);
+  EXPECT_GT([[bar_ view] frame].size.height, 0);
+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([GetSubviewAt(kLocationIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBackIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kForwardIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kReloadIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kWrenchIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kHomeIndex) isHidden]);
+  EXPECT_TRUE([GetSubviewAt(kBrowserActionContainerViewIndex) isHidden]);
+}
+
 // Make sure it works in the completely undecorated case.
 TEST_F(ToolbarControllerTest, NoLocationBar) {
   NSView* view = [bar_ view];
 
   [bar_ setHasToolbar:NO hasLocationBar:NO];
-  EXPECT_NE(view, [bar_ view]);
   EXPECT_TRUE([[bar_ view] isHidden]);
 
   // Simulate a popup going fullscreen and back by performing the reparenting
   // that happens during fullscreen transitions
   NSView* superview = [view superview];
   [view removeFromSuperview];
   [superview addSubview:view];
 }
 
 // Make some changes to the enabled state of a few of the buttons and ensure
@@ skipping 137 matching lines @@
   BrowserRemovedObserver observer;
   // This is normally called by BrowserWindowController, but since |bar_| is not
   // owned by one, call it here.
   [bar_ browserWillBeDestroyed];
   CloseBrowserWindow();
   observer.WaitUntilBrowserRemoved();
   // |bar_| is released in TearDown().
 }
 
 }  // namespace