[MacViews] Show/hide top UI in fullscreen mode correctly

Unlike Windows and Linux, Mac allows top UI to show or hide in browser
fullscreen mode. Also this option can be toggled at runtime to make the
change. But in tab fullscreen mode, top UI should always hide. These
require propagating fullscreen and user preference changes, recording
the toolbar styles and applying the layout accordingly.

This CL makes sure the top chrome UI status is correct across different
fullscreen modes, and responds to user changes as well. Some tests are
also added.

BUG=851261,831219,852779
TEST=Pls check the toolbar/tabstrip show or hide correctly when
entering and exiting various fullscreen modes. Bookmark bar, slide-down
menu and traffic lights will be fixed later.

Change-Id: I78a2b68821d5e633ac5c2823f65390809a57dad5
Reviewed-on: https://chromium-review.googlesource.com/1102026
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569301}

chrome/browser/ui/cocoa/browser_window_controller.mm

@@ -1919,7 +1919,9 @@ willAnimateFromState:(BookmarkBar::State)oldState
 
 - (void)updateUIForTabFullscreen:
     (ExclusiveAccessContext::TabFullscreenState)state {
-  DCHECK([self isInAnyFullscreenMode]);
+  // Only need to update and layout toolbar in fullscreen mode.
+  if (![self isInAnyFullscreenMode])
+    return;
   [fullscreenToolbarController_
       layoutToolbarStyleIsExitingTabFullscreen:
           state == ExclusiveAccessContext::STATE_EXIT_TAB_FULLSCREEN];

chrome/browser/ui/exclusive_access/fullscreen_controller.cc

@@ -134,6 +134,10 @@ void FullscreenController::EnterFullscreenModeForTab(WebContents* web_contents,
 
   ExclusiveAccessContext* exclusive_access_context =
       exclusive_access_manager()->context();
+  // This is needed on Mac as entering into Tab Fullscreen might change the top
+  // UI style.
+  exclusive_access_context->UpdateUIForTabFullscreen(
+      ExclusiveAccessContext::STATE_ENTER_TAB_FULLSCREEN);
 
   if (!exclusive_access_context->IsFullscreen()) {
     // Normal -> Tab Fullscreen.
@@ -143,11 +147,8 @@ void FullscreenController::EnterFullscreenModeForTab(WebContents* web_contents,
   }
 
   // Browser Fullscreen -> Tab Fullscreen.
-  if (exclusive_access_context->IsFullscreen()) {
-    exclusive_access_context->UpdateUIForTabFullscreen(
-        ExclusiveAccessContext::STATE_ENTER_TAB_FULLSCREEN);
+  if (exclusive_access_context->IsFullscreen())
     state_prior_to_tab_fullscreen_ = STATE_BROWSER_FULLSCREEN;
-  }
 
   // We need to update the fullscreen exit bubble, e.g., going from browser
   // fullscreen to tab fullscreen will need to show different content.

chrome/browser/ui/views/frame/browser_frame_mac.h

@@ -26,6 +26,7 @@ class BrowserFrameMac : public views::NativeWidgetMac,
 
   // Overridden from views::NativeWidgetMac:
   int SheetPositionY() override;
+  void OnWindowFullscreenStateChange() override;
   void InitNativeWidget(const views::Widget::InitParams& params) override;
 
   // Overridden from NativeBrowserFrame:

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

@@ -62,6 +62,10 @@ int BrowserFrameMac::SheetPositionY() {
   return host_view_y - dialog_host->GetDialogPosition(gfx::Size()).y();
 }
 
+void BrowserFrameMac::OnWindowFullscreenStateChange() {
+  browser_view_->FullscreenStateChanged();
+}
+
 void BrowserFrameMac::InitNativeWidget(
     const views::Widget::InitParams& params) {
   views::NativeWidgetMac::InitNativeWidget(params);

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

@@ -81,6 +81,13 @@ bool BrowserNonClientFrameView::CaptionButtonsOnLeadingEdge() const {
   return false;
 }
 
+void BrowserNonClientFrameView::UpdateFullscreenTopUI(
+    bool is_exiting_fullscreen) {}
+
+bool BrowserNonClientFrameView::ShouldHideTopUIForFullscreen() const {
+  return frame()->IsFullscreen();
+}
+
 gfx::ImageSkia BrowserNonClientFrameView::GetIncognitoAvatarIcon() const {
   const SkColor icon_color = color_utils::PickContrastingColor(
       SK_ColorWHITE, gfx::kChromeIconGrey, GetFrameColor());

chrome/browser/ui/views/frame/browser_non_client_frame_view.h

@@ -65,6 +65,13 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
   // Returns the amount that the theme background should be inset.
   virtual int GetThemeBackgroundXInset() const = 0;
 
+  // Updates the top UI state to be hidden or shown in fullscreen according to
+  // the preference's state. Currently only used on Mac.
+  virtual void UpdateFullscreenTopUI(bool is_exiting_fullscreen);
+
+  // Returns whether the top UI should hide.
+  virtual bool ShouldHideTopUIForFullscreen() const;
+
   // Retrieves the icon to use in the frame to indicate an incognito window.
   gfx::ImageSkia GetIncognitoAvatarIcon() const;
 

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

@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/views/frame/avatar_button_manager.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/profiles/profile_indicator_icon.h"
+#include "components/prefs/pref_change_registrar.h"
 
 class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
  public:
@@ -21,6 +22,8 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
   gfx::Rect GetBoundsForTabStrip(views::View* tabstrip) const override;
   int GetTopInset(bool restored) const override;
   int GetThemeBackgroundXInset() const override;
+  void UpdateFullscreenTopUI(bool is_exiting_fullscreen) override;
+  bool ShouldHideTopUIForFullscreen() const override;
   void UpdateThrobber(bool running) override;
   int GetTabStripLeftInset() const override;
 
@@ -47,9 +50,32 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
   AvatarButtonStyle GetAvatarButtonStyle() const override;
 
  private:
+  // This enum class represents the appearance of the fullscreen toolbar, which
+  // includes the tab strip and omnibox. These values are logged in a histogram
+  // and shouldn't be renumbered or removed.
+  enum class FullscreenToolbarStyle {
+    // The toolbar is present. Moving the cursor to the top
+    // causes the menubar to appear and the toolbar to slide down.
+    kToolbarPresent = 0,
+    // The toolbar is hidden. Moving cursor to top shows the
+    // toolbar and menubar.
+    kToolbarHidden,
+    // Toolbar is hidden. Moving cursor to top causes the menubar
+    // to appear, but not the toolbar.
+    kToolbarNone,
+    // The last enum value. Used for logging in a histogram.
+    kToolbarLast = kToolbarNone
+  };
+
   void PaintThemedFrame(gfx::Canvas* canvas);
   int GetTabStripRightInset() const;
 
+  // Used to keep track of the update of kShowFullscreenToolbar preference.
+  PrefChangeRegistrar pref_registrar_;
+
+  // The style of the fullscreen toolbar.
+  FullscreenToolbarStyle toolbar_style_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserNonClientFrameViewMac);
 };
 

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

@@ -4,12 +4,16 @@
 
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h"
 
+#include "base/metrics/histogram_macros.h"
 #include "chrome/browser/themes/theme_properties.h"
+#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/browser_view_layout.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
@@ -26,7 +30,13 @@ constexpr int kTabstripTopInset = 8;
 BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac(
     BrowserFrame* frame,
     BrowserView* browser_view)
-    : BrowserNonClientFrameView(frame, browser_view) {}
+    : BrowserNonClientFrameView(frame, browser_view) {
+  pref_registrar_.Init(browser_view->GetProfile()->GetPrefs());
+  pref_registrar_.Add(
+      prefs::kShowFullscreenToolbar,
+      base::BindRepeating(&BrowserNonClientFrameViewMac::UpdateFullscreenTopUI,
+                          base::Unretained(this), false));
+}
 
 BrowserNonClientFrameViewMac::~BrowserNonClientFrameViewMac() {
 }
@@ -68,6 +78,40 @@ int BrowserNonClientFrameViewMac::GetThemeBackgroundXInset() const {
   return 0;
 }
 
+void BrowserNonClientFrameViewMac::UpdateFullscreenTopUI(
+    bool is_exiting_fullscreen) {
+  FullscreenToolbarStyle old_style = toolbar_style_;
+
+  // Update to the new toolbar style if needed.
+  FullscreenController* controller =
+      browser_view()->GetExclusiveAccessManager()->fullscreen_controller();
+  if ((controller->IsWindowFullscreenForTabOrPending() ||
+       controller->IsExtensionFullscreenOrPending()) &&
+      !is_exiting_fullscreen) {
+    toolbar_style_ = FullscreenToolbarStyle::kToolbarNone;
+  } else {
+    PrefService* prefs = browser_view()->GetProfile()->GetPrefs();
+    toolbar_style_ = prefs->GetBoolean(prefs::kShowFullscreenToolbar)
+                         ? FullscreenToolbarStyle::kToolbarPresent
+                         : FullscreenToolbarStyle::kToolbarHidden;
+  }
+
+  if (old_style != toolbar_style_) {
+    // 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);
+  }
+}
+
+bool BrowserNonClientFrameViewMac::ShouldHideTopUIForFullscreen() const {
+  return frame()->IsFullscreen()
+             ? toolbar_style_ != FullscreenToolbarStyle::kToolbarPresent
+             : false;
+}
+
 void BrowserNonClientFrameViewMac::UpdateThrobber(bool running) {
 }
 

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

@@ -951,14 +951,7 @@ bool BrowserView::ShouldHideUIForFullscreen() const {
   if (immersive_mode_controller_->IsEnabled())
     return false;
 
-#if defined(OS_MACOSX)
-  // On Mac, fullscreen mode still has top chrome UI such as Toolbar and
-  // Tabstrip unless the user explicitly turned that off.
-  return IsFullscreen() && !browser_->profile()->GetPrefs()->GetBoolean(
-                               prefs::kShowFullscreenToolbar);
-#else
-  return IsFullscreen();
-#endif
+  return frame_->GetFrameView()->ShouldHideTopUIForFullscreen();
 }
 
 bool BrowserView::IsFullscreen() const {
@@ -976,8 +969,12 @@ void BrowserView::RestoreFocus() {
 }
 
 void BrowserView::FullscreenStateChanged() {
-  CHECK(!IsFullscreen());
-  ProcessFullscreen(false, GURL(), EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE);
+  bool fullscreen = IsFullscreen();
+  ProcessFullscreen(
+      fullscreen, GURL(),
+      fullscreen
+          ? GetExclusiveAccessManager()->GetExclusiveAccessExitBubbleType()
+          : EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE);
 }
 
 void BrowserView::SetToolbarButtonProvider(ToolbarButtonProvider* provider) {
@@ -2749,6 +2746,11 @@ Profile* BrowserView::GetProfile() {
   return browser_->profile();
 }
 
+void BrowserView::UpdateUIForTabFullscreen(TabFullscreenState state) {
+  frame()->GetFrameView()->UpdateFullscreenTopUI(
+      state == ExclusiveAccessContext::STATE_EXIT_TAB_FULLSCREEN);
+}
+
 WebContents* BrowserView::GetActiveWebContents() {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }

chrome/browser/ui/views/frame/browser_view.h

@@ -256,7 +256,6 @@ class BrowserView : public BrowserWindow,
 
   // Called after the widget's fullscreen state is changed without going through
   // FullscreenController. This method does any processing which was skipped.
-  // Only exiting fullscreen in this way is currently supported.
   void FullscreenStateChanged();
 
   // Sets the button provider for this BrowserView. Must be called before
@@ -470,6 +469,7 @@ class BrowserView : public BrowserWindow,
 
   // ExclusiveAccessContext:
   Profile* GetProfile() override;
+  void UpdateUIForTabFullscreen(TabFullscreenState state) override;
   content::WebContents* GetActiveWebContents() override;
   void HideDownloadShelf() override;
   void UnhideDownloadShelf() override;

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

@@ -7,9 +7,11 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/views/scoped_macviews_browser_mode.h"
+#include "content/public/browser/web_contents.h"
 
 #if defined(OS_MACOSX)
 #include "chrome/browser/ui/browser_commands_mac.h"
@@ -54,8 +56,8 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, MAYBE_FullscreenClearsFocus) {
 }
 
 // Test whether the top view including toolbar and tab strip shows up or hides
-// correctly in full screen mode.
-IN_PROC_BROWSER_TEST_F(BrowserViewTest, FullscreenShowTopView) {
+// correctly in browser full screen mode.
+IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
 
   // The top view should always show up in regular mode.
@@ -80,6 +82,11 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, FullscreenShowTopView) {
   chrome::ToggleFullscreenMode(browser());
   EXPECT_TRUE(browser_view->IsFullscreen());
   EXPECT_FALSE(browser_view->IsTabStripVisible());
+
+  // Test toggling toolbar while being in fullscreen mode.
+  chrome::ToggleFullscreenToolbar(browser());
+  EXPECT_TRUE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
 #else
   // In immersive full screen mode, the top view should show up; otherwise, it
   // always hides.
@@ -88,4 +95,47 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, FullscreenShowTopView) {
   else
     EXPECT_FALSE(browser_view->IsTabStripVisible());
 #endif
+
+  // Enter into tab full screen mode from browser fullscreen mode.
+  FullscreenController* controller =
+      browser()->exclusive_access_manager()->fullscreen_controller();
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  controller->EnterFullscreenModeForTab(web_contents, GURL());
+  EXPECT_TRUE(browser_view->IsFullscreen());
+  if (browser_view->immersive_mode_controller()->IsEnabled())
+    EXPECT_TRUE(browser_view->IsTabStripVisible());
+  else
+    EXPECT_FALSE(browser_view->IsTabStripVisible());
+
+  // Return back to regular mode.
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_FALSE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
+}
+
+// Test whether the top view including toolbar and tab strip appears or hides
+// correctly in tab full screen mode.
+IN_PROC_BROWSER_TEST_F(BrowserViewTest, TabFullscreenShowTopView) {
+  BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+
+  // The top view should always show up in regular mode.
+  EXPECT_FALSE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
+
+  // Enter into tab full screen mode.
+  FullscreenController* controller =
+      browser()->exclusive_access_manager()->fullscreen_controller();
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  controller->EnterFullscreenModeForTab(web_contents, GURL());
+  EXPECT_TRUE(browser_view->IsFullscreen());
+
+  // The top view should not show up.
+  EXPECT_FALSE(browser_view->IsTabStripVisible());
+
+  // After exiting the fullscreen mode, the top view should show up again.
+  controller->ExitFullscreenModeForTab(web_contents);
+  EXPECT_FALSE(browser_view->IsFullscreen());
+  EXPECT_TRUE(browser_view->IsTabStripVisible());
 }

ui/views/cocoa/bridged_native_widget.mm

@@ -677,6 +677,9 @@ void BridgedNativeWidget::OnFullscreenTransitionStart(
   // If going into fullscreen, store an answer for GetRestoredBounds().
   if (target_fullscreen_state)
     bounds_before_fullscreen_ = gfx::ScreenRectFromNSRect([window_ frame]);
+
+  // Notify that fullscreen state changed.
+  native_widget_mac_->OnWindowFullscreenStateChange();
 }
 
 void BridgedNativeWidget::OnFullscreenTransitionComplete(

ui/views/widget/native_widget_mac.h

@@ -51,6 +51,9 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
   // from the bottom of the window.
   virtual int SheetPositionY();
 
+  // Notifies that the widget starts to enter or exit fullscreen mode.
+  virtual void OnWindowFullscreenStateChange() {}
+
   // internal::NativeWidgetPrivate:
   void InitNativeWidget(const Widget::InitParams& params) override;
   void OnWidgetInitDone() override;