[VK] Add browser tests for overscrolling.

Add a new browser test for checking that a maximized window responds
correctly to the virtual keyboard.

Had to change how WaitUntilShown and WaitUntilHidden worked.
|WaitUntilShown| used to just wait for the window visibility to change,
but KeyboardController sets the window visiblity at the beginning of
the show animation. So |WaitUntilShown| was more like
"WaitUntilStartingToShow".

This was a problem because overscrolling occurred after the animation
finishes, so the tests have to wait for animations to finish.

We changed it to wait instead of the keyboard visiblity as dictated
by the KeyboardController. This visiblity gets updated when the
show animation finishes. Unfortunately this is not true for the
hide animation, but it doesn't really affect our tests since
overscrolling also happens at the beginning of the hide animation.

Also see crbug.com/866332 for more about the different types of
"visibility".

Bug: 849995
Change-Id: I9c9dbee9423b63db3df58814e59eb41a9cab2d90
Reviewed-on: https://chromium-review.googlesource.com/1233314
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593105}

chrome/browser/ui/ash/keyboard_end_to_end_browsertest.cc

@@ -7,10 +7,13 @@
 #include "base/files/file.h"
 #include "chrome/browser/chromeos/input_method/textinput_test_helper.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/test/browser_test_utils.h"
+#include "ui/aura/test/mus/change_completion_waiter.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/keyboard_resource_util.h"
@@ -259,4 +262,42 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_FALSE(IsKeyboardShowing());
 }
 
+class KeyboardEndToEndOverscrollTest : public KeyboardEndToEndTest {
+ public:
+  KeyboardEndToEndOverscrollTest()
+      : KeyboardEndToEndTest(base::FilePath("form.html")) {}
+  ~KeyboardEndToEndOverscrollTest() override {}
+
+  void FocusAndShowKeyboard() { ClickElementWithId(web_contents, "username"); }
+
+  void HideKeyboard() { KeyboardController::Get()->HideKeyboardByUser(); }
+
+ protected:
+  int GetViewportHeight(content::WebContents* web_contents) {
+    return web_contents->GetRenderWidgetHostView()
+        ->GetVisibleViewportSize()
+        .height();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(KeyboardEndToEndOverscrollTest);
+};
+
+IN_PROC_BROWSER_TEST_F(KeyboardEndToEndOverscrollTest,
+                       ToggleKeyboardOnMaximizedWindowAffectsViewport) {
+  browser()->window()->Maximize();
+  aura::test::WaitForAllChangesToComplete();
+
+  const int old_height = GetViewportHeight(web_contents);
+
+  FocusAndShowKeyboard();
+  ASSERT_TRUE(WaitUntilShown());
+
+  EXPECT_LT(GetViewportHeight(web_contents), old_height);
+
+  HideKeyboard();
+  ASSERT_TRUE(WaitUntilHidden());
+
+  EXPECT_EQ(GetViewportHeight(web_contents), old_height);
+}
+
 }  // namespace keyboard

testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter

@@ -221,6 +221,7 @@
 -DefaultKeyboardExtensionBrowserTest.*
 -KeyboardEndToEndFocusTest.*
 -KeyboardEndToEndFormTest.*
+-KeyboardEndToEndOverscrollTest.*
 -KioskVirtualKeyboardTest.*
 -VirtualKeyboardAppWindowTest.*
 -VirtualKeyboardStateTest.*

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

@@ -15,6 +15,7 @@
 -DefaultKeyboardExtensionBrowserTest.*
 -KeyboardEndToEndFocusTest.*
 -KeyboardEndToEndFormTest.*
+-KeyboardEndToEndOverscrollTest.*
 -KioskVirtualKeyboardTest.*
 -VirtualKeyboardAppWindowTest.*
 -VirtualKeyboardStateTest.*

ui/keyboard/test/keyboard_test_util.cc

@@ -10,37 +10,38 @@
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/keyboard_controller_observer.h"
 
+namespace keyboard {
+
 namespace {
 
-class WindowVisibilityChangeWaiter : public aura::WindowObserver {
+class KeyboardVisibilityChangeWaiter : public KeyboardControllerObserver {
  public:
-  explicit WindowVisibilityChangeWaiter(aura::Window* window, bool wait_until)
-      : window_(window), wait_until_(wait_until) {
-    window_->AddObserver(this);
+  explicit KeyboardVisibilityChangeWaiter(bool wait_until)
+      : wait_until_(wait_until) {
+    KeyboardController::Get()->AddObserver(this);
+  }
+  ~KeyboardVisibilityChangeWaiter() override {
+    KeyboardController::Get()->RemoveObserver(this);
   }
-  ~WindowVisibilityChangeWaiter() override { window_->RemoveObserver(this); }
 
   void Wait() { run_loop_.Run(); }
 
  private:
-  void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
-    if (window_ == window && visible == wait_until_) {
+  void OnKeyboardVisibilityStateChanged(const bool is_visible) override {
+    if (is_visible == wait_until_)
       run_loop_.QuitWhenIdle();
-    }
   }
 
-  aura::Window* window_;
   base::RunLoop run_loop_;
-  bool const wait_until_;
+  const bool wait_until_;
 
-  DISALLOW_COPY_AND_ASSIGN(WindowVisibilityChangeWaiter);
+  DISALLOW_COPY_AND_ASSIGN(KeyboardVisibilityChangeWaiter);
 };
 
-class ControllerStateChangeWaiter
-    : public keyboard::KeyboardControllerObserver {
+class ControllerStateChangeWaiter : public KeyboardControllerObserver {
  public:
-  explicit ControllerStateChangeWaiter(keyboard::KeyboardControllerState state)
-      : controller_(keyboard::KeyboardController::Get()), state_(state) {
+  explicit ControllerStateChangeWaiter(KeyboardControllerState state)
+      : controller_(KeyboardController::Get()), state_(state) {
     controller_->AddObserver(this);
   }
   ~ControllerStateChangeWaiter() override { controller_->RemoveObserver(this); }
@@ -48,39 +49,41 @@ class ControllerStateChangeWaiter
   void Wait() { run_loop_.Run(); }
 
  private:
-  void OnStateChanged(const keyboard::KeyboardControllerState state) override {
+  void OnStateChanged(const KeyboardControllerState state) override {
     if (state == state_) {
       run_loop_.QuitWhenIdle();
     }
   }
 
   base::RunLoop run_loop_;
-  keyboard::KeyboardController* controller_;
-  keyboard::KeyboardControllerState state_;
+  KeyboardController* controller_;
+  KeyboardControllerState state_;
 
   DISALLOW_COPY_AND_ASSIGN(ControllerStateChangeWaiter);
 };
 
-bool WaitVisibilityChangesTo(bool visibility) {
-  aura::Window* keyboard_window =
-      keyboard::KeyboardController::Get()->GetKeyboardWindow();
-  if (keyboard_window->IsVisible() == visibility)
+bool WaitVisibilityChangesTo(bool wait_until) {
+  if (KeyboardController::Get()->IsKeyboardVisible() == wait_until)
     return true;
-  WindowVisibilityChangeWaiter waiter(keyboard_window, visibility);
+  KeyboardVisibilityChangeWaiter waiter(wait_until);
   waiter.Wait();
   return true;
 }
 
 }  // namespace
 
-namespace keyboard {
-
 bool WaitUntilShown() {
-  return WaitVisibilityChangesTo(true);
+  // KeyboardController send a visibility update once the show animation
+  // finishes.
+  return WaitVisibilityChangesTo(true /* wait_until */);
 }
 
 bool WaitUntilHidden() {
-  return WaitVisibilityChangesTo(false);
+  // Unlike |WaitUntilShown|, KeyboardController updates its visibility
+  // at the beginning of the hide animation. There's currently no way to
+  // actually detect when the hide animation finishes.
+  // TODO(https://crbug.com/849995): Find a proper solution to this.
+  return WaitVisibilityChangesTo(false /* wait_until */);
 }
 
 void WaitControllerStateChangesTo(KeyboardControllerState state) {
@@ -89,7 +92,7 @@ void WaitControllerStateChangesTo(KeyboardControllerState state) {
 }
 
 bool IsKeyboardShowing() {
-  auto* keyboard_controller = keyboard::KeyboardController::Get();
+  auto* keyboard_controller = KeyboardController::Get();
   DCHECK(keyboard_controller->enabled());
 
   // KeyboardController sets its state to SHOWN when it is about to show.
@@ -98,7 +101,7 @@ bool IsKeyboardShowing() {
 }
 
 bool IsKeyboardHiding() {
-  auto* keyboard_controller = keyboard::KeyboardController::Get();
+  auto* keyboard_controller = KeyboardController::Get();
   DCHECK(keyboard_controller->enabled());
 
   return keyboard_controller->GetStateForTest() ==

ui/keyboard/test/keyboard_test_util.h

@@ -17,12 +17,10 @@ class Rect;
 
 namespace keyboard {
 
-// Waits until the keyboard is shown. Return false if there is no keyboard
-// window created.
+// Waits until the keyboard is fully shown, with no pending animations.
 bool WaitUntilShown();
 
-// Waits until the keyboard is hidden. Return false if there is no keyboard
-// window created.
+// Waits until the keyboard starts to hide, with possible pending animations.
 bool WaitUntilHidden();
 
 // Waits until the keyboard state is changed to the given state.