ash.mojom.KeyboardController: Pass bounds as screen coordinates.

Bounds passed over mojo should be in Screen coordinates since
Chrome and Ash do not share a common window tree in Mash.

For comment-only change to keyboard_controller.mojom:
TBR=tsepez@chromium.org

Bug: 843332
Change-Id: I40cee828ee5872cf69c4589141a9bb79c7c08cee
Reviewed-on: https://chromium-review.googlesource.com/c/1351801
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611324}

ash/keyboard/ash_keyboard_controller.cc

@@ -14,6 +14,7 @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/keyboard_ui.h"
+#include "ui/wm/core/coordinate_conversion.h"
 
 using keyboard::mojom::KeyboardConfig;
 using keyboard::mojom::KeyboardConfigPtr;
@@ -264,15 +265,21 @@ void AshKeyboardController::OnKeyboardVisibilityStateChanged(bool is_visible) {
 
 void AshKeyboardController::OnKeyboardVisibleBoundsChanged(
     const gfx::Rect& bounds) {
-  observers_.ForAllPtrs([&bounds](mojom::KeyboardControllerObserver* observer) {
-    observer->OnKeyboardVisibleBoundsChanged(bounds);
+  // Pass the bounds in screen coordinates over mojo.
+  gfx::Rect screen_bounds = BoundsToScreen(bounds);
+  observers_.ForAllPtrs(
+      [&screen_bounds](mojom::KeyboardControllerObserver* observer) {
+        observer->OnKeyboardVisibleBoundsChanged(screen_bounds);
       });
 }
 
 void AshKeyboardController::OnKeyboardWorkspaceOccludedBoundsChanged(
     const gfx::Rect& bounds) {
-  observers_.ForAllPtrs([&bounds](mojom::KeyboardControllerObserver* observer) {
-    observer->OnKeyboardOccludedBoundsChanged(bounds);
+  // Pass the bounds in screen coordinates over mojo.
+  gfx::Rect screen_bounds = BoundsToScreen(bounds);
+  observers_.ForAllPtrs(
+      [&screen_bounds](mojom::KeyboardControllerObserver* observer) {
+        observer->OnKeyboardOccludedBoundsChanged(screen_bounds);
       });
 }
 
@@ -292,4 +299,12 @@ void AshKeyboardController::OnKeyboardEnabledChanged(bool is_enabled) {
       });
 }
 
+gfx::Rect AshKeyboardController::BoundsToScreen(const gfx::Rect& bounds) {
+  DCHECK(keyboard_controller_->GetKeyboardWindow());
+  gfx::Rect screen_bounds(bounds);
+  ::wm::ConvertRectToScreen(keyboard_controller_->GetKeyboardWindow(),
+                            &screen_bounds);
+  return screen_bounds;
+}
+
 }  // namespace ash

ash/keyboard/ash_keyboard_controller.h

@@ -116,6 +116,8 @@ class ASH_EXPORT AshKeyboardController
       override;
   void OnKeyboardEnabledChanged(bool is_enabled) override;
 
+  gfx::Rect BoundsToScreen(const gfx::Rect& bounds);
+
   SessionController* session_controller_;  // unowned
   std::unique_ptr<keyboard::KeyboardController> keyboard_controller_;
   std::unique_ptr<VirtualKeyboardController> virtual_keyboard_controller_;

ash/public/interfaces/keyboard_controller.mojom

@@ -34,11 +34,13 @@ interface KeyboardControllerObserver {
   // field is focused or blurred, or the user hides the keyboard.
   OnKeyboardVisibilityChanged(bool visible);
 
-  // Called when the keyboard bounds change.
-  OnKeyboardVisibleBoundsChanged(gfx.mojom.Rect new_bounds);
+  // Called when the keyboard bounds change. |screen_bounds| is in screen
+  // coordinates.
+  OnKeyboardVisibleBoundsChanged(gfx.mojom.Rect screen_bounds);
 
-  // Called when the keyboard occluded bounds change.
-  OnKeyboardOccludedBoundsChanged(gfx.mojom.Rect new_bounds);
+  // Called when the keyboard occluded bounds change. |screen_bounds| is in
+  // screen coordinates.
+  OnKeyboardOccludedBoundsChanged(gfx.mojom.Rect screen_bounds);
 };
 
 interface KeyboardController {

chrome/browser/ui/ash/chrome_keyboard_bounds_observer.cc

@@ -15,6 +15,7 @@
 #include "ui/base/ui_base_features.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/view.h"
+#include "ui/wm/core/coordinate_conversion.h"
 
 ChromeKeyboardBoundsObserver::ChromeKeyboardBoundsObserver(
     aura::Window* keyboard_window)
@@ -32,7 +33,9 @@ ChromeKeyboardBoundsObserver::~ChromeKeyboardBoundsObserver() {
 }
 
 void ChromeKeyboardBoundsObserver::OnKeyboardOccludedBoundsChanged(
-    const gfx::Rect& new_bounds) {
+    const gfx::Rect& screen_bounds) {
+  gfx::Rect new_bounds(screen_bounds);
+  wm::ConvertRectFromScreen(keyboard_window_, &new_bounds);
   UpdateOccludedBounds(
       ChromeKeyboardControllerClient::Get()->IsKeyboardOverscrollEnabled()
           ? new_bounds

chrome/browser/ui/ash/chrome_keyboard_bounds_observer.h

@@ -31,7 +31,7 @@ class ChromeKeyboardBoundsObserver
 
   // keyboard::ChromeKeyboardControllerClient::Observer:
   void OnKeyboardVisibilityChanged(bool visible) override {}
-  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& new_bounds) override;
+  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& screen_bounds) override;
 
  private:
   void AddObservedWindow(aura::Window* window);

chrome/browser/ui/ash/chrome_keyboard_controller_client.cc

@@ -281,7 +281,7 @@ void ChromeKeyboardControllerClient::OnKeyboardVisibilityChanged(bool visible) {
 }
 
 void ChromeKeyboardControllerClient::OnKeyboardVisibleBoundsChanged(
-    const gfx::Rect& bounds) {
+    const gfx::Rect& screen_bounds) {
   Profile* profile = GetProfile();
   extensions::EventRouter* router = extensions::EventRouter::Get(profile);
   // |router| may be null in tests.
@@ -290,12 +290,14 @@ void ChromeKeyboardControllerClient::OnKeyboardVisibleBoundsChanged(
     return;
   }
 
+  // Note: |bounds| is in screen coordinates, which is what we want to pass to
+  // the extension.
   auto event_args = std::make_unique<base::ListValue>();
   auto new_bounds = std::make_unique<base::DictionaryValue>();
-  new_bounds->SetInteger("left", bounds.x());
-  new_bounds->SetInteger("top", bounds.y());
-  new_bounds->SetInteger("width", bounds.width());
-  new_bounds->SetInteger("height", bounds.height());
+  new_bounds->SetInteger("left", screen_bounds.x());
+  new_bounds->SetInteger("top", screen_bounds.y());
+  new_bounds->SetInteger("width", screen_bounds.width());
+  new_bounds->SetInteger("height", screen_bounds.height());
   event_args->Append(std::move(new_bounds));
 
   auto event = std::make_unique<extensions::Event>(
@@ -306,9 +308,9 @@ void ChromeKeyboardControllerClient::OnKeyboardVisibleBoundsChanged(
 }
 
 void ChromeKeyboardControllerClient::OnKeyboardOccludedBoundsChanged(
-    const gfx::Rect& bounds) {
+    const gfx::Rect& screen_bounds) {
   for (auto& observer : observers_)
-    observer.OnKeyboardOccludedBoundsChanged(bounds);
+    observer.OnKeyboardOccludedBoundsChanged(screen_bounds);
 }
 
 Profile* ChromeKeyboardControllerClient::GetProfile() {

chrome/browser/ui/ash/chrome_keyboard_controller_client.h

@@ -41,7 +41,8 @@ class ChromeKeyboardControllerClient
 
     // Forwards the 'OnKeyboardOccludedBoundsChanged' mojo observer method.
     // This is used by overscrolling.
-    virtual void OnKeyboardOccludedBoundsChanged(const gfx::Rect& new_bounds) {}
+    virtual void OnKeyboardOccludedBoundsChanged(
+        const gfx::Rect& screen_bounds) {}
 
     // Notifies observers when the keyboard content (i.e. the extension) has
     // loaded. Note: if the content is already loaded when the observer is
@@ -125,8 +126,8 @@ class ChromeKeyboardControllerClient
   void OnKeyboardConfigChanged(
       keyboard::mojom::KeyboardConfigPtr config) override;
   void OnKeyboardVisibilityChanged(bool visible) override;
-  void OnKeyboardVisibleBoundsChanged(const gfx::Rect& bounds) override;
-  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& bounds) override;
+  void OnKeyboardVisibleBoundsChanged(const gfx::Rect& screen_bounds) override;
+  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& screen_bounds) override;
 
   // Returns either the test profile or the active user profile.
   Profile* GetProfile();