[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 41 matching lines @@
   // |-toolbarViews| method.
   enum {
     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);
   }
 
+  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

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

Make sure you test this to ensure it still behaves

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

This should now be unaffected by the showLocationBarOnly call; the only change
is that it's the toolbarView being used, not locationBarView.

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

So did you open site that does HTML5 fullscreen in a popup, and ask it to go
fullscreen?

The issue is that there used to be code around this to resolve ownership
problems that came about with the weird locationView hack. The hierarchy is now
changed, so ownership patters could break.

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

I tested a site with HTML5 fullscreen in a popup and it seemed to work the same
as without the patch.

   // that happens during fullscreen transitions
   NSView* superview = [view superview];
   [view removeFromSuperview];
   [superview addSubview:view];
 
   [bar_ setHasToolbar:YES hasLocationBar:YES];
   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];
+  [bar_ setHasToolbar:NO hasLocationBar:YES];

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

Before calling this, EXPECT_FOO the button visibility for the other states
(which *can* be returned from). I.e., do

setHasToolbar:YES hasLocationBar:YES
setHasToolbar:NO hasLocationBar:NO
setHasToolbar:NO hasLocationBar:YES

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

Done.

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

Ping

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

Oops. Done.

+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([[[bar_ toolbarViews] objectAtIndex:kLocationIndex] isHidden]);

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

This is a lot of repetition. Since bar_ is a data member, I think nicest would
be a helper method on ToolbarControllerTest like

NSView* ToolbarControllerTest::GetSubviewAt(SubviewIndex index) {..}

(after giving the enum a name)

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

Done.

+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kBackIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kForwardIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kReloadIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kWrenchIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kHomeIndex] isHidden]);
+
+  // Maintain visible state.
+  [bar_ updateVisibility:YES withAnimation:NO];
+  EXPECT_GT([[bar_ view] frame].size.height, 0);
+  EXPECT_FALSE([view isHidden]);
+  EXPECT_FALSE([[[bar_ toolbarViews] objectAtIndex:kLocationIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kBackIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kForwardIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kReloadIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kWrenchIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kHomeIndex] isHidden]);

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

what about the browser action container?

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

I believe that for popup and bookmark app windows there is no browser action
controller.

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

Can we include it in the test?

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

Done.

+
+  // 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([[[bar_ toolbarViews] objectAtIndex:kLocationIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kBackIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kForwardIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kReloadIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kWrenchIndex] isHidden]);
+  EXPECT_TRUE([[[bar_ toolbarViews] objectAtIndex:kHomeIndex] 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