Fix top-chrome slide visual properties synchronization

If the page exists in a maximized browser window before going to
tablet mode, the layout that results from going to tablet mode
does not change the size of the page viewport. Hence, the visual
properties of the renderer and the browser are not automatically
synchrnoized. But going to tablet mode enables the top-chrome
sliding feature (i.e. BrowserView::GetTopControlsHeight() now
returns a non-zero value).

This CL makees sure that we synchronize the visual properties
manually, so that the renderer can get the new top-controls
height whenever it changes.

BUG=867063
TEST=manual, added a browser test.

Change-Id: Ifbbc224e1de452b2582635ea8f9b4c3992a0dceb
Reviewed-on: https://chromium-review.googlesource.com/1217355Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590525}

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

@@ -111,6 +111,26 @@ void UpdateBrowserControlsStateShown(content::WebContents* web_contents,
                                        animate);
 }
 
+// Triggers a visual properties synchrnoization event on |contents|' main
+// frame's view's widget.
+void SynchronizeVisualProperties(content::WebContents* contents) {
+  DCHECK(contents);
+
+  content::RenderFrameHost* main_frame = contents->GetMainFrame();
+  if (!main_frame)
+    return;
+
+  auto* rvh = main_frame->GetRenderViewHost();
+  if (!rvh)
+    return;
+
+  auto* widget = rvh->GetWidget();
+  if (!widget)
+    return;
+
+  widget->SynchronizeVisualProperties();
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -129,19 +149,7 @@ class TopControlsSlideTabObserver : public content::WebContentsObserver {
     // browser's tabstrip, meaning that Browser is now the delegate of
     // |web_contents|. Updating the visual properties will now sync the correct
     // top chrome height in the renderer.
-    content::RenderFrameHost* main_frame = web_contents->GetMainFrame();
-    if (!main_frame)
-      return;
-
-    auto* rvh = main_frame->GetRenderViewHost();
-    if (!rvh)
-      return;
-
-    auto* widget = rvh->GetWidget();
-    if (!widget)
-      return;
-
-    widget->SynchronizeVisualProperties();
+    SynchronizeVisualProperties(web_contents);
   }
 
   ~TopControlsSlideTabObserver() override = default;
@@ -282,14 +290,20 @@ void TopControlsSlideControllerChromeOS::SetShownRatio(
 
 void TopControlsSlideControllerChromeOS::OnBrowserFullscreenStateWillChange(
     bool new_fullscreen_state) {
+  auto* active_web_contents = browser_view_->GetActiveWebContents();
   if (new_fullscreen_state && !browser_frame_is_fullscreen_ &&
       shown_ratio_ < 1.f) {
     // Immersive fullscreen mode could be about to start for this browser's
     // window. Therefore show the top-chrome immediately without animation.
-    ShowTopChrome(browser_view_->GetActiveWebContents(), false /* animate */);
+    ShowTopChrome(active_web_contents, false /* animate */);
   }
 
   browser_frame_is_fullscreen_ = new_fullscreen_state;
+
+  // Now that the state of this feature is changed, force the renderer to get
+  // the new top controls height by triggering a visual properties
+  // synchrnoization event.
+  SynchronizeVisualProperties(active_web_contents);
 }
 
 void TopControlsSlideControllerChromeOS::SetTopControlsGestureScrollInProgress(
@@ -311,10 +325,16 @@ void TopControlsSlideControllerChromeOS::SetTopControlsGestureScrollInProgress(
 }
 
 void TopControlsSlideControllerChromeOS::OnTabletModeToggled(bool enabled) {
+  auto* active_web_contents = browser_view_->GetActiveWebContents();
   if (!enabled)
-    ShowTopChrome(browser_view_->GetActiveWebContents(), false /* animate */);
+    ShowTopChrome(active_web_contents, false /* animate */);
 
   tablet_mode_enabled_ = enabled;
+
+  // Now that the state of this feature is changed, force the renderer to get
+  // the new top controls height by triggering a visual properties
+  // synchrnoization event.
+  SynchronizeVisualProperties(active_web_contents);
 }
 
 void TopControlsSlideControllerChromeOS::TabInsertedAt(

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

@@ -29,6 +29,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/test/browser_test_utils.h"
+#include "ipc/ipc_message_macros.h"
 #include "net/dns/mock_host_resolver.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "ui/aura/window.h"
@@ -695,4 +696,79 @@ IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest, DisplayRotation) {
   }
 }
 
+// Waits for receiving an IPC message from the render frame that the page state
+// has been updated. This makes sure that the renderer now sees the new top
+// controls height if it changed.
+class PageStateUpdateWaiter : content::WebContentsObserver {
+ public:
+  explicit PageStateUpdateWaiter(content::WebContents* contents)
+      : WebContentsObserver(contents) {}
+  ~PageStateUpdateWaiter() override = default;
+
+  void Wait() { run_loop_.Run(); }
+
+  // content::WebContentsObserver:
+  void NavigationEntryChanged(
+      const content::EntryChangedDetails& change_details) override {
+    // This notification is triggered upon receiving the
+    // |FrameHostMsg_UpdateState| message in the |RenderFrameHostImpl|, which
+    // indicates that the page state now has been updated, and we can now
+    // proceeed with testing gesture scrolls behavior.
+    run_loop_.Quit();
+  }
+
+ private:
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(PageStateUpdateWaiter);
+};
+
+IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest,
+                       TestScrollingMaximizedPageBeforeGoingToTabletMode) {
+  // If the page exists in a maximized browser window before going to tablet
+  // mode, the layout that results from going to tablet mode does not change
+  // the size of the page viewport. Hence, the visual properties of the renderer
+  // and the browser are not automatically synchrnoized. But going to tablet
+  // mode enables the top-chrome sliding feature (i.e.
+  // BrowserView::GetTopControlsHeight() now returns a non-zero value). We must
+  // make sure that we synchronize the visual properties manually, otherwise
+  // the renderer will never get the new top-controls height.
+  browser_view()->frame()->Maximize();
+
+  // Navigate to our test scrollable page.
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/top_controls_scroll.html"));
+  content::WebContents* active_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  TabNonEmptyPaintWaiter paint_waiter(active_contents);
+  paint_waiter.Wait();
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 1.f);
+  EXPECT_EQ(browser_view()->GetTopControlsHeight(), 0);
+
+  // Switch to tablet mode. This should trigger a synchronize visual properties
+  // event with the renderer so that it can get the correct top controls height
+  // now that the top-chrome slide feature is enabled. Otherwise hiding top
+  // chrome with gesture scrolls won't be possible at all.
+  PageStateUpdateWaiter page_state_update_waiter(active_contents);
+  ToggleTabletMode();
+  ASSERT_TRUE(GetTabletModeEnabled());
+  EXPECT_TRUE(top_controls_slide_controller()->IsEnabled());
+  EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 1.f);
+  EXPECT_NE(browser_view()->GetTopControlsHeight(), 0);
+  page_state_update_waiter.Wait();
+
+  // Scroll to fully hide top-chrome.
+  auto* browser_window = browser()->window()->GetNativeWindow();
+  ui::test::EventGenerator event_generator(browser_window->GetRootWindow(),
+                                           browser_window);
+  const gfx::Point start_point = event_generator.current_location();
+  const gfx::Point end_point = start_point + gfx::Vector2d(0, -100);
+  GenerateGestureFlingScrollSequence(&event_generator, start_point, end_point);
+  TopControlsShownRatioWaiter waiter(top_controls_slide_controller());
+  waiter.WaitForRatio(0.f);
+  EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 0);
+  CheckBrowserLayout(browser_view(), TopChromeShownState::kFullyHidden);
+}
+
 }  // namespace

testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

@@ -33,10 +33,11 @@
 
 # Touch gestures don't work in webcontents. https://crbug.com/866991.
 -TopControlsSlideControllerTest.DisplayRotation
--TopControlsSlideControllerTest.TestScrollingPage
--TopControlsSlideControllerTest.TestScrollingPageAndSwitchingToNTP
 -TopControlsSlideControllerTest.TestClosingATab
 -TopControlsSlideControllerTest.TestFocusEditableElements
+-TopControlsSlideControllerTest.TestScrollingMaximizedPageBeforeGoingToTabletMode
+-TopControlsSlideControllerTest.TestScrollingPage
+-TopControlsSlideControllerTest.TestScrollingPageAndSwitchingToNTP
 
 # Direct access to ash window frames, tablet mode, overview mode, etc.
 -NonHomeLauncherBrowserNonClientFrameViewAshTest.*