Keep virtual keyboard visibility the same after enable an IME in a different extension

BUG=404340

Review URL: https://codereview.chromium.org/487253003

Cr-Commit-Position: refs/heads/master@{#291678}

chrome/browser/ui/ash/keyboard_controller_browsertest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/web_contents.h"
+#include "ui/base/ime/dummy_text_input_client.h"
+#include "ui/base/ime/input_method.h"
+#include "ui/base/ime/input_method_factory.h"
+#include "ui/keyboard/keyboard_constants.h"
+#include "ui/keyboard/keyboard_controller.h"
+#include "ui/keyboard/keyboard_controller_proxy.h"
+#include "ui/keyboard/keyboard_switches.h"
+#include "ui/keyboard/keyboard_util.h"
+
+namespace {
+const int kKeyboardHeightForTest = 100;
+}  // namespace
+
+class VirtualKeyboardWebContentTest : public InProcessBrowserTest {
+ public:
+  VirtualKeyboardWebContentTest() {};
+  virtual ~VirtualKeyboardWebContentTest() {};
+
+  virtual void SetUp() OVERRIDE {
+    ui::SetUpInputMethodFactoryForTesting();
+    InProcessBrowserTest::SetUp();
+  }
+
+  // Ensure that the virtual keyboard is enabled.
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(
+        keyboard::switches::kEnableVirtualKeyboard);
+  }
+
+  keyboard::KeyboardControllerProxy* proxy() {
+    return keyboard::KeyboardController::GetInstance()->proxy();
+  }
+
+ protected:
+  void FocusEditableNodeAndShowKeyboard() {
+    client.reset(new ui::DummyTextInputClient(ui::TEXT_INPUT_TYPE_TEXT));
+    ui::InputMethod* input_method = proxy()->GetInputMethod();
+    input_method->SetFocusedTextInputClient(client.get());
+    input_method->ShowImeIfNeeded();
+    ResizeKeyboardWindow();
+  }
+
+  void FocusNonEditableNode() {
+    client.reset(new ui::DummyTextInputClient(ui::TEXT_INPUT_TYPE_NONE));
+    ui::InputMethod* input_method = proxy()->GetInputMethod();
+    input_method->SetFocusedTextInputClient(client.get());
+  }
+
+  void MockEnableIMEInDifferentExtension(const std::string& url) {
+    keyboard::SetOverrideContentUrl(GURL(url));
+    keyboard::KeyboardController::GetInstance()->Reload();
+    ResizeKeyboardWindow();
+  }
+
+  bool IsKeyboardVisible() const {
+    return keyboard::KeyboardController::GetInstance()->keyboard_visible();
+  }
+
+ private:
+  // Mock window.resizeTo that is expected to be called after navigate to a new
+  // virtual keyboard.
+  void ResizeKeyboardWindow() {
+    gfx::Rect bounds = proxy()->GetKeyboardWindow()->bounds();
+    proxy()->GetKeyboardWindow()->SetBounds(gfx::Rect(
+        bounds.x(),
+        bounds.bottom() - kKeyboardHeightForTest,
+        bounds.width(),
+        kKeyboardHeightForTest));
+  }
+
+  scoped_ptr<ui::DummyTextInputClient> client;
+
+  DISALLOW_COPY_AND_ASSIGN(VirtualKeyboardWebContentTest);
+};
+
+// Test for crbug.com/404340. After enabling an IME in a different extension,
+// its virtual keyboard should not become visible if previous one is not.
+IN_PROC_BROWSER_TEST_F(VirtualKeyboardWebContentTest,
+                       EnableIMEInDifferentExtension) {
+  FocusEditableNodeAndShowKeyboard();
+  EXPECT_TRUE(IsKeyboardVisible());
+  FocusNonEditableNode();
+  EXPECT_FALSE(IsKeyboardVisible());
+
+  MockEnableIMEInDifferentExtension("chrome-extension://domain-1");
+  // Keyboard should not become visible if previous keyboard is not.
+  EXPECT_FALSE(IsKeyboardVisible());
+
+  FocusEditableNodeAndShowKeyboard();
+  // Keyboard should become visible after focus on an editable node.
+  EXPECT_TRUE(IsKeyboardVisible());
+
+  // Simulate hide keyboard by pressing hide key on the virtual keyboard.
+  keyboard::KeyboardController::GetInstance()->HideKeyboard(
+      keyboard::KeyboardController::HIDE_REASON_MANUAL);
+  EXPECT_FALSE(IsKeyboardVisible());
+
+  MockEnableIMEInDifferentExtension("chrome-extension://domain-2");
+  // Keyboard should not become visible if previous keyboard is not, even if it
+  // is currently focused on an editable node.
+  EXPECT_FALSE(IsKeyboardVisible());
+}

chrome/chrome_tests.gypi

@@ -1402,6 +1402,7 @@
         'browser/ui/ash/accelerator_commands_browsertest.cc',
         'browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc',
         'browser/ui/ash/launcher/launcher_favicon_loader_browsertest.cc',
+        'browser/ui/ash/keyboard_controller_browsertest.cc',
         'browser/ui/ash/shelf_browsertest.cc',
         'browser/ui/ash/volume_controller_browsertest_chromeos.cc',
         'browser/ui/autofill/autofill_dialog_controller_browsertest.cc',
@@ -1807,6 +1808,7 @@
             'browser/invalidation/profile_invalidation_provider_factory_browsertest.cc',
             'browser/net/nss_context_chromeos_browsertest.cc',
             'browser/notifications/login_state_notification_blocker_chromeos_browsertest.cc',
+            'browser/ui/ash/keyboard_controller_browsertest.cc',
             'browser/ui/views/select_file_dialog_extension_browsertest.cc',
             'test/data/webui/certificate_viewer_dialog_test.js',
             'test/data/webui/certificate_viewer_ui_test-inl.h',

ui/keyboard/keyboard_controller.cc

@@ -206,6 +206,7 @@ class WindowBoundsChangeObserver : public aura::WindowObserver {
   virtual void OnWindowBoundsChanged(aura::Window* window,
                                      const gfx::Rect& old_bounds,
                                      const gfx::Rect& new_bounds) OVERRIDE;
+  virtual void OnWindowDestroyed(aura::Window* window) OVERRIDE;
 };
 
 void WindowBoundsChangeObserver::OnWindowBoundsChanged(aura::Window* window,
@@ -215,6 +216,11 @@ void WindowBoundsChangeObserver::OnWindowBoundsChanged(aura::Window* window,
     controller->UpdateWindowInsets(window);
 }
 
+void WindowBoundsChangeObserver::OnWindowDestroyed(aura::Window* window) {
+   if (window->HasObserver(this))
+     window->RemoveObserver(this);
+}
+
 // static
 KeyboardController* KeyboardController::instance_ = NULL;
 
@@ -288,7 +294,10 @@ void KeyboardController::NotifyKeyboardBoundsChanging(
         // the render process crashed.
         if (view) {
           aura::Window *window = view->GetNativeView();
-          if (window != keyboard_window &&
+          // If virtual keyboard failed to load, a widget that displays error
+          // message will be created and adds as a child of the virtual keyboard
+          // window. We want to avoid add BoundsChangedObserver to that window.
+          if (GetFrameWindow(window) != keyboard_window &&
               window->GetRootWindow() == root_window) {
             gfx::Rect window_bounds = window->GetBoundsInScreen();
             gfx::Rect intersect = gfx::IntersectRects(window_bounds,
@@ -360,8 +369,12 @@ void KeyboardController::OnWindowHierarchyChanged(
 }
 
 void KeyboardController::Reload() {
-  if (proxy_->HasKeyboardWindow())
+  if (proxy_->HasKeyboardWindow()) {
+    // A reload should never try to show virtual keyboard. If keyboard is not
+    // visible before reload, it should keep invisible after reload.
+    show_on_resize_ = false;
     proxy_->ReloadKeyboardIfNeeded();
+  }
 }
 
 void KeyboardController::OnTextInputStateChanged(
@@ -453,8 +466,12 @@ void KeyboardController::ShowKeyboardInternal() {
 
   proxy_->ReloadKeyboardIfNeeded();
 
-  if (keyboard_visible_ || proxy_->GetKeyboardWindow()->bounds().height() == 0)
+  if (keyboard_visible_) {
     return;
+  } else if (proxy_->GetKeyboardWindow()->bounds().height() == 0) {
+    show_on_resize_ = true;
+    return;
+  }
 
   keyboard_visible_ = true;
 

ui/keyboard/keyboard_controller.h

@@ -93,6 +93,8 @@ class KEYBOARD_EXPORT KeyboardController : public ui::InputMethodObserver,
   // Returns true if keyboard is currently visible.
   bool keyboard_visible() { return keyboard_visible_; }
 
+  bool show_on_resize() { return show_on_resize_; }
+
   // Returns the current keyboard bounds. When the keyboard is not shown,
   // an empty rectangle will get returned.
   const gfx::Rect& current_keyboard_bounds() {
@@ -153,6 +155,7 @@ class KEYBOARD_EXPORT KeyboardController : public ui::InputMethodObserver,
 
   ui::InputMethod* input_method_;
   bool keyboard_visible_;
+  bool show_on_resize_;
   bool lock_keyboard_;
   ui::TextInputType type_;
 

ui/keyboard/keyboard_layout_manager.cc

@@ -49,11 +49,11 @@ void KeyboardLayoutManager::SetChildBounds(aura::Window* child,
 
   gfx::Rect old_bounds = child->bounds();
   SetChildBoundsDirect(child, requested_bounds);
-  if (old_bounds.height() == 0 && child->bounds().height() != 0) {
-    // The window height is set to 0 initially. If the height of |old_bounds| is
-    // 0 and the new bounds is not 0, it probably means window.resizeTo is
-    // called to set the window height. We should try to show keyboard again in
-    // case the show keyboard request is called before the height is set.
+  if (old_bounds.height() == 0 && child->bounds().height() != 0 &&
+      controller_->show_on_resize()) {
+    // The window height is set to 0 initially or before switch to an IME in a
+    // different extension. Virtual keyboard window may wait for this bounds
+    // change to correctly animate in.
     controller_->ShowKeyboard(false);
   } else {
     // We need to send out this notification only if keyboard is visible since