From 183e28d67e650ad4c019daeacd3eaf4ccb23f103 Mon Sep 17 00:00:00 2001 From: "kevers@chromium.org" Date: Mon, 20 Jan 2014 18:18:02 +0000 Subject: [PATCH] Limit display of the virtual keyboard to state changes triggered from a user gesture. This patch reuses OnTextInputStateChanged, which was previously Android specific, but which includes not only type information, but whether the virtual keyboard should be displayed. This is the first step in consolidating the IPC messages between RenderWidget and RenderWidgetHostView for IME messages. Ideally, we can phase out OnTextInputTypeChanged in favor of the approach used by Android across all platforms. BUG=289659, 294191, 331690 Review URL: https://codereview.chromium.org/29943002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245932 0039d316-1c4b-4281-b951-d872f2087c98 --- ash/root_window_controller.cc | 3 + ash/root_window_controller_unittest.cc | 1 + .../extensions/virtual_keyboard_browsertest.cc | 13 +++ .../chromeos/input_method/textinput_test_helper.cc | 3 + .../chromeos/input_method/textinput_test_helper.h | 1 + chrome/browser/ui/omnibox/omnibox_view.cc | 3 + chrome/browser/ui/omnibox/omnibox_view.h | 3 + chrome/browser/ui/search/search_tab_helper.cc | 1 + .../browser/ui/views/omnibox/omnibox_view_views.cc | 4 + .../browser/ui/views/omnibox/omnibox_view_views.h | 1 + .../renderer_host/render_widget_host_view_aura.cc | 22 +++++ .../renderer_host/render_widget_host_view_aura.h | 3 + content/renderer/render_widget.cc | 15 ++- content/renderer/render_widget.h | 2 +- ui/base/ime/dummy_input_method.cc | 3 + ui/base/ime/dummy_input_method.h | 2 + ui/base/ime/input_method.h | 3 + ui/base/ime/input_method_base.cc | 4 + ui/base/ime/input_method_base.h | 1 + ui/base/ime/input_method_base_unittest.cc | 2 + ui/base/ime/input_method_observer.h | 4 + ui/base/ime/mock_input_method.cc | 4 + ui/base/ime/mock_input_method.h | 1 + ui/base/ime/remote_input_method_win.cc | 3 + ui/base/ime/remote_input_method_win_unittest.cc | 2 + ui/keyboard/keyboard_controller.cc | 105 +++++++++++---------- ui/keyboard/keyboard_controller.h | 6 ++ ui/keyboard/keyboard_controller_unittest.cc | 36 ++++--- ui/keyboard/keyboard_util.cc | 5 + ui/views/controls/textfield/textfield.cc | 10 +- ui/views/controls/textfield/textfield.h | 3 + ui/views/ime/input_method.h | 3 + ui/views/ime/input_method_bridge.cc | 6 ++ ui/views/ime/input_method_bridge.h | 1 + ui/views/ime/mock_input_method.cc | 3 + ui/views/ime/mock_input_method.h | 1 + 36 files changed, 216 insertions(+), 67 deletions(-) diff --git a/ash/root_window_controller.cc b/ash/root_window_controller.cc index 8a52e327f9a3..79c1a93604ae 100644 --- a/ash/root_window_controller.cc +++ b/ash/root_window_controller.cc @@ -617,6 +617,9 @@ void RootWindowController::DeactivateKeyboard( return; DCHECK(keyboard_controller); + if (!keyboard_controller->keyboard_container_initialized()) + return; + aura::Window* keyboard_container = keyboard_controller->GetContainerWindow(); if (keyboard_container->GetRootWindow() == root_window()) { diff --git a/ash/root_window_controller_unittest.cc b/ash/root_window_controller_unittest.cc index ff01846c1d65..f331afb6912f 100644 --- a/ash/root_window_controller_unittest.cc +++ b/ash/root_window_controller_unittest.cc @@ -621,6 +621,7 @@ TEST_F(VirtualKeyboardRootWindowControllerTest, aura::Window* keyboard_window = Shell::GetInstance()->keyboard_controller()-> proxy()->GetKeyboardWindow(); keyboard_container->AddChild(keyboard_window); + keyboard_window->set_owned_by_parent(false); keyboard_window->SetBounds(gfx::Rect()); keyboard_window->Show(); diff --git a/chrome/browser/chromeos/extensions/virtual_keyboard_browsertest.cc b/chrome/browser/chromeos/extensions/virtual_keyboard_browsertest.cc index 7a13b78059c5..a2da9e0e0328 100644 --- a/chrome/browser/chromeos/extensions/virtual_keyboard_browsertest.cc +++ b/chrome/browser/chromeos/extensions/virtual_keyboard_browsertest.cc @@ -6,6 +6,7 @@ #include +#include "ash/shell.h" #include "base/command_line.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -16,6 +17,9 @@ #include "content/public/browser/site_instance.h" #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" +#include "ui/aura/client/aura_constants.h" +#include "ui/base/ime/input_method.h" +#include "ui/keyboard/keyboard_controller.h" #include "ui/keyboard/keyboard_switches.h" namespace { @@ -67,7 +71,16 @@ class VirtualKeyboardBrowserTest : public InProcessBrowserTest { EXPECT_TRUE(ExecuteWebUIResourceTest(rvh, resource_ids)); } + void showVirtualKeyboard() { + aura::Window *window = ash::Shell::GetPrimaryRootWindow(); + ui::InputMethod* input_method = window->GetProperty( + aura::client::kRootWindowInputMethodKey); + ASSERT_TRUE(input_method); + input_method->ShowImeIfNeeded(); + } + content::RenderViewHost* GetKeyboardRenderViewHost() { + showVirtualKeyboard(); std::string kVirtualKeyboardURL = "chrome-extension://mppnpdlheglhdfmldimlhpnegondlapf/"; scoped_ptr widgets( diff --git a/chrome/browser/chromeos/input_method/textinput_test_helper.cc b/chrome/browser/chromeos/input_method/textinput_test_helper.cc index 44c33715edc6..d28fa675472e 100644 --- a/chrome/browser/chromeos/input_method/textinput_test_helper.cc +++ b/chrome/browser/chromeos/input_method/textinput_test_helper.cc @@ -78,6 +78,9 @@ void TextInputTestHelper::OnTextInputTypeChanged( base::MessageLoop::current()->Quit(); } +void TextInputTestHelper::OnShowImeIfNeeded() { +} + void TextInputTestHelper::OnInputMethodDestroyed( const ui::InputMethod* input_method) { } diff --git a/chrome/browser/chromeos/input_method/textinput_test_helper.h b/chrome/browser/chromeos/input_method/textinput_test_helper.h index 7013a0d4eba5..2b974251a543 100644 --- a/chrome/browser/chromeos/input_method/textinput_test_helper.h +++ b/chrome/browser/chromeos/input_method/textinput_test_helper.h @@ -78,6 +78,7 @@ class TextInputTestHelper : public ui::InputMethodObserver { virtual void OnCaretBoundsChanged(const ui::TextInputClient* client) OVERRIDE; virtual void OnTextInputStateChanged( const ui::TextInputClient* client) OVERRIDE; + virtual void OnShowImeIfNeeded() OVERRIDE; virtual void OnInputMethodDestroyed( const ui::InputMethod* input_method) OVERRIDE; diff --git a/chrome/browser/ui/omnibox/omnibox_view.cc b/chrome/browser/ui/omnibox/omnibox_view.cc index 92248349a526..ab3aed76e862 100644 --- a/chrome/browser/ui/omnibox/omnibox_view.cc +++ b/chrome/browser/ui/omnibox/omnibox_view.cc @@ -147,6 +147,9 @@ bool OmniboxView::IsImeShowingPopup() const { return false; } +void OmniboxView::ShowImeIfNeeded() { +} + bool OmniboxView::IsIndicatingQueryRefinement() const { // The default implementation always returns false. Mobile ports can override // this method and implement as needed. diff --git a/chrome/browser/ui/omnibox/omnibox_view.h b/chrome/browser/ui/omnibox/omnibox_view.h index 13729f879fe4..bebbd9b3bf88 100644 --- a/chrome/browser/ui/omnibox/omnibox_view.h +++ b/chrome/browser/ui/omnibox/omnibox_view.h @@ -209,6 +209,9 @@ class OmniboxView { // which may overlap the omnibox's popup window. virtual bool IsImeShowingPopup() const; + // Display a virtual keybaord or alternate input view if enabled. + virtual void ShowImeIfNeeded(); + // Returns true if the view is displaying UI that indicates that query // refinement will take place when the user selects the current match. For // search matches, this will cause the omnibox to search over the existing diff --git a/chrome/browser/ui/search/search_tab_helper.cc b/chrome/browser/ui/search/search_tab_helper.cc index cf249b7b7887..696eca073df1 100644 --- a/chrome/browser/ui/search/search_tab_helper.cc +++ b/chrome/browser/ui/search/search_tab_helper.cc @@ -445,6 +445,7 @@ void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) { // visual cue to users who really understand selection state about what // will happen if they start typing. omnibox->SelectAll(false); + omnibox->ShowImeIfNeeded(); break; case OMNIBOX_FOCUS_NONE: // Remove focus only if the popup is closed. This will prevent someone diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc index a044497e2a12..e660cf262a17 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc @@ -696,6 +696,10 @@ bool OmniboxViewViews::IsImeShowingPopup() const { #endif } +void OmniboxViewViews::ShowImeIfNeeded() { + GetInputMethod()->ShowImeIfNeeded(); +} + bool OmniboxViewViews::IsCommandIdEnabled(int command_id) const { if (command_id == IDS_APP_PASTE) return !read_only() && !GetClipboardText().empty(); diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.h b/chrome/browser/ui/views/omnibox/omnibox_view_views.h index 07154a3363cb..7d30f3cb8533 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.h +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.h @@ -110,6 +110,7 @@ class OmniboxViewViews virtual int GetWidth() const OVERRIDE; virtual bool IsImeComposing() const OVERRIDE; virtual bool IsImeShowingPopup() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE; virtual bool IsItemForCommandIdDynamic(int command_id) const OVERRIDE; virtual base::string16 GetLabelForCommandId(int command_id) const OVERRIDE; diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index 174b1e7a2a01..c479174c6105 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -497,6 +497,20 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host) //////////////////////////////////////////////////////////////////////////////// // RenderWidgetHostViewAura, RenderWidgetHostView implementation: +bool RenderWidgetHostViewAura::OnMessageReceived( + const IPC::Message& message) { + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(RenderWidgetHostViewAura, message) + // TODO(kevers): Move to RenderWidgetHostViewImpl and consolidate IPC + // messages for TextInputChanged. Corresponding code in + // RenderWidgetHostViewAndroid should also be moved at the same time. + IPC_MESSAGE_HANDLER(ViewHostMsg_TextInputStateChanged, + OnTextInputStateChanged) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + return handled; +} + void RenderWidgetHostViewAura::InitAsChild( gfx::NativeView parent_view) { window_->SetType(ui::wm::WINDOW_TYPE_CONTROL); @@ -913,6 +927,14 @@ void RenderWidgetHostViewAura::TextInputTypeChanged( } } +void RenderWidgetHostViewAura::OnTextInputStateChanged( + const ViewHostMsg_TextInputState_Params& params) { + if (params.show_ime_if_needed && params.type != ui::TEXT_INPUT_TYPE_NONE) { + if (GetInputMethod()) + GetInputMethod()->ShowImeIfNeeded(); + } +} + void RenderWidgetHostViewAura::ImeCancelComposition() { if (GetInputMethod()) GetInputMethod()->CancelComposition(this); diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h index 0cef4e5730a2..9004c93ad42b 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.h +++ b/content/browser/renderer_host/render_widget_host_view_aura.h @@ -155,6 +155,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAura } // RenderWidgetHostView implementation. + virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE; virtual void InitAsChild(gfx::NativeView parent_view) OVERRIDE; virtual RenderWidgetHost* GetRenderWidgetHost() const OVERRIDE; virtual void SetSize(const gfx::Size& size) OVERRIDE; @@ -349,6 +350,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura bool CanCopyToBitmap() const; + void OnTextInputStateChanged(const ViewHostMsg_TextInputState_Params& params); + #if defined(OS_WIN) // Sets the cutout rects from constrained windows. These are rectangles that // windowed NPAPI plugins shouldn't paint in. Overwrites any previous cutout diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index 7d8700f3dd79..7fee77936fe9 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc @@ -1198,6 +1198,12 @@ void RenderWidget::OnHandleInputEvent(const blink::WebInputEvent* input_event, // of a processed touch end event. if (input_event->type == WebInputEvent::TouchEnd && processed) UpdateTextInputState(true, true); +#elif defined(USE_AURA) + // Show the virtual keyboard if enabled and a user gesture triggers a focus + // change. + if (processed && (input_event->type == WebInputEvent::TouchEnd || + input_event->type == WebInputEvent::MouseUp)) + UpdateTextInputState(true, false); #endif TRACE_EVENT_SYNTHETIC_DELAY_END("blink.HandleInputEvent"); @@ -2503,7 +2509,7 @@ void RenderWidget::UpdateTextInputType() { } } -#if defined(OS_ANDROID) +#if defined(OS_ANDROID) || defined(USE_AURA) void RenderWidget::UpdateTextInputState(bool show_ime_if_needed, bool send_ime_ack) { if (handling_ime_event_) @@ -2526,6 +2532,9 @@ void RenderWidget::UpdateTextInputState(bool show_ime_if_needed, || text_input_info_ != new_info || can_compose_inline_ != new_can_compose_inline)) { ViewHostMsg_TextInputState_Params p; +#if defined(USE_AURA) + p.require_ack = false; +#endif p.type = new_type; p.value = new_info.value.utf8(); p.selection_start = new_info.selectionStart; @@ -2534,9 +2543,11 @@ void RenderWidget::UpdateTextInputState(bool show_ime_if_needed, p.composition_end = new_info.compositionEnd; p.can_compose_inline = new_can_compose_inline; p.show_ime_if_needed = show_ime_if_needed; +#if defined(OS_ANDROID) p.require_ack = send_ime_ack; if (p.require_ack) IncrementOutstandingImeEventAcks(); +#endif Send(new ViewHostMsg_TextInputStateChanged(routing_id(), p)); text_input_info_ = new_info; @@ -2712,7 +2723,7 @@ void RenderWidget::resetInputMethod() { void RenderWidget::didHandleGestureEvent( const WebGestureEvent& event, bool event_cancelled) { -#if defined(OS_ANDROID) +#if defined(OS_ANDROID) || defined(USE_AURA) if (event_cancelled) return; if (event.type == WebInputEvent::GestureTap || diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h index c2d743169e5f..aa379269cd2f 100644 --- a/content/renderer/render_widget.h +++ b/content/renderer/render_widget.h @@ -448,7 +448,7 @@ class CONTENT_EXPORT RenderWidget void set_next_paint_is_restore_ack(); void set_next_paint_is_repaint_ack(); -#if defined(OS_ANDROID) +#if defined(OS_ANDROID) || defined(USE_AURA) // |show_ime_if_needed| should be true iff the update may cause the ime to be // displayed, e.g. after a tap on an input field on mobile. // |send_ime_ack| should be true iff the browser side is required to diff --git a/ui/base/ime/dummy_input_method.cc b/ui/base/ime/dummy_input_method.cc index 0700d7246ca9..a12bbfc51bf8 100644 --- a/ui/base/ime/dummy_input_method.cc +++ b/ui/base/ime/dummy_input_method.cc @@ -79,6 +79,9 @@ bool DummyInputMethod::IsCandidatePopupOpen() const { return false; } +void DummyInputMethod::ShowImeIfNeeded() { +} + void DummyInputMethod::AddObserver(InputMethodObserver* observer) { } diff --git a/ui/base/ime/dummy_input_method.h b/ui/base/ime/dummy_input_method.h index fe0fd3f450ed..39b04195f9d1 100644 --- a/ui/base/ime/dummy_input_method.h +++ b/ui/base/ime/dummy_input_method.h @@ -38,6 +38,8 @@ class DummyInputMethod : public InputMethod { virtual TextInputMode GetTextInputMode() const OVERRIDE; virtual bool CanComposeInline() const OVERRIDE; virtual bool IsCandidatePopupOpen() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; + virtual void AddObserver(InputMethodObserver* observer) OVERRIDE; virtual void RemoveObserver(InputMethodObserver* observer) OVERRIDE; diff --git a/ui/base/ime/input_method.h b/ui/base/ime/input_method.h index b6b30e2aacaf..8571a050218b 100644 --- a/ui/base/ime/input_method.h +++ b/ui/base/ime/input_method.h @@ -154,6 +154,9 @@ class InputMethod { // of IME popups is not supported. virtual bool IsCandidatePopupOpen() const = 0; + // Displays an on screen keyboard if enabled. + virtual void ShowImeIfNeeded() = 0; + // Management of the observer list. virtual void AddObserver(InputMethodObserver* observer) = 0; virtual void RemoveObserver(InputMethodObserver* observer) = 0; diff --git a/ui/base/ime/input_method_base.cc b/ui/base/ime/input_method_base.cc index d88fccfc6ff3..edb19df4f02d 100644 --- a/ui/base/ime/input_method_base.cc +++ b/ui/base/ime/input_method_base.cc @@ -80,6 +80,10 @@ bool InputMethodBase::CanComposeInline() const { return client ? client->CanComposeInline() : true; } +void InputMethodBase::ShowImeIfNeeded() { + FOR_EACH_OBSERVER(InputMethodObserver, observer_list_, OnShowImeIfNeeded()); +} + void InputMethodBase::AddObserver(InputMethodObserver* observer) { observer_list_.AddObserver(observer); } diff --git a/ui/base/ime/input_method_base.h b/ui/base/ime/input_method_base.h index 6b3c8ef52392..ce3801537d26 100644 --- a/ui/base/ime/input_method_base.h +++ b/ui/base/ime/input_method_base.h @@ -50,6 +50,7 @@ class UI_BASE_EXPORT InputMethodBase virtual TextInputType GetTextInputType() const OVERRIDE; virtual TextInputMode GetTextInputMode() const OVERRIDE; virtual bool CanComposeInline() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; virtual void AddObserver(InputMethodObserver* observer) OVERRIDE; virtual void RemoveObserver(InputMethodObserver* observer) OVERRIDE; diff --git a/ui/base/ime/input_method_base_unittest.cc b/ui/base/ime/input_method_base_unittest.cc index 9ecd4edc28b9..a8489e7bfea5 100644 --- a/ui/base/ime/input_method_base_unittest.cc +++ b/ui/base/ime/input_method_base_unittest.cc @@ -205,6 +205,8 @@ class MockInputMethodObserver : public InputMethodObserver { virtual void OnTextInputStateChanged(const TextInputClient* client) OVERRIDE { verifier_->OnTextInputStateChanged(client); } + virtual void OnShowImeIfNeeded() OVERRIDE { + } virtual void OnInputMethodDestroyed(const InputMethod* client) OVERRIDE { } diff --git a/ui/base/ime/input_method_observer.h b/ui/base/ime/input_method_observer.h index 8e4355d35575..d3432c9782dd 100644 --- a/ui/base/ime/input_method_observer.h +++ b/ui/base/ime/input_method_observer.h @@ -39,6 +39,10 @@ class UI_BASE_EXPORT InputMethodObserver { // Called when the observed InputMethod is being destroyed. virtual void OnInputMethodDestroyed(const InputMethod* input_method) = 0; + + // Called when a user gesture should trigger showing the virtual keyboard + // or alternate input view (e.g. handwriting palette). Used in ChromeOS. + virtual void OnShowImeIfNeeded() = 0; }; } // namespace ui diff --git a/ui/base/ime/mock_input_method.cc b/ui/base/ime/mock_input_method.cc index 5ae577a5f911..0aa2005cb1d8 100644 --- a/ui/base/ime/mock_input_method.cc +++ b/ui/base/ime/mock_input_method.cc @@ -101,6 +101,10 @@ bool MockInputMethod::IsCandidatePopupOpen() const { return false; } +void MockInputMethod::ShowImeIfNeeded() { + FOR_EACH_OBSERVER(InputMethodObserver, observer_list_, OnShowImeIfNeeded()); +} + void MockInputMethod::AddObserver(InputMethodObserver* observer) { observer_list_.AddObserver(observer); } diff --git a/ui/base/ime/mock_input_method.h b/ui/base/ime/mock_input_method.h index 495fca405d39..7716065ce985 100644 --- a/ui/base/ime/mock_input_method.h +++ b/ui/base/ime/mock_input_method.h @@ -49,6 +49,7 @@ class UI_BASE_EXPORT MockInputMethod : NON_EXPORTED_BASE(public InputMethod) { virtual TextInputMode GetTextInputMode() const OVERRIDE; virtual bool CanComposeInline() const OVERRIDE; virtual bool IsCandidatePopupOpen() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; virtual void AddObserver(InputMethodObserver* observer) OVERRIDE; virtual void RemoveObserver(InputMethodObserver* observer) OVERRIDE; diff --git a/ui/base/ime/remote_input_method_win.cc b/ui/base/ime/remote_input_method_win.cc index b03a57404c0d..c8d66500a549 100644 --- a/ui/base/ime/remote_input_method_win.cc +++ b/ui/base/ime/remote_input_method_win.cc @@ -277,6 +277,9 @@ class RemoteInputMethodWin : public InputMethod, return is_candidate_popup_open_; } + virtual void ShowImeIfNeeded() OVERRIDE { + } + virtual void AddObserver(InputMethodObserver* observer) OVERRIDE { observer_list_.AddObserver(observer); } diff --git a/ui/base/ime/remote_input_method_win_unittest.cc b/ui/base/ime/remote_input_method_win_unittest.cc index f3100d0fb2af..568a8061297e 100644 --- a/ui/base/ime/remote_input_method_win_unittest.cc +++ b/ui/base/ime/remote_input_method_win_unittest.cc @@ -257,6 +257,8 @@ class MockInputMethodObserver : public InputMethodObserver { virtual void OnInputMethodDestroyed(const InputMethod* client) OVERRIDE { ++on_input_method_destroyed_changed_; } + virtual void OnShowImeIfNeeded() { + } size_t on_text_input_state_changed_; size_t on_input_method_destroyed_changed_; diff --git a/ui/keyboard/keyboard_controller.cc b/ui/keyboard/keyboard_controller.cc index 15d520880129..7326875205f9 100644 --- a/ui/keyboard/keyboard_controller.cc +++ b/ui/keyboard/keyboard_controller.cc @@ -178,13 +178,8 @@ KeyboardController::KeyboardController(KeyboardControllerProxy* proxy) } KeyboardController::~KeyboardController() { - if (container_) { + if (container_) container_->RemoveObserver(this); - // Remove the keyboard window from the children because the keyboard window - // is owned by proxy and it should be destroyed by proxy. - if (container_->Contains(proxy_->GetKeyboardWindow())) - container_->RemoveChild(proxy_->GetKeyboardWindow()); - } if (input_method_) input_method_->RemoveObserver(this); } @@ -249,50 +244,15 @@ void KeyboardController::OnTextInputStateChanged( if (!container_.get()) return; - bool was_showing = keyboard_visible_; - bool should_show = was_showing; - ui::TextInputType type = - client ? client->GetTextInputType() : ui::TEXT_INPUT_TYPE_NONE; - if (type == ui::TEXT_INPUT_TYPE_NONE && - !IsKeyboardUsabilityExperimentEnabled() && - !lock_keyboard_) { - should_show = false; - } else { - if (container_->children().empty()) { - keyboard::MarkKeyboardLoadStarted(); - aura::Window* keyboard = proxy_->GetKeyboardWindow(); - keyboard->Show(); - container_->AddChild(keyboard); - } - if (type != ui::TEXT_INPUT_TYPE_NONE) - proxy_->SetUpdateInputType(type); - container_->parent()->StackChildAtTop(container_.get()); - should_show = true; + if (IsKeyboardUsabilityExperimentEnabled()) { + OnShowImeIfNeeded(); + return; } - if (was_showing != should_show) { - if (should_show) { - keyboard_visible_ = true; - - // If the controller is in the process of hiding the keyboard, do not log - // the stat here since the keyboard will not actually be shown. - if (!WillHideKeyboard()) - keyboard::LogKeyboardControlEvent(keyboard::KEYBOARD_CONTROL_SHOW); - - weak_factory_.InvalidateWeakPtrs(); - // If |container_| has hide animation, its visibility is set to false when - // hide animation finished. So even if the container is visible at this - // point, it may in the process of hiding. We still need to show keyboard - // container in this case. - if (container_->IsVisible() && - !container_->layer()->GetAnimator()->is_animating()) { - return; - } - - NotifyKeyboardBoundsChanging(container_->children()[0]->bounds()); - - proxy_->ShowKeyboardContainer(container_.get()); - } else { + ui::TextInputType type = + client ? client->GetTextInputType() : ui::TEXT_INPUT_TYPE_NONE; + if (type == ui::TEXT_INPUT_TYPE_NONE && !lock_keyboard_) { + if (keyboard_visible_) { // Set the visibility state here so that any queries for visibility // before the timer fires returns the correct future value. keyboard_visible_ = false; @@ -302,6 +262,18 @@ void KeyboardController::OnTextInputStateChanged( weak_factory_.GetWeakPtr(), HIDE_REASON_AUTOMATIC), base::TimeDelta::FromMilliseconds(kHideKeyboardDelayMs)); } + } else { + // Abort a pending keyboard hide. + if (WillHideKeyboard()) { + weak_factory_.InvalidateWeakPtrs(); + keyboard_visible_ = true; + } + proxy_->SetUpdateInputType(type); + // Do not explicitly show the Virtual keyboard unless it is in the process + // of hiding. Instead, the virtual keyboard is shown in response to a user + // gesture (mouse or touch) that is received while an element has input + // focus. Showing the keyboard requires an explicit call to + // OnShowImeIfNeeded. } // 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. @@ -314,6 +286,43 @@ void KeyboardController::OnInputMethodDestroyed( input_method_ = NULL; } +void KeyboardController::OnShowImeIfNeeded() { + if (!container_.get()) + return; + + if (container_->children().empty()) { + keyboard::MarkKeyboardLoadStarted(); + aura::Window* keyboard = proxy_->GetKeyboardWindow(); + keyboard->Show(); + container_->AddChild(keyboard); + keyboard->set_owned_by_parent(false); + } + if (keyboard_visible_) + return; + + keyboard_visible_ = true; + + // If the controller is in the process of hiding the keyboard, do not log + // the stat here since the keyboard will not actually be shown. + if (!WillHideKeyboard()) + keyboard::LogKeyboardControlEvent(keyboard::KEYBOARD_CONTROL_SHOW); + + weak_factory_.InvalidateWeakPtrs(); + + // If |container_| has hide animation, its visibility is set to false when + // hide animation finished. So even if the container is visible at this + // point, it may in the process of hiding. We still need to show keyboard + // container in this case. + if (container_->IsVisible() && + !container_->layer()->GetAnimator()->is_animating()) { + return; + } + + NotifyKeyboardBoundsChanging(container_->children()[0]->bounds()); + + proxy_->ShowKeyboardContainer(container_.get()); +} + bool KeyboardController::WillHideKeyboard() const { return weak_factory_.HasWeakPtrs(); } diff --git a/ui/keyboard/keyboard_controller.h b/ui/keyboard/keyboard_controller.h index e5cfbe5d8475..a513659044e1 100644 --- a/ui/keyboard/keyboard_controller.h +++ b/ui/keyboard/keyboard_controller.h @@ -52,6 +52,11 @@ class KEYBOARD_EXPORT KeyboardController : public ui::InputMethodObserver, // KeyboardController. aura::Window* GetContainerWindow(); + // Whether the container window for the keyboard has been initialized. + bool keyboard_container_initialized() const { + return container_.get() != NULL; + } + // Sets the override content url. This is used by for input view for extension // IMEs. void SetOverrideContentUrl(const GURL& url); @@ -92,6 +97,7 @@ class KEYBOARD_EXPORT KeyboardController : public ui::InputMethodObserver, const ui::TextInputClient* client) OVERRIDE; virtual void OnInputMethodDestroyed( const ui::InputMethod* input_method) OVERRIDE; + virtual void OnShowImeIfNeeded() OVERRIDE; // Returns true if keyboard is scheduled to hide. bool WillHideKeyboard() const; diff --git a/ui/keyboard/keyboard_controller_unittest.cc b/ui/keyboard/keyboard_controller_unittest.cc index 88a364780795..98e08957b22d 100644 --- a/ui/keyboard/keyboard_controller_unittest.cc +++ b/ui/keyboard/keyboard_controller_unittest.cc @@ -218,10 +218,17 @@ class KeyboardControllerTest : public testing::Test { void ShowKeyboard() { TestTextInputClient test_text_input_client(ui::TEXT_INPUT_TYPE_TEXT); - controller_->OnTextInputStateChanged(&test_text_input_client); + SetFocus(&test_text_input_client); } protected: + void SetFocus(ui::TextInputClient* client) { + ui::InputMethod* input_method = proxy()->GetInputMethod(); + input_method->SetFocusedTextInputClient(client); + if (client && client->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE) + input_method->ShowImeIfNeeded(); + } + bool WillHideKeyboard() { return controller_->WillHideKeyboard(); } @@ -339,13 +346,11 @@ TEST_F(KeyboardControllerTest, EventHitTestingInContainer) { TEST_F(KeyboardControllerTest, VisibilityChangeWithTextInputTypeChange) { const gfx::Rect& root_bounds = root_window()->bounds(); - ui::InputMethod* input_method = proxy()->GetInputMethod(); TestTextInputClient input_client_0(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient input_client_1(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient input_client_2(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient no_input_client_0(ui::TEXT_INPUT_TYPE_NONE); TestTextInputClient no_input_client_1(ui::TEXT_INPUT_TYPE_NONE); - input_method->SetFocusedTextInputClient(&input_client_0); aura::Window* keyboard_container(controller()->GetContainerWindow()); scoped_ptr keyboard_container_observer( @@ -353,9 +358,11 @@ TEST_F(KeyboardControllerTest, VisibilityChangeWithTextInputTypeChange) { keyboard_container->SetBounds(root_bounds); root_window()->AddChild(keyboard_container); + SetFocus(&input_client_0); + EXPECT_TRUE(keyboard_container->IsVisible()); - input_method->SetFocusedTextInputClient(&no_input_client_0); + SetFocus(&no_input_client_0); // Keyboard should not immediately hide itself. It is delayed to avoid layout // flicker when the focus of input field quickly change. EXPECT_TRUE(keyboard_container->IsVisible()); @@ -364,14 +371,14 @@ TEST_F(KeyboardControllerTest, VisibilityChangeWithTextInputTypeChange) { base::MessageLoop::current()->Run(); EXPECT_FALSE(keyboard_container->IsVisible()); - input_method->SetFocusedTextInputClient(&input_client_1); + SetFocus(&input_client_1); EXPECT_TRUE(keyboard_container->IsVisible()); // Schedule to hide keyboard. - input_method->SetFocusedTextInputClient(&no_input_client_1); + SetFocus(&no_input_client_1); EXPECT_TRUE(WillHideKeyboard()); // Cancel keyboard hide. - input_method->SetFocusedTextInputClient(&input_client_2); + SetFocus(&input_client_2); EXPECT_FALSE(WillHideKeyboard()); EXPECT_TRUE(keyboard_container->IsVisible()); @@ -380,12 +387,10 @@ TEST_F(KeyboardControllerTest, VisibilityChangeWithTextInputTypeChange) { TEST_F(KeyboardControllerTest, AlwaysVisibleWhenLocked) { const gfx::Rect& root_bounds = root_window()->bounds(); - ui::InputMethod* input_method = proxy()->GetInputMethod(); TestTextInputClient input_client_0(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient input_client_1(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient no_input_client_0(ui::TEXT_INPUT_TYPE_NONE); TestTextInputClient no_input_client_1(ui::TEXT_INPUT_TYPE_NONE); - input_method->SetFocusedTextInputClient(&input_client_0); aura::Window* keyboard_container(controller()->GetContainerWindow()); scoped_ptr keyboard_container_observer( @@ -393,24 +398,26 @@ TEST_F(KeyboardControllerTest, AlwaysVisibleWhenLocked) { keyboard_container->SetBounds(root_bounds); root_window()->AddChild(keyboard_container); + SetFocus(&input_client_0); + EXPECT_TRUE(keyboard_container->IsVisible()); // Lock keyboard. controller()->set_lock_keyboard(true); - input_method->SetFocusedTextInputClient(&no_input_client_0); + SetFocus(&no_input_client_0); // Keyboard should not try to hide itself as it is locked. EXPECT_TRUE(keyboard_container->IsVisible()); EXPECT_FALSE(WillHideKeyboard()); - input_method->SetFocusedTextInputClient(&input_client_1); + SetFocus(&input_client_1); EXPECT_TRUE(keyboard_container->IsVisible()); // Unlock keyboard. controller()->set_lock_keyboard(false); // Keyboard should hide when focus on no input client. - input_method->SetFocusedTextInputClient(&no_input_client_1); + SetFocus(&no_input_client_1); EXPECT_TRUE(WillHideKeyboard()); // Wait for hide keyboard to finish. @@ -464,18 +471,17 @@ class KeyboardControllerUsabilityTest : public KeyboardControllerTest { TEST_F(KeyboardControllerUsabilityTest, KeyboardAlwaysVisibleInUsabilityTest) { const gfx::Rect& root_bounds = root_window()->bounds(); - ui::InputMethod* input_method = proxy()->GetInputMethod(); TestTextInputClient input_client(ui::TEXT_INPUT_TYPE_TEXT); TestTextInputClient no_input_client(ui::TEXT_INPUT_TYPE_NONE); - input_method->SetFocusedTextInputClient(&input_client); aura::Window* keyboard_container(controller()->GetContainerWindow()); keyboard_container->SetBounds(root_bounds); root_window()->AddChild(keyboard_container); + SetFocus(&input_client); EXPECT_TRUE(keyboard_container->IsVisible()); - input_method->SetFocusedTextInputClient(&no_input_client); + SetFocus(&no_input_client); // Keyboard should not hide itself after lost focus. EXPECT_TRUE(keyboard_container->IsVisible()); EXPECT_FALSE(WillHideKeyboard()); diff --git a/ui/keyboard/keyboard_util.cc b/ui/keyboard/keyboard_util.cc index d8c7b7add01e..c3b66b51aced 100644 --- a/ui/keyboard/keyboard_util.cc +++ b/ui/keyboard/keyboard_util.cc @@ -166,6 +166,11 @@ const void MarkKeyboardLoadStarted() { } const void MarkKeyboardLoadFinished() { + // Possible to get a load finished without a start if navigating directly to + // chrome://keyboard. + if (!g_keyboard_load_time_start.Get().ToInternalValue()) + return; + // It should not be possible to finish loading the keyboard without starting // to load it first. DCHECK(g_keyboard_load_time_start.Get().ToInternalValue()); diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc index 5f3d119a53a8..88d4c1e334c9 100644 --- a/ui/views/controls/textfield/textfield.cc +++ b/ui/views/controls/textfield/textfield.cc @@ -230,6 +230,10 @@ base::string16 Textfield::GetPlaceholderText() const { return placeholder_text_; } +void Textfield::ShowImeIfNeeded() { + GetInputMethod()->ShowImeIfNeeded(); +} + bool Textfield::IsIMEComposing() const { return model_->HasCompositionText(); } @@ -463,8 +467,10 @@ bool Textfield::OnMousePressed(const ui::MouseEvent& event) { TrackMouseClicks(event); if (!controller_ || !controller_->HandleMouseEvent(this, event)) { - if (event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) + if (event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) { RequestFocus(); + ShowImeIfNeeded(); + } if (event.IsOnlyLeftMouseButton()) { initiating_drag_ = false; @@ -634,6 +640,8 @@ void Textfield::OnGestureEvent(ui::GestureEvent* event) { case ui::ET_GESTURE_TAP_DOWN: OnBeforeUserAction(); RequestFocus(); + ShowImeIfNeeded(); + // We don't deselect if the point is in the selection // because TAP_DOWN may turn into a LONG_PRESS. if (!GetRenderText()->IsPointInSelection(event->location()) && diff --git a/ui/views/controls/textfield/textfield.h b/ui/views/controls/textfield/textfield.h index 782a0b48b548..94d98aef09a6 100644 --- a/ui/views/controls/textfield/textfield.h +++ b/ui/views/controls/textfield/textfield.h @@ -131,6 +131,9 @@ class VIEWS_EXPORT Textfield : public View, placeholder_text_color_ = color; } + // Displays a virtual keyboard or alternate input view if enabled. + void ShowImeIfNeeded(); + // Returns whether or not an IME is composing text. bool IsIMEComposing() const; diff --git a/ui/views/ime/input_method.h b/ui/views/ime/input_method.h index a8f3aaadd8a4..04ad430079b3 100644 --- a/ui/views/ime/input_method.h +++ b/ui/views/ime/input_method.h @@ -114,6 +114,9 @@ class VIEWS_EXPORT InputMethod { // of IME popups is not supported. virtual bool IsCandidatePopupOpen() const = 0; + // Displays an on screen keyboard if enabled. + virtual void ShowImeIfNeeded() = 0; + // Returns true if the input method is a mock instance used for testing. virtual bool IsMock() const = 0; diff --git a/ui/views/ime/input_method_bridge.cc b/ui/views/ime/input_method_bridge.cc index b6106d8962c4..5fc7fde3d77e 100644 --- a/ui/views/ime/input_method_bridge.cc +++ b/ui/views/ime/input_method_bridge.cc @@ -33,6 +33,7 @@ class InputMethodBridge::HostObserver : public ui::InputMethodObserver { const ui::TextInputClient* client) OVERRIDE {} virtual void OnInputMethodDestroyed( const ui::InputMethod* input_method) OVERRIDE; + virtual void OnShowImeIfNeeded() OVERRIDE {} private: InputMethodBridge* bridge_; @@ -171,6 +172,11 @@ bool InputMethodBridge::IsCandidatePopupOpen() const { return host_->IsCandidatePopupOpen(); } +void InputMethodBridge::ShowImeIfNeeded() { + DCHECK(host_); + host_->ShowImeIfNeeded(); +} + // Overridden from TextInputClient. Forward an event from the system-wide IME // to the text input |client|, which is e.g. views::Textfield. void InputMethodBridge::SetCompositionText( diff --git a/ui/views/ime/input_method_bridge.h b/ui/views/ime/input_method_bridge.h index 5c0e697a54e3..dbc9765bcfeb 100644 --- a/ui/views/ime/input_method_bridge.h +++ b/ui/views/ime/input_method_bridge.h @@ -47,6 +47,7 @@ class InputMethodBridge : public InputMethodBase, virtual std::string GetInputLocale() OVERRIDE; virtual bool IsActive() OVERRIDE; virtual bool IsCandidatePopupOpen() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; // Overridden from TextInputClient: virtual void SetCompositionText( diff --git a/ui/views/ime/mock_input_method.cc b/ui/views/ime/mock_input_method.cc index 9188b74ca749..ccc054db498e 100644 --- a/ui/views/ime/mock_input_method.cc +++ b/ui/views/ime/mock_input_method.cc @@ -130,6 +130,9 @@ bool MockInputMethod::IsCandidatePopupOpen() const { return false; } +void MockInputMethod::ShowImeIfNeeded() { +} + bool MockInputMethod::IsMock() const { return true; } diff --git a/ui/views/ime/mock_input_method.h b/ui/views/ime/mock_input_method.h index d22022caf120..013742cff1c2 100644 --- a/ui/views/ime/mock_input_method.h +++ b/ui/views/ime/mock_input_method.h @@ -36,6 +36,7 @@ class VIEWS_EXPORT MockInputMethod : public InputMethodBase { virtual std::string GetInputLocale() OVERRIDE; virtual bool IsActive() OVERRIDE; virtual bool IsCandidatePopupOpen() const OVERRIDE; + virtual void ShowImeIfNeeded() OVERRIDE; virtual bool IsMock() const OVERRIDE; bool focus_changed() const { return focus_changed_; } -- 2.11.4.GIT