[Ash] Make Virtual Keyboard Easier to Use

Currently the Virtual Keyboard is difficult to use
when the Fullscreen Magnifier is enabled

Improvements include:
- Keeping the Virtual Keyboard in the same size and
  position when Fullscreen Magnifier is enabled. This
  is done by applying the magnifier's inverse transform
  on the Virtual Keyboard's container.
- Reduce the bottom margin to pan down the magnifier so
  that you can interact with the keyboard without
  panning.

This CL disables keyboard overscroll when the fullscreen
magnifier is enabled since overscroll would cause layout
issues.

Screenshot: https://screenshot.googleplex.com/BXnT2RG4A7h

Testing: ash_unittests
  - MagnificationControllerTest.TextfieldFocusedWithKeyboard
  - MagnificationControllerTest.KeyboardOverscrollDisabled

Bug: 347252
Change-Id: Ibcdd43573b211d6bad21b60bb19f427a58490c3f
Reviewed-on: https://chromium-review.googlesource.com/1009269
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: David Vallet <dvallet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561685}

ash/display/root_window_transformers.cc

@@ -40,22 +40,6 @@ gfx::Transform CreateRootWindowRotationTransform(
                                  info.GetActiveRotation(), display.bounds());
 }
 
-gfx::Transform CreateMagnifierTransform(aura::Window* root_window) {
-  MagnificationController* magnifier = Shell::Get()->magnification_controller();
-  float magnifier_scale = 1.f;
-  gfx::Point magnifier_offset;
-  if (magnifier && magnifier->IsEnabled()) {
-    magnifier_scale = magnifier->GetScale();
-    magnifier_offset = magnifier->GetWindowPosition();
-  }
-  gfx::Transform transform;
-  if (magnifier_scale != 1.f) {
-    transform.Scale(magnifier_scale, magnifier_scale);
-    transform.Translate(-magnifier_offset.x(), -magnifier_offset.y());
-  }
-  return transform;
-}
-
 gfx::Transform CreateInsetsAndScaleTransform(const gfx::Insets& insets,
                                              float device_scale_factor,
                                              float ui_scale) {
@@ -99,7 +83,12 @@ class AshRootWindowTransformer : public RootWindowTransformer {
       root_window_bounds_transform_ =
           root_window_bounds_transform_ * CreateMirrorTransform(display);
     }
-    transform_ = root_window_bounds_transform_ * CreateMagnifierTransform(root);
+
+    transform_ = root_window_bounds_transform_;
+    MagnificationController* magnifier =
+        Shell::Get()->magnification_controller();
+    if (magnifier)
+      transform_ *= magnifier->GetMagnifierTransform();
 
     CHECK(transform_.GetInverse(&invert_transform_));
   }

ash/magnifier/magnification_controller.cc

@@ -15,6 +15,7 @@
 #include "ash/host/root_window_transformer.h"
 #include "ash/magnifier/magnifier_scale_utils.h"
 #include "ash/public/cpp/config.h"
+#include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_port.h"
@@ -35,6 +36,7 @@
 #include "ui/gfx/geometry/point3_f.h"
 #include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/rect_conversions.h"
+#include "ui/keyboard/keyboard_controller.h"
 #include "ui/wm/core/compound_event_filter.h"
 #include "ui/wm/core/coordinate_conversion.h"
 
@@ -64,10 +66,16 @@ constexpr gfx::Tween::Type kCenterCaretAnimationTweenType = gfx::Tween::LINEAR;
 // input focus.
 constexpr int kMoveMagnifierDelayInMs = 10;
 
-// Threadshold of panning. If the cursor moves to within pixels (in DIP) of
+// Threshold of panning. If the cursor moves to within pixels (in DIP) of
 // |kCursorPanningMargin| from the edge, the view-port moves.
 constexpr int kCursorPanningMargin = 100;
 
+// Threshold of panning at the bottom when the virtual keyboard is up. If the
+// cursor moves to within pixels (in DIP) of |kKeyboardBottomPanningMargin| from
+// the bottom edge, the view-port moves. This is only used by
+// MoveMagnifierWindowFollowPoint() when |reduce_bottom_margin| is true.
+constexpr int kKeyboardBottomPanningMargin = 10;
+
 // Threadshold of panning. If the caret moves to within pixels (in DIP) of
 // |kCaretPanningMargin| from the edge, the view-port moves.
 constexpr int kCaretPanningMargin = 50;
@@ -168,6 +176,13 @@ void MagnificationController::SetEnabled(bool enabled) {
     RedrawKeepingMousePosition(kNonMagnifiedScale, true, false);
     is_enabled_ = enabled;
   }
+
+  // Keyboard overscroll creates layout issues with fullscreen magnification
+  // so it needs to be disabled when magnification is enabled.
+  // TODO(spqchan): Fix the keyboard overscroll issues.
+  keyboard::SetKeyboardOverscrollOverride(
+      is_enabled_ ? keyboard::KEYBOARD_OVERSCROLL_OVERRIDE_DISABLED
+                  : keyboard::KEYBOARD_OVERSCROLL_OVERRIDE_NONE);
 }
 
 bool MagnificationController::IsEnabled() const {
@@ -278,6 +293,17 @@ void MagnificationController::SwitchTargetRootWindow(
   root_window_->AddObserver(this);
 }
 
+gfx::Transform MagnificationController::GetMagnifierTransform() const {
+  gfx::Transform transform;
+  if (IsEnabled()) {
+    transform.Scale(scale_, scale_);
+    gfx::Point offset = GetWindowPosition();
+    transform.Translate(-offset.x(), -offset.y());
+  }
+
+  return transform;
+}
+
 void MagnificationController::OnCaretBoundsChanged(
     const ui::TextInputClient* client) {
   // caret bounds in screen coordinates.
@@ -310,7 +336,8 @@ void MagnificationController::OnCaretBoundsChanged(
     const int panning_margin = kCaretPanningMargin / scale_;
     MoveMagnifierWindowFollowPoint(caret_point_, panning_margin, panning_margin,
                                    visible_window_rect.width() / 2,
-                                   visible_window_rect.height() / 2);
+                                   visible_window_rect.height() / 2,
+                                   false /* reduce_bottom_margin */);
     return;
   }
 
@@ -594,26 +621,42 @@ bool MagnificationController::RedrawDIP(const gfx::PointF& position_in_dip,
   origin_.set_y(y);
   scale_ = scale;
 
-  // Creates transform matrix.
-  gfx::Transform transform;
-  // Flips the signs intentionally to convert them from the position of the
-  // magnification window.
-  transform.Scale(scale_, scale_);
-  transform.Translate(-origin_.x(), -origin_.y());
+  const ui::LayerAnimator::PreemptionStrategy strategy =
+      ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET;
+  const base::TimeDelta duration =
+      base::TimeDelta::FromMilliseconds(duration_in_ms);
 
-  ui::ScopedLayerAnimationSettings settings(
+  ui::ScopedLayerAnimationSettings root_layer_settings(
       root_window_->layer()->GetAnimator());
-  settings.AddObserver(this);
-  settings.SetPreemptionStrategy(
-      ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-  settings.SetTweenType(tween_type);
-  settings.SetTransitionDuration(
-      base::TimeDelta::FromMilliseconds(duration_in_ms));
+  root_layer_settings.AddObserver(this);
+  root_layer_settings.SetPreemptionStrategy(strategy);
+  root_layer_settings.SetTweenType(tween_type);
+  root_layer_settings.SetTransitionDuration(duration);
 
   display::Display display =
       display::Screen::GetScreen()->GetDisplayNearestWindow(root_window_);
   std::unique_ptr<RootWindowTransformer> transformer(
       CreateRootWindowTransformerForDisplay(root_window_, display));
+
+  // Inverse the transformation on the keyboard container so the keyboard will
+  // remain zoomed out. Apply the same animation settings to it.
+  // Note: if |scale_| is 1.0f, the transform matrix will be an identity matrix.
+  // Applying the inverse of an identity matrix will not change the
+  // transformation.
+  // TODO(spqchan): Find a way to sync the layer animations together.
+  aura::Window* virtual_keyboard_container =
+      root_window_->GetChildById(kShellWindowId_ImeWindowParentContainer);
+
+  gfx::Transform vk_transform;
+  if (GetMagnifierTransform().GetInverse(&vk_transform)) {
+    ui::ScopedLayerAnimationSettings vk_layer_settings(
+        virtual_keyboard_container->layer()->GetAnimator());
+    vk_layer_settings.SetPreemptionStrategy(strategy);
+    vk_layer_settings.SetTweenType(tween_type);
+    vk_layer_settings.SetTransitionDuration(duration);
+    virtual_keyboard_container->SetTransform(vk_transform);
+  }
+
   RootWindowController::ForWindow(root_window_)
       ->ash_host()
       ->SetRootWindowTransformer(std::move(transformer));
@@ -679,7 +722,16 @@ void MagnificationController::OnMouseMove(const gfx::Point& location) {
 
   gfx::Point mouse(location);
   int margin = kCursorPanningMargin / scale_;  // No need to consider DPI.
-  MoveMagnifierWindowFollowPoint(mouse, margin, margin, margin, margin);
+
+  // Reduce the bottom margin if the keyboard is visible.
+  bool reduce_bottom_margin = false;
+  if (keyboard::KeyboardController::GetInstance()) {
+    reduce_bottom_margin =
+        keyboard::KeyboardController::GetInstance()->keyboard_visible();
+  }
+
+  MoveMagnifierWindowFollowPoint(mouse, margin, margin, margin, margin,
+                                 reduce_bottom_margin);
 }
 
 void MagnificationController::AfterAnimationMoveCursorTo(
@@ -809,7 +861,8 @@ void MagnificationController::MoveMagnifierWindowFollowPoint(
     int x_panning_margin,
     int y_panning_margin,
     int x_target_margin,
-    int y_target_margin) {
+    int y_target_margin,
+    bool reduce_bottom_margin) {
   DCHECK(root_window_);
   bool start_zoom = false;
 
@@ -832,14 +885,24 @@ void MagnificationController::MoveMagnifierWindowFollowPoint(
   const int top = window_rect.y();
   const int bottom = window_rect.bottom();
 
+  // If |reduce_bottom_margin| is true, use kKeyboardBottomPanningMargin instead
+  // of |y_panning_margin|. This is to prevent the magnifier from panning when
+  // the user is trying to interact with the bottom of the keyboard.
+  const int bottom_panning_margin = reduce_bottom_margin
+                                        ? kKeyboardBottomPanningMargin / scale_
+                                        : y_panning_margin;
+
   int y_diff = 0;
   if (point.y() < top + y_panning_margin) {
     // Panning up.
     y_diff = point.y() - (top + y_target_margin);
     start_zoom = true;
-  } else if (bottom - y_panning_margin < point.y()) {
+  } else if (bottom - bottom_panning_margin < point.y()) {
     // Panning down.
-    y_diff = point.y() - (bottom - y_target_margin);
+    const int bottom_target_margin =
+        reduce_bottom_margin ? std::min(bottom_panning_margin, y_target_margin)
+                             : y_target_margin;
+    y_diff = point.y() - (bottom - bottom_target_margin);
     start_zoom = true;
   }
   int y = top + y_diff;
@@ -860,7 +923,17 @@ void MagnificationController::MoveMagnifierWindowCenterPoint(
     const gfx::Point& point) {
   DCHECK(root_window_);
 
-  const gfx::Rect window_rect = GetViewportRect();
+  gfx::Rect window_rect = GetViewportRect();
+
+  // Reduce the viewport bounds if the keyboard is up.
+  if (keyboard::KeyboardController::GetInstance()) {
+    gfx::Rect keyboard_rect = keyboard::KeyboardController::GetInstance()
+                                  ->GetContainerWindow()
+                                  ->bounds();
+    window_rect.set_height(window_rect.height() -
+                           keyboard_rect.height() / scale_);
+  }
+
   if (point == window_rect.CenterPoint())
     return;
 

ash/magnifier/magnification_controller.h

@@ -25,6 +25,10 @@ namespace aura {
 class Window;
 }  // namespace aura
 
+namespace gfx {
+class Transform;
+}
+
 namespace ui {
 class GestureProviderAura;
 }  // namespace ui
@@ -110,6 +114,10 @@ class ASH_EXPORT MagnificationController : public ui::EventHandler,
   void SwitchTargetRootWindow(aura::Window* new_root_window,
                               bool redraw_original_root);
 
+  // Returns the magnification transformation for the root window. If
+  // magnification is disabled, return an empty Transform.
+  gfx::Transform GetMagnifierTransform() const;
+
   // ui::InputMethodObserver:
   void OnFocus() override {}
   void OnBlur() override {}
@@ -215,15 +223,18 @@ class ASH_EXPORT MagnificationController : public ui::EventHandler,
   bool ProcessGestures();
 
   // Moves the view port when |point| is located within
-  // |x_panning_margin| and |y_pannin_margin| to the edge of the visible
+  // |x_panning_margin| and |y_panning_margin| to the edge of the visible
   // window region. The view port will be moved so that the |point| will be
   // moved to the point where it has |x_target_margin| and |y_target_margin|
-  // to the edge of the visible region.
+  // to the edge of the visible region. If |reduce_bottom_margin| is true,
+  // then a reduced value will be used as the |y_panning_margin| and
+  // |y_target_margin| for the bottom edge.
   void MoveMagnifierWindowFollowPoint(const gfx::Point& point,
                                       int x_panning_margin,
                                       int y_panning_margin,
                                       int x_target_margin,
-                                      int y_target_margin);
+                                      int y_target_margin,
+                                      bool reduce_bottom_margin);
 
   // Moves the view port to center |point| in magnifier screen.
   void MoveMagnifierWindowCenterPoint(const gfx::Point& point);

ash/magnifier/magnification_controller_unittest.cc

@@ -20,6 +20,7 @@
 #include "ui/events/event_handler.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/geometry/rect_conversions.h"
+#include "ui/keyboard/keyboard_controller.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/widget.h"
@@ -988,4 +989,63 @@ TEST_F(MagnificationControllerTest, ZoomsIntoCenter) {
               GetMagnificationController()->GetViewportRect().CenterPoint()));
 }
 
+// Tests to see if keyboard overscroll is disabled when fullscreen magnification
+// is enabled.
+TEST_F(MagnificationControllerTest, KeyboardOverscrollDisabled) {
+  GetMagnificationController()->SetEnabled(false);
+  bool old_keyboard_overscroll_value = keyboard::IsKeyboardOverscrollEnabled();
+
+  // Enable magnification. Keyboard overscroll should be disabled.
+  GetMagnificationController()->SetEnabled(true);
+  EXPECT_FALSE(keyboard::IsKeyboardOverscrollEnabled());
+
+  // Disable magnification. Keyboard overscroll should be back to the way it was
+  // before magnification was enabled.
+  GetMagnificationController()->SetEnabled(false);
+  EXPECT_EQ(keyboard::IsKeyboardOverscrollEnabled(),
+            old_keyboard_overscroll_value);
+}
+
+TEST_F(MagnificationControllerTest, TextfieldFocusedWithKeyboard) {
+  // Set up text input view.
+  text_input_helper_.CreateAndShowTextInputView(gfx::Rect(500, 200, 80, 80));
+  gfx::Rect text_input_bounds = text_input_helper_.GetTextInputViewBounds();
+
+  // Enables magnifier and confirm the viewport is at center.
+  GetMagnificationController()->SetEnabled(true);
+  EXPECT_EQ(2.0f, GetMagnificationController()->GetScale());
+  EXPECT_EQ("200,150 400x300", GetViewport().ToString());
+  EXPECT_FALSE(GetMagnificationController()->KeepFocusCentered());
+
+  GetMagnificationController()->SetKeepFocusCentered(true);
+
+  // Set up and show the keyboard.
+  keyboard::SetAccessibilityKeyboardEnabled(true);
+  ash::Shell::Get()->CreateKeyboard();
+  keyboard::KeyboardController* keyboard_controller =
+      keyboard::KeyboardController::GetInstance();
+  keyboard_controller->ShowKeyboard(true);
+
+  gfx::Rect keyboard_bounds = gfx::Rect(0, 300, 800, 300);
+  keyboard_controller->GetContainerWindow()->SetBounds(keyboard_bounds);
+
+  // Focus on the text input field.
+  text_input_helper_.FocusOnTextInputView();
+  base::RunLoop().RunUntilIdle();
+
+  // Verify that the caret bounds is centered in the area above the keyboard.
+  gfx::Rect viewport_outside_keyboard_bounds = GetViewport();
+  viewport_outside_keyboard_bounds.set_height(
+      viewport_outside_keyboard_bounds.height() -
+      keyboard_bounds.height() / GetMagnificationController()->GetScale());
+
+  gfx::Rect caret_bounds = text_input_helper_.GetCaretBounds();
+
+  EXPECT_TRUE(GetMagnificationController()->KeepFocusCentered());
+  EXPECT_TRUE(viewport_outside_keyboard_bounds.Contains(text_input_bounds));
+  EXPECT_TRUE(text_input_bounds.Contains(caret_bounds.CenterPoint()));
+  EXPECT_EQ(caret_bounds.CenterPoint(),
+            viewport_outside_keyboard_bounds.CenterPoint());
+}
+
 }  // namespace ash

testing/buildbot/filters/mash.ash_unittests.filter

@@ -145,6 +145,7 @@
 -MagnificationControllerTest.MoveToSecondDisplayWithTouch
 -MagnificationControllerTest.PanWindowToLeft
 -MagnificationControllerTest.PanWindowToRight
+-MagnificationControllerTest.TextfieldFocusedWithKeyboard
 
 # TODO: CursorManagerTestApi. http://crbug.com/780637
 -MirrorWindowControllerTest.MirrorCursorBasic