Fix re-layout condition upon fullscreen style change

When Chrome's fullscreen toolbar style changes, it needs to re-layout to
make sure the UI is updated. This CL fixed a recent regression due to
using |is_exiting_fullscreen| to avoid re-layout. Instead, only when
Chrome exits fullscreen mode to non-fullscreen mode, we should avoid
doing re-layout because exiting tab fullscreen to browser fullscreen
still may requires a re-layout.

BUG=884029

Change-Id: I7392c2cb000b184c0f54b9cb6bb3de2b81a79dcf
Reviewed-on: https://chromium-review.googlesource.com/1226014
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592124}

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

@@ -95,7 +95,7 @@ bool BrowserNonClientFrameView::CaptionButtonsOnLeadingEdge() const {
 }
 
 void BrowserNonClientFrameView::UpdateFullscreenTopUI(
-    bool is_exiting_fullscreen) {}
+    bool needs_check_tab_fullscreen) {}
 
 bool BrowserNonClientFrameView::ShouldHideTopUIForFullscreen() const {
   return frame_->IsFullscreen();

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

@@ -82,7 +82,7 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
 
   // 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);
+  virtual void UpdateFullscreenTopUI(bool needs_check_tab_fullscreen);
 
   // Returns whether the top UI should hide.
   virtual bool ShouldHideTopUIForFullscreen() const;

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

@@ -26,7 +26,7 @@ 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;
+  void UpdateFullscreenTopUI(bool needs_check_tab_fullscreen) override;
   bool ShouldHideTopUIForFullscreen() const override;
   void UpdateThrobber(bool running) override;
   int GetTabStripLeftInset() const override;

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

@@ -47,7 +47,7 @@ BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac(
   pref_registrar_.Add(
       prefs::kShowFullscreenToolbar,
       base::BindRepeating(&BrowserNonClientFrameViewMac::UpdateFullscreenTopUI,
-                          base::Unretained(this), false));
+                          base::Unretained(this), true));
 }
 
 BrowserNonClientFrameViewMac::~BrowserNonClientFrameViewMac() {
@@ -65,7 +65,7 @@ void BrowserNonClientFrameViewMac::OnFullscreenStateChanged() {
     // Exiting tab fullscreen requires updating Top UI.
     // Called from here so we can capture exiting tab fullscreen both by
     // pressing 'ESC' key and by clicking green traffic light button.
-    UpdateFullscreenTopUI(true);
+    UpdateFullscreenTopUI(false);
     [fullscreen_toolbar_controller_ exitFullscreenMode];
   }
   browser_view()->Layout();
@@ -141,7 +141,7 @@ int BrowserNonClientFrameViewMac::GetThemeBackgroundXInset() const {
 }
 
 void BrowserNonClientFrameViewMac::UpdateFullscreenTopUI(
-    bool is_exiting_fullscreen) {
+    bool needs_check_tab_fullscreen) {
   FullscreenToolbarStyle old_style =
       [fullscreen_toolbar_controller_ toolbarStyle];
 
@@ -151,7 +151,7 @@ void BrowserNonClientFrameViewMac::UpdateFullscreenTopUI(
       browser_view()->GetExclusiveAccessManager()->fullscreen_controller();
   if ((controller->IsWindowFullscreenForTabOrPending() ||
        controller->IsExtensionFullscreenOrPending()) &&
-      !is_exiting_fullscreen) {
+      needs_check_tab_fullscreen) {
     new_style = FullscreenToolbarStyle::TOOLBAR_NONE;
   } else {
     new_style =
@@ -159,7 +159,8 @@ void BrowserNonClientFrameViewMac::UpdateFullscreenTopUI(
   }
   [fullscreen_toolbar_controller_ setToolbarStyle:new_style];
 
-  if (is_exiting_fullscreen || old_style == new_style)
+  if (![fullscreen_toolbar_controller_ isInFullscreen] ||
+      old_style == new_style)
     return;
 
   // Notify browser that top ui state has been changed so that we can update

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

@@ -2975,7 +2975,7 @@ Profile* BrowserView::GetProfile() {
 
 void BrowserView::UpdateUIForTabFullscreen(TabFullscreenState state) {
   frame()->GetFrameView()->UpdateFullscreenTopUI(
-      state == ExclusiveAccessContext::STATE_EXIT_TAB_FULLSCREEN);
+      state == ExclusiveAccessContext::STATE_ENTER_TAB_FULLSCREEN);
 }
 
 WebContents* BrowserView::GetActiveWebContents() {

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

@@ -9,6 +9,7 @@
 #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/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/views/scoped_macviews_browser_mode.h"
@@ -75,6 +76,7 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   chrome::ToggleFullscreenMode(browser());
   EXPECT_TRUE(browser_view->IsFullscreen());
 
+  bool top_view_in_browser_fullscreen = false;
 #if defined(OS_MACOSX)
   // The top view should show up by default.
   EXPECT_TRUE(browser_view->IsTabStripVisible());
@@ -93,15 +95,14 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   // Test toggling toolbar while being in fullscreen mode.
   chrome::ToggleFullscreenToolbar(browser());
   EXPECT_TRUE(browser_view->IsFullscreen());
-  EXPECT_TRUE(browser_view->IsTabStripVisible());
+  top_view_in_browser_fullscreen = true;
 #else
   // 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());
-  else
-    EXPECT_FALSE(browser_view->IsTabStripVisible());
+    top_view_in_browser_fullscreen = true;
 #endif
+  EXPECT_EQ(top_view_in_browser_fullscreen, browser_view->IsTabStripVisible());
 
   // Enter into tab fullscreen mode from browser fullscreen mode.
   FullscreenController* controller =
@@ -115,7 +116,19 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, BrowserFullscreenShowTopView) {
   else
     EXPECT_FALSE(browser_view->IsTabStripVisible());
 
-  // Return back to regular mode.
+  // Return back to browser fullscreen mode.
+  content::NativeWebKeyboardEvent event(
+      blink::WebInputEvent::kKeyDown, blink::WebInputEvent::kNoModifiers,
+      blink::WebInputEvent::GetStaticTimeStampForTests());
+  event.windows_key_code = ui::VKEY_ESCAPE;
+  browser()->exclusive_access_manager()->HandleUserKeyEvent(event);
+  EXPECT_TRUE(browser_view->IsFullscreen());
+  EXPECT_EQ(top_view_in_browser_fullscreen, browser_view->IsTabStripVisible());
+  // This makes sure that the layout was updated accordingly.
+  EXPECT_EQ(top_view_in_browser_fullscreen,
+            browser_view->tabstrip()->visible());
+
+  // Return to regular mode.
   chrome::ToggleFullscreenMode(browser());
   EXPECT_FALSE(browser_view->IsFullscreen());
   EXPECT_TRUE(browser_view->IsTabStripVisible());