Fix the issue that AppListView is moved downward in tablet mode

AppListView's bounds are stored as the property called
kVirtualKeyboardRestoreBoundsKey when the search box is activated in
clamshell mode. When the tablet mode is turned on, this property
is still kept in AppListView's native window. As a result, when closing
the virtual keyboard in tablet mode, the native window's bounds are
restored. It explains why we can see that AppListView is moved downward
when dismissing the virtual keyboard.

In tablet mode, AppListView should not be moved because of the virtual
keyboard visibility. So in this CL, clear that property when tablet
mode is turned on.

Test: ash_unittests
Bug: 944133
Change-Id: I714ebd50448a995f77bae759f6282efc8adc6df1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538983
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644445}

ash/app_list/app_list_controller_impl_unittest.cc

@@ -73,6 +73,10 @@ void DismissAppListNow() {
       base::TimeTicks::Now());
 }
 
+aura::Window* GetAppListViewNativeWindow() {
+  return GetAppListView()->get_fullscreen_widget_for_test()->GetNativeView();
+}
+
 }  // namespace
 
 class AppListControllerImplTest : public AshTestBase {
@@ -188,28 +192,51 @@ TEST_F(AppListControllerImplTest, CheckAppListViewBoundsWhenVKeyboardEnabled) {
   // Show the AppListView again. Check the following things:
   // (1) Virtual keyboard does not show.
   // (2) AppListView is in PEEKING state.
-  // (3) AppListView's bounds are the same with the preferred bounds for
+  // (3) AppListView's bounds are the same as the preferred bounds for
   // the PEEKING state.
   ShowAppListNow();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(app_list::AppListViewState::PEEKING,
             GetAppListView()->app_list_state());
   EXPECT_EQ(nullptr, GetVirtualKeyboardWindow());
-  gfx::Rect current_app_list_bounds_in_screen =
-      GetAppListView()
-          ->get_fullscreen_widget_for_test()
-          ->GetNativeView()
-          ->GetBoundsInScreen();
-  gfx::Rect expected_app_list_bounds_in_parent =
-      GetAppListView()->GetPreferredWidgetBoundsForState(
-          app_list::AppListViewState::PEEKING);
-
-  // The expected bounds are in the parent coordinate. But it should still be
-  // the same with the AppListView's bounds in screen coordinate. Because the
-  // parent window of the AppListView should have the same size with the display
-  // root window.
-  EXPECT_EQ(expected_app_list_bounds_in_parent,
-            current_app_list_bounds_in_screen);
+  EXPECT_EQ(GetAppListView()->GetPreferredWidgetBoundsForState(
+                app_list::AppListViewState::PEEKING),
+            GetAppListViewNativeWindow()->bounds());
+}
+
+// Verifies that in tablet mode, the AppListView has correct bounds when the
+// virtual keyboard is dismissed (see https://crbug.com/944133).
+TEST_F(AppListControllerImplTest, CheckAppListViewBoundsWhenDismissVKeyboard) {
+  Shell::Get()->ash_keyboard_controller()->SetEnableFlag(
+      keyboard::mojom::KeyboardEnableFlag::kShelfEnabled);
+
+  // Show the AppListView and click on the search box with mouse so the
+  // VirtualKeyboard is shown. Wait until the virtual keyboard shows.
+  ShowAppListNow();
+  GetSearchBoxView()->SetSearchBoxActive(true, ui::ET_MOUSE_PRESSED);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(GetVirtualKeyboardWindow()->IsVisible());
+
+  // Turn on the tablet mode. The virtual keyboard should still show.
+  Shell::Get()->tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  EXPECT_TRUE(IsTabletMode());
+  EXPECT_TRUE(GetVirtualKeyboardWindow()->IsVisible());
+
+  // Close the virtual keyboard. Wait until it is hidden.
+  Shell::Get()->ash_keyboard_controller()->HideKeyboard(
+      mojom::HideReason::kUser);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(nullptr, GetVirtualKeyboardWindow());
+
+  // Check the following things:
+  // (1) AppListView's state is FULLSCREEN_SEARCH
+  // (2) AppListView's bounds are the same as the preferred bounds for
+  // the FULLSCREEN_SEARCH state.
+  EXPECT_EQ(app_list::AppListViewState::FULLSCREEN_SEARCH,
+            GetAppListView()->app_list_state());
+  EXPECT_EQ(GetAppListView()->GetPreferredWidgetBoundsForState(
+                app_list::AppListViewState::FULLSCREEN_SEARCH),
+            GetAppListViewNativeWindow()->bounds());
 }
 
 class AppListControllerImplMetricsTest : public AshTestBase {

ash/app_list/views/app_list_view.cc

@@ -56,6 +56,7 @@
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 #include "ui/wm/core/coordinate_conversion.h"
+#include "ui/wm/core/ime_util_chromeos.h"
 #include "ui/wm/core/shadow_types.h"
 
 using ash::ColorProfileType;
@@ -1244,6 +1245,13 @@ void AppListView::OnTabletModeChanged(bool started) {
                ? AppListViewState::FULLSCREEN_SEARCH
                : AppListViewState::FULLSCREEN_ALL_APPS);
 
+  // In tablet mode, AppListView should not be moved because of the change in
+  // virtual keyboard's visibility.
+  if (started) {
+    fullscreen_widget_->GetNativeView()->ClearProperty(
+        wm::kVirtualKeyboardRestoreBoundsKey);
+  }
+
   // Update background color opacity.
   SetBackgroundShieldColor();
 

ash/app_list/views/app_list_view.h

@@ -35,6 +35,8 @@ class AnimationMetricsReporter;
 namespace ash {
 FORWARD_DECLARE_TEST(AppListControllerImplTest,
                      CheckAppListViewBoundsWhenVKeyboardEnabled);
+FORWARD_DECLARE_TEST(AppListControllerImplTest,
+                     CheckAppListViewBoundsWhenDismissVKeyboard);
 }
 
 namespace app_list {
@@ -302,6 +304,8 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
  private:
   FRIEND_TEST_ALL_PREFIXES(ash::AppListControllerImplTest,
                            CheckAppListViewBoundsWhenVKeyboardEnabled);
+  FRIEND_TEST_ALL_PREFIXES(ash::AppListControllerImplTest,
+                           CheckAppListViewBoundsWhenDismissVKeyboard);
 
   // A widget observer that is responsible for keeping the AppListView state up
   // to date on closing.