[VK] Observe bounds changes for overscrolling, rather than notify.

Currently when keyboard bounds change, KeyboardController notifies
the ChromeKeyboardUI to update the viewports of web contents on the
screen. This adds cruft to the KeyboardUI interface and will pose
problems for mash as well.

Alternatively, we could flip the relationship and have something in
Chrome observing keyboard bounds changes. This simplifies the
KeyboardUI interface and integrates nicely with the existing mojo
set up.

Bug: 845780
Change-Id: I0be697351f52619863f93e2b42dc263cd9dbab82
Reviewed-on: https://chromium-review.googlesource.com/c/1328481Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609444}

ash/keyboard/ash_keyboard_controller.cc

@@ -269,6 +269,13 @@ void AshKeyboardController::OnKeyboardVisibleBoundsChanged(
   });
 }
 
+void AshKeyboardController::OnKeyboardWorkspaceOccludedBoundsChanged(
+    const gfx::Rect& bounds) {
+  observers_.ForAllPtrs([&bounds](mojom::KeyboardControllerObserver* observer) {
+    observer->OnKeyboardOccludedBoundsChanged(bounds);
+  });
+}
+
 void AshKeyboardController::OnKeyboardEnableFlagsChanged(
     std::set<keyboard::mojom::KeyboardEnableFlag>& keyboard_enable_flags) {
   std::vector<keyboard::mojom::KeyboardEnableFlag> flags(

ash/keyboard/ash_keyboard_controller.h

@@ -109,6 +109,8 @@ class ASH_EXPORT AshKeyboardController
   void OnKeyboardConfigChanged() override;
   void OnKeyboardVisibilityStateChanged(bool is_visible) override;
   void OnKeyboardVisibleBoundsChanged(const gfx::Rect& bounds) override;
+  void OnKeyboardWorkspaceOccludedBoundsChanged(
+      const gfx::Rect& bounds) override;
   void OnKeyboardEnableFlagsChanged(
       std::set<keyboard::mojom::KeyboardEnableFlag>& keyboard_enable_flags)
       override;

ash/keyboard/ash_keyboard_controller_unittest.cc

@@ -48,6 +48,7 @@ class TestObserver : public mojom::KeyboardControllerObserver {
   }
   void OnKeyboardVisibilityChanged(bool visible) override {}
   void OnKeyboardVisibleBoundsChanged(const gfx::Rect& bounds) override {}
+  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& bounds) override {}
   void OnKeyboardConfigChanged(KeyboardConfigPtr config) override {
     config_ = *config;
   }

ash/keyboard/test_keyboard_ui.cc

@@ -48,7 +48,5 @@ ui::InputMethod* TestKeyboardUI::GetInputMethod() {
 }
 
 void TestKeyboardUI::ReloadKeyboardIfNeeded() {}
-void TestKeyboardUI::InitInsets(const gfx::Rect& keyboard_bounds) {}
-void TestKeyboardUI::ResetInsets() {}
 
 }  // namespace ash

ash/keyboard/test_keyboard_ui.h

@@ -31,8 +31,6 @@ class TestKeyboardUI : public keyboard::KeyboardUI {
   // Overridden from keyboard::KeyboardUI:
   ui::InputMethod* GetInputMethod() override;
   void ReloadKeyboardIfNeeded() override;
-  void InitInsets(const gfx::Rect& keyboard_bounds) override;
-  void ResetInsets() override;
 
   aura::test::TestWindowDelegate delegate_;
   std::unique_ptr<aura::Window> keyboard_window_;

ash/public/interfaces/keyboard_controller.mojom

@@ -36,6 +36,9 @@ interface KeyboardControllerObserver {
 
   // Called when the keyboard bounds change.
   OnKeyboardVisibleBoundsChanged(gfx.mojom.Rect new_bounds);
+
+  // Called when the keyboard occluded bounds change.
+  OnKeyboardOccludedBoundsChanged(gfx.mojom.Rect new_bounds);
 };
 
 interface KeyboardController {

chrome/browser/ui/ash/chrome_keyboard_bounds_observer.cc

@@ -7,7 +7,6 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
 #include "chrome/browser/ui/ash/chrome_keyboard_bounds_observer.h"
-#include "chrome/browser/ui/ash/chrome_keyboard_controller_client.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/render_widget_host_view.h"
@@ -18,10 +17,23 @@ ChromeKeyboardBoundsObserver::ChromeKeyboardBoundsObserver(
     aura::Window* keyboard_window)
     : keyboard_window_(keyboard_window) {
   DCHECK(keyboard_window_);
+  ChromeKeyboardControllerClient::Get()->AddObserver(this);
 }
 
 ChromeKeyboardBoundsObserver::~ChromeKeyboardBoundsObserver() {
+  UpdateOccludedBounds(gfx::Rect());
+
   RemoveAllObservedWindows();
+
+  ChromeKeyboardControllerClient::Get()->RemoveObserver(this);
+}
+
+void ChromeKeyboardBoundsObserver::OnKeyboardOccludedBoundsChanged(
+    const gfx::Rect& new_bounds) {
+  UpdateOccludedBounds(
+      ChromeKeyboardControllerClient::Get()->IsKeyboardOverscrollEnabled()
+          ? new_bounds
+          : gfx::Rect());
 }
 
 void ChromeKeyboardBoundsObserver::UpdateOccludedBounds(

chrome/browser/ui/ash/chrome_keyboard_bounds_observer.h

@@ -8,12 +8,15 @@
 #include <set>
 
 #include "base/macros.h"
+#include "chrome/browser/ui/ash/chrome_keyboard_controller_client.h"
 #include "ui/aura/window_observer.h"
 #include "ui/gfx/geometry/rect.h"
 
 // Class responsible for updating insets for windows overlapping the virtual
 // keyboard.
-class ChromeKeyboardBoundsObserver : public aura::WindowObserver {
+class ChromeKeyboardBoundsObserver
+    : public aura::WindowObserver,
+      public ChromeKeyboardControllerClient::Observer {
  public:
   explicit ChromeKeyboardBoundsObserver(aura::Window* keyboard_window);
   ~ChromeKeyboardBoundsObserver() override;
@@ -22,6 +25,10 @@ class ChromeKeyboardBoundsObserver : public aura::WindowObserver {
   // (i.e. by the KeyboardController through KeyboardUI::InitInsets).
   void UpdateOccludedBounds(const gfx::Rect& occluded_bounds);
 
+  // keyboard::ChromeKeyboardControllerClient::Observer:
+  void OnKeyboardVisibilityChanged(bool visible) override {}
+  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& new_bounds) override;
+
  private:
   void AddObservedWindow(aura::Window* window);
   void RemoveAllObservedWindows();

chrome/browser/ui/ash/chrome_keyboard_controller_client.cc

@@ -305,6 +305,12 @@ void ChromeKeyboardControllerClient::OnKeyboardVisibleBoundsChanged(
   router->BroadcastEvent(std::move(event));
 }
 
+void ChromeKeyboardControllerClient::OnKeyboardOccludedBoundsChanged(
+    const gfx::Rect& bounds) {
+  for (auto& observer : observers_)
+    observer.OnKeyboardOccludedBoundsChanged(bounds);
+}
+
 Profile* ChromeKeyboardControllerClient::GetProfile() {
   if (profile_for_test_)
     return profile_for_test_;

chrome/browser/ui/ash/chrome_keyboard_controller_client.h

@@ -39,6 +39,10 @@ class ChromeKeyboardControllerClient
     // This is used by oobe and login to adjust the UI.
     virtual void OnKeyboardVisibilityChanged(bool visible) {}
 
+    // Forwards the 'OnKeyboardOccludedBoundsChanged' mojo observer method.
+    // This is used by overscrolling.
+    virtual void OnKeyboardOccludedBoundsChanged(const gfx::Rect& new_bounds) {}
+
     // Notifies observers when the keyboard content (i.e. the extension) has
     // loaded. Note: if the content is already loaded when the observer is
     // added, this will not be triggered, but see is_keyboard_loaded().
@@ -122,6 +126,7 @@ class ChromeKeyboardControllerClient
       keyboard::mojom::KeyboardConfigPtr config) override;
   void OnKeyboardVisibilityChanged(bool visible) override;
   void OnKeyboardVisibleBoundsChanged(const gfx::Rect& bounds) override;
+  void OnKeyboardOccludedBoundsChanged(const gfx::Rect& bounds) override;
 
   // Returns either the test profile or the active user profile.
   Profile* GetProfile();

chrome/browser/ui/ash/chrome_keyboard_ui.cc

@@ -34,7 +34,6 @@ ChromeKeyboardUI::ChromeKeyboardUI(content::BrowserContext* context)
     : browser_context_(context) {}
 
 ChromeKeyboardUI::~ChromeKeyboardUI() {
-  ResetInsets();
   DCHECK(!keyboard_controller());
 }
 
@@ -84,16 +83,6 @@ void ChromeKeyboardUI::ReloadKeyboardIfNeeded() {
       ChromeKeyboardControllerClient::Get()->GetVirtualKeyboardUrl());
 }
 
-void ChromeKeyboardUI::InitInsets(const gfx::Rect& new_bounds) {
-  DCHECK(keyboard_contents_);
-  keyboard_contents_->window_bounds_observer()->UpdateOccludedBounds(
-      new_bounds);
-}
-
-void ChromeKeyboardUI::ResetInsets() {
-  InitInsets(gfx::Rect());
-}
-
 // aura::WindowObserver:
 
 void ChromeKeyboardUI::OnWindowBoundsChanged(aura::Window* window,

chrome/browser/ui/ash/chrome_keyboard_ui.h

@@ -38,8 +38,6 @@ class ChromeKeyboardUI : public keyboard::KeyboardUI,
   aura::Window* GetKeyboardWindow() const override;
   ui::InputMethod* GetInputMethod() override;
   void ReloadKeyboardIfNeeded() override;
-  void InitInsets(const gfx::Rect& new_bounds) override;
-  void ResetInsets() override;
 
   // aura::WindowObserver:
   void OnWindowBoundsChanged(aura::Window* window,

chrome/browser/ui/ash/chrome_keyboard_web_contents_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/run_loop.h"
+#include "chrome/browser/ui/ash/chrome_keyboard_controller_client.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/web_contents.h"
@@ -22,8 +23,16 @@ class ChromeKeyboardWebContentsTest : public ChromeRenderViewHostTestHarness {
   ChromeKeyboardWebContentsTest() = default;
   ~ChromeKeyboardWebContentsTest() override = default;
 
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    chrome_keyboard_controller_client_ =
+        std::make_unique<ChromeKeyboardControllerClient>(
+            nullptr /* connector */);
+  }
+
   void TearDown() override {
     chrome_keyboard_web_contents_.reset();
+    chrome_keyboard_controller_client_.reset();
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
@@ -34,6 +43,8 @@ class ChromeKeyboardWebContentsTest : public ChromeRenderViewHostTestHarness {
   }
 
  protected:
+  std::unique_ptr<ChromeKeyboardControllerClient>
+      chrome_keyboard_controller_client_;
   std::unique_ptr<ChromeKeyboardWebContents> chrome_keyboard_web_contents_;
 };
 

ui/keyboard/keyboard_controller.cc

@@ -350,11 +350,6 @@ void KeyboardController::NotifyKeyboardBoundsChanging(
     notification_manager_.SendNotifications(
         container_behavior_->OccludedBoundsAffectWorkspaceLayout(), new_bounds,
         occluded_bounds_in_screen, observer_list_);
-
-    if (IsKeyboardOverscrollEnabled())
-      ui_->InitInsets(occluded_bounds_in_screen);
-    else
-      ui_->ResetInsets();
   } else {
     visual_bounds_in_screen_ = gfx::Rect();
   }

ui/keyboard/keyboard_ui.h

@@ -13,9 +13,6 @@
 namespace aura {
 class Window;
 }
-namespace gfx {
-class Rect;
-}
 namespace ui {
 class InputMethod;
 }
@@ -70,13 +67,6 @@ class KEYBOARD_EXPORT KeyboardUI {
   // TODO(https://crbug.com/845780): Change this to accept a callback.
   virtual void ReloadKeyboardIfNeeded() = 0;
 
-  // When the embedder changes the keyboard bounds, asks the keyboard to adjust
-  // insets for windows affected by this.
-  virtual void InitInsets(const gfx::Rect& keyboard_bounds) = 0;
-
-  // Resets insets for affected windows.
-  virtual void ResetInsets() = 0;
-
   // |controller| may be null when KeyboardController is being destroyed.
   void SetController(KeyboardController* controller);
 

ui/keyboard/test/keyboard_test_util.h

@@ -45,8 +45,6 @@ class TestKeyboardUI : public KeyboardUI {
   aura::Window* GetKeyboardWindow() const override;
   ui::InputMethod* GetInputMethod() override;
   void ReloadKeyboardIfNeeded() override {}
-  void InitInsets(const gfx::Rect& keyboard_bounds) override {}
-  void ResetInsets() override {}
 
  private:
   std::unique_ptr<aura::Window> window_;