[MacViews] Track fullscreen bookmark bar state correctly

Mac browser fullscreen mode allows to keep the top UI, as well as
toggling between showing and hiding the top UI. In these cases, bookmark
bar should always show or hide in sync with the top UI. This requires
tracking top UI change, and change bookmark bar state or visibility
accordingly.

In Views, bookmark bar state is cached in Browser::bookmark_bar_state_,
and is updated through Browser::UpdateBookmarkBarState(). So when the
visibility of top UI changes, we need to execute
UpdateBookmarkBarState() to update bookmark bar state as well.

BUG=831219
TEST=pls check bookmark bar (non-detached) in fullscreen mode, which
     should always show or hide along with toolbar/tabstrip.

Change-Id: I04475edc455a62e045857ed666c44a31ead2b402
Reviewed-on: https://chromium-review.googlesource.com/1109405
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570324}

chrome/browser/ui/browser.cc

@@ -871,6 +871,10 @@ void Browser::WindowFullscreenStateChanged() {
   UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_TOGGLE_FULLSCREEN);
 }
 
+void Browser::FullscreenTopUIStateChanged() {
+  UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_TOOLBAR_OPTION_CHANGE);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, Assorted browser commands:
 
@@ -2482,6 +2486,10 @@ bool Browser::SupportsWindowFeatureImpl(WindowFeature feature,
 
   unsigned int features = FEATURE_INFOBAR | FEATURE_DOWNLOADSHELF;
 
+  // Bookmark bar could be present even if the top UI is hidden in fullscreen
+  // mode, such as when it is in 'detached' mode on NTP. Therefore we
+  // support this feature regardless of |hide_ui_for_fullscreen| and manage
+  // its visibility based on its own state.
   if (is_type_tabbed())
     features |= FEATURE_BOOKMARKBAR;
 

chrome/browser/ui/browser.h

@@ -385,11 +385,13 @@ class Browser : public TabStripModelObserver,
 
   // External state change handling ////////////////////////////////////////////
 
-  // BrowserWindow::EnterFullscreen invokes WindowFullscreenStateWillChange at
-  // the beginning of a fullscreen transition, and WindowFullscreenStateChanged
-  // at the end.
+  // WindowFullscreenStateWillChange is invoked at the beginning of a fullscreen
+  // transition, and WindowFullscreenStateChanged is at the end.
   void WindowFullscreenStateWillChange();
   void WindowFullscreenStateChanged();
+  // Only used on Mac. Called when the top ui style has been changed since this
+  // may trigger bookmark bar state change.
+  void FullscreenTopUIStateChanged();
 
   // Assorted browser commands ////////////////////////////////////////////////
 
@@ -572,6 +574,10 @@ class Browser : public TabStripModelObserver,
 
     // Change is the result of window toggling in/out of fullscreen mode.
     BOOKMARK_BAR_STATE_CHANGE_TOGGLE_FULLSCREEN,
+
+    // Change is the result of switching the option of showing toolbar in full
+    // screen. Only used on Mac.
+    BOOKMARK_BAR_STATE_CHANGE_TOOLBAR_OPTION_CHANGE,
   };
 
   // Overridden from content::WebContentsDelegate:

chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm

@@ -97,9 +97,14 @@ void BrowserNonClientFrameViewMac::UpdateFullscreenTopUI(
   }
 
   if (old_style != toolbar_style_) {
+    // Notify browser that top ui state has been changed so that we can update
+    // the bookmark bar state as well.
+    browser_view()->browser()->FullscreenTopUIStateChanged();
+
     // Re-layout if toolbar style changes in fullscreen mode.
     if (frame()->IsFullscreen())
       browser_view()->Layout();
+
     UMA_HISTOGRAM_ENUMERATION(
         "OSX.Fullscreen.ToolbarStyle", toolbar_style_,
         static_cast<int>(FullscreenToolbarStyle::kToolbarLast) + 1);

chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc

@@ -57,7 +57,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, MAYBE_FullscreenClearsFocus) {
 }
 
 // Test whether the top view including toolbar and tab strip shows up or hides
-// correctly in browser full screen mode.
+// correctly in browser fullscreen mode.
 IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
 #if defined(OS_MACOSX)
@@ -71,7 +71,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   EXPECT_FALSE(browser_view->IsFullscreen());
   EXPECT_TRUE(browser_view->IsTabStripVisible());
 
-  // Enter into full screen mode.
+  // Enter into fullscreen mode.
   chrome::ToggleFullscreenMode(browser());
   EXPECT_TRUE(browser_view->IsFullscreen());
 
@@ -85,7 +85,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   EXPECT_FALSE(browser_view->IsFullscreen());
   chrome::ToggleFullscreenToolbar(browser());
 
-  // While back to full screen mode, the top view no longer shows up.
+  // While back to fullscreen mode, the top view no longer shows up.
   chrome::ToggleFullscreenMode(browser());
   EXPECT_TRUE(browser_view->IsFullscreen());
   EXPECT_FALSE(browser_view->IsTabStripVisible());
@@ -95,7 +95,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   EXPECT_TRUE(browser_view->IsFullscreen());
   EXPECT_TRUE(browser_view->IsTabStripVisible());
 #else
-  // In immersive full screen mode, the top view should show up; otherwise, it
+  // In immersive fullscreen mode, the top view should show up; otherwise, it
   // always hides.
   if (browser_view->immersive_mode_controller()->IsEnabled())
     EXPECT_TRUE(browser_view->IsTabStripVisible());
@@ -103,7 +103,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
     EXPECT_FALSE(browser_view->IsTabStripVisible());
 #endif
 
-  // Enter into tab full screen mode from browser fullscreen mode.
+  // Enter into tab fullscreen mode from browser fullscreen mode.
   FullscreenController* controller =
       browser()->exclusive_access_manager()->fullscreen_controller();
   content::WebContents* web_contents =
@@ -122,7 +122,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
 }
 
 // Test whether the top view including toolbar and tab strip appears or hides
-// correctly in tab full screen mode.
+// correctly in tab fullscreen mode.
 IN_PROC_BROWSER_TEST_F(BrowserViewTest, TabFullscreenShowTopView) {
   BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
 
@@ -130,7 +130,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, TabFullscreenShowTopView) {
   EXPECT_FALSE(browser_view->IsFullscreen());
   EXPECT_TRUE(browser_view->IsTabStripVisible());
 
-  // Enter into tab full screen mode.
+  // Enter into tab fullscreen mode.
   FullscreenController* controller =
       browser()->exclusive_access_manager()->fullscreen_controller();
   content::WebContents* web_contents =
@@ -146,3 +146,49 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, TabFullscreenShowTopView) {
   EXPECT_FALSE(browser_view->IsFullscreen());
   EXPECT_TRUE(browser_view->IsTabStripVisible());
 }
+
+// Test whether bookmark bar shows up or hides correctly for fullscreen modes.
+IN_PROC_BROWSER_TEST_F(BrowserViewTest, FullscreenShowBookmarkBar) {
+  BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+
+  // If the bookmark bar is not showing, enable showing it so that we can check
+  // its state.
+  if (!browser_view->IsBookmarkBarVisible())
+    chrome::ToggleBookmarkBar(browser());
+#if defined(OS_MACOSX)
+  // Disable showing toolbar in fullscreen mode to make its bahavior similar to
+  // other platforms.
+  chrome::ToggleFullscreenToolbar(browser());
+#endif
+  AddTabAtIndex(0, GURL("about:blank"), ui::PAGE_TRANSITION_TYPED);
+
+  // Now the bookmark bar should show up in regular mode.
+  EXPECT_FALSE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsBookmarkBarVisible());
+
+  // Enter into fullscreen mode.
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_TRUE(browser_view->IsFullscreen());
+  if (browser_view->immersive_mode_controller()->IsEnabled())
+    EXPECT_TRUE(browser_view->IsBookmarkBarVisible());
+  else
+    EXPECT_FALSE(browser_view->IsBookmarkBarVisible());
+
+#if defined(OS_MACOSX)
+  // Test toggling toolbar state in fullscreen mode would also affect bookmark
+  // bar state.
+  chrome::ToggleFullscreenToolbar(browser());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
+  EXPECT_TRUE(browser_view->IsBookmarkBarVisible());
+
+  chrome::ToggleFullscreenToolbar(browser());
+  EXPECT_FALSE(browser_view->IsTabStripVisible());
+  EXPECT_FALSE(browser_view->IsBookmarkBarVisible());
+#endif
+
+  // Exit from fullscreen mode.
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_FALSE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
+  EXPECT_TRUE(browser_view->IsBookmarkBarVisible());
+}