Delayed loading of the virtual keyboard.

ui/keyboard/keyboard_controller.cc

Index: ui/keyboard/keyboard_controller.cc
diff --git a/ui/keyboard/keyboard_controller.cc b/ui/keyboard/keyboard_controller.cc
index cdecdc36dfb5971d10b4bd14bd2cf23e53284acb..46e01eabee7e4906b39565c6841db03f2f97c2c5 100644
--- a/ui/keyboard/keyboard_controller.cc
+++ b/ui/keyboard/keyboard_controller.cc
@@ -9,7 +9,6 @@
 #include "ui/aura/window_delegate.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/ime/input_method.h"
-#include "ui/base/ime/input_method_base.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/base/ime/text_input_type.h"
 #include "ui/gfx/path.h"
@@ -28,38 +27,6 @@ gfx::Rect KeyboardBoundsFromWindowBounds(const gfx::Rect& window_bounds) {
       window_bounds.height() * kKeyboardHeightRatio);
 }
 
-// LayoutManager for the virtual keyboard container.  Manages a single window
-// (the virtual keyboard) and keeps it positioned at the bottom of the
-// container window.
-class KeyboardLayoutManager : public aura::LayoutManager {
- public:
-  KeyboardLayoutManager(aura::Window* owner, aura::Window* keyboard)
-    : owner_(owner), keyboard_(keyboard) {}
-
-  // Overridden from aura::LayoutManager
-  virtual void OnWindowResized() OVERRIDE {
-    SetChildBoundsDirect(keyboard_,
-                         KeyboardBoundsFromWindowBounds(owner_->bounds()));
-  }
-  virtual void OnWindowAddedToLayout(aura::Window* child) OVERRIDE {
-    CHECK(child == keyboard_);
-  }
-  virtual void OnWillRemoveWindowFromLayout(aura::Window* child) OVERRIDE {}
-  virtual void OnWindowRemovedFromLayout(aura::Window* child) OVERRIDE {}
-  virtual void OnChildWindowVisibilityChanged(aura::Window* child,
-                                              bool visible) OVERRIDE {}
-  virtual void SetChildBounds(aura::Window* child,
-                              const gfx::Rect& requested_bounds) OVERRIDE {
-    // Drop these: the size should only be set in OnWindowResized.
-  }
-
- private:
-  aura::Window* owner_;
-  aura::Window* keyboard_;
-
-  DISALLOW_COPY_AND_ASSIGN(KeyboardLayoutManager);
-};
-
 // The KeyboardWindowDelegate makes sure the keyboard-window does not get focus.
 // This is necessary to make sure that the synthetic key-events reach the target
 // window.
@@ -110,47 +77,101 @@ class KeyboardWindowDelegate : public aura::WindowDelegate {
 
 namespace keyboard {
 
+// LayoutManager for the virtual keyboard container.  Manages a single window
+// (the virtual keyboard) and keeps it positioned at the bottom of the
+// owner window.
+class KeyboardLayoutManager : public aura::LayoutManager {
+ public:
+  KeyboardLayoutManager(aura::Window* container)
+      : container_(container), keyboard_(NULL) {
+    CHECK(container_);
+  }
+
+  void AddKeyboard(aura::Window* keyboard) {
+    keyboard->Show();
+    container_->AddChild(keyboard);
+    OnWindowResized();
+  }
+
+  void ShowKeyboard() {
+    container_->parent()->StackChildAtTop(container_);
+    container_->Show();
+  }
+
+  void HideKeyboard() {
+    container_->Hide();
+  }
+
+  // Overridden from aura::LayoutManager
+  virtual void OnWindowResized() OVERRIDE {
+    if (!keyboard_)
+      return;
+    SetChildBoundsDirect(keyboard_,
+                         KeyboardBoundsFromWindowBounds(container_->bounds()));
+  }
+  virtual void OnWindowAddedToLayout(aura::Window* child) OVERRIDE {
+    DCHECK(!keyboard_);
+    keyboard_ = child;
+  }
+  virtual void OnWillRemoveWindowFromLayout(aura::Window* child) OVERRIDE {}
+  virtual void OnWindowRemovedFromLayout(aura::Window* child) OVERRIDE {}
+  virtual void OnChildWindowVisibilityChanged(aura::Window* child,
+                                              bool visible) OVERRIDE {}
+  virtual void SetChildBounds(aura::Window* child,
+                              const gfx::Rect& requested_bounds) OVERRIDE {
+    // Drop these: the size should only be set in OnWindowResized.
+  }
+
+ private:
+  aura::Window* container_;
+  aura::Window* keyboard_;
+
+  DISALLOW_COPY_AND_ASSIGN(KeyboardLayoutManager);
+};
+
 KeyboardController::KeyboardController(KeyboardControllerProxy* proxy)
-    : proxy_(proxy), container_(NULL) {
+    : proxy_(proxy),
+      container_(NULL),
+      layout_manager_(NULL),
+      input_method_(NULL) {
   CHECK(proxy);
-  proxy_->GetInputMethod()->AddObserver(this);
+  input_method_ = proxy_->GetInputMethod();
+  input_method_->AddObserver(this);
 }
 
 KeyboardController::~KeyboardController() {
-  if (container_)
-    container_->RemoveObserver(this);

[sadrul, 2013/04/15 16:49:37]

The KC does not own the |container_|. So it is possible that the KC owner
deletes the KC, but keeps the container_ around. In that case, the KC should
remove itself as an observer of the container. So this code (and setting
container_ to NULL in OnWindowDestroying) should still be here, I think.

[bryeung, 2013/04/15 17:42:36]

Oops!  Good catch.

-  proxy_->GetInputMethod()->RemoveObserver(this);

[sadrul, 2013/04/15 16:49:37]

I suppose it is possible that the root-window and/or the input-method has been
destroyed by the time the KC is destroyed? Perhaps there could be an
InputMethodObserver::OnInputMethodDestroyed()?

[bryeung, 2013/04/15 17:42:36]

Done.

+  input_method_->RemoveObserver(this);
 }
 
 aura::Window* KeyboardController::GetContainerWindow() {
-  if (!container_) {
+  if (!layout_manager_) {
+    CHECK(!container_);
     container_ = new aura::Window(new KeyboardWindowDelegate());
     container_->SetName("KeyboardContainer");
     container_->Init(ui::LAYER_NOT_DRAWN);
     container_->AddObserver(this);
 
-    aura::Window* keyboard = proxy_->GetKeyboardWindow();
-    keyboard->Show();
-
-    container_->SetLayoutManager(
-        new KeyboardLayoutManager(container_, keyboard));
-    container_->AddChild(keyboard);
+    layout_manager_ = new KeyboardLayoutManager(container_);
+    container_->SetLayoutManager(layout_manager_);
   }
+
+  CHECK(layout_manager_);
+  CHECK(container_);
   return container_;
 }
 
 void KeyboardController::OnTextInputStateChanged(
     const ui::TextInputClient* client) {
-  if (!container_)
+  if (!layout_manager_)
     return;
 
   if (!client || client->GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE) {
-    container_->Hide();
+    layout_manager_->HideKeyboard();
   } else {
-    container_->parent()->StackChildAtTop(container_);
-    container_->Show();
+    if (container_->children().empty())
+      layout_manager_->AddKeyboard(proxy_->GetKeyboardWindow());
+    layout_manager_->ShowKeyboard();

[sadrul, 2013/04/15 16:49:37]

I am not sure having |layout_manager_| is all that useful. Directly manipulating
container_ visibility should be sufficient? (unless you plan on using
|layout_manager_| for other things in the future)

[bryeung, 2013/04/15 17:42:36]

Done.

   }
-
   // TODO(bryeung): whenever the TextInputClient changes we need to notify the
   // keyboard (with the TextInputType) so that it can reset it's state (e.g.
   // abandon compositions in progress)
@@ -163,7 +184,7 @@ void KeyboardController::OnWindowParentChanged(aura::Window* window,
 
 void KeyboardController::OnWindowDestroying(aura::Window* window) {
   DCHECK_EQ(container_, window);
-  container_ = NULL;
+  container_->RemoveObserver(this);
 }
 
 }  // namespace keyboard