From f4148f30946db3e5d316e3dd411eebca3bf5649c Mon Sep 17 00:00:00 2001 From: "nona@chromium.org" Date: Thu, 13 Sep 2012 18:10:08 +0000 Subject: Fix text input type inconsistency between renderer and TsfBridge It turned out that ITfThreadMgr::AssociateFocus is not necessary if we manually handle Win32 focus event in RWHVW.(Actually we already have monitored these Win32 focus events.) So we decided to refactor TsfBridge to simplify the focus management model. BUG=148918 TEST=Manually checked on Win8 metro. Review URL: https://chromiumcodereview.appspot.com/10928180 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156583 0039d316-1c4b-4281-b951-d872f2087c98 --- .../renderer_host/render_widget_host_view_win.cc | 18 +++----- ui/base/win/tsf_bridge.cc | 53 ++++++++-------------- ui/base/win/tsf_bridge.h | 14 ++---- 3 files changed, 28 insertions(+), 57 deletions(-) diff --git a/content/browser/renderer_host/render_widget_host_view_win.cc b/content/browser/renderer_host/render_widget_host_view_win.cc index 12a53b6..610f989 100644 --- a/content/browser/renderer_host/render_widget_host_view_win.cc +++ b/content/browser/renderer_host/render_widget_host_view_win.cc @@ -1252,8 +1252,6 @@ LRESULT RenderWidgetHostViewWin::OnCreate(CREATESTRUCT* create_struct) { else SetToGestureMode(); - if (base::win::IsTsfAwareRequired()) - ui::TsfBridge::GetInstance()->AssociateFocus(m_hWnd); UpdateIMEState(); return 0; @@ -3046,19 +3044,15 @@ LRESULT RenderWidgetHostViewWin::OnQueryCharPosition( } void RenderWidgetHostViewWin::UpdateIMEState() { + if (base::win::IsTsfAwareRequired()) { + ui::TsfBridge::GetInstance()->OnTextInputTypeChanged(this); + return; + } if (text_input_type_ != ui::TEXT_INPUT_TYPE_NONE && text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD) { - if (base::win::IsTsfAwareRequired()) { - ui::TsfBridge::GetInstance()->EnableIME(); - } else { - ime_input_.EnableIME(m_hWnd); - } + ime_input_.EnableIME(m_hWnd); } else { - if (base::win::IsTsfAwareRequired()) { - ui::TsfBridge::GetInstance()->DisableIME(); - } else { - ime_input_.DisableIME(m_hWnd); - } + ime_input_.DisableIME(m_hWnd); } } diff --git a/ui/base/win/tsf_bridge.cc b/ui/base/win/tsf_bridge.cc index 8e98b76..03cb6ca 100644 --- a/ui/base/win/tsf_bridge.cc +++ b/ui/base/win/tsf_bridge.cc @@ -11,6 +11,7 @@ #include "base/threading/thread_local_storage.h" #include "base/win/scoped_comptr.h" #include "base/win/scoped_variant.h" +#include "ui/base/ime/text_input_client.h" #include "ui/base/win/tsf_bridge.h" #include "ui/base/win/tsf_text_store.h" @@ -101,33 +102,21 @@ class TsfBridgeDelegate : public TsfBridge { } // TsfBridge override. - virtual bool EnableIME() OVERRIDE { + virtual void OnTextInputTypeChanged(TextInputClient* client) OVERRIDE { DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); DCHECK(IsInitialized()); - // Since EnableIME and DisableIME are designed to be a swap operation of - // |document_manager_| and |disabled_document_manager_|, do nothing unless - // the current focused document manager is |disabled_document_manager_|. - // In other words, ITfThreadMgr::SetFocus should be called if and only if - // TsfBridge actually has TSF input focus. Otherwise, TSF input focus will - // be inconsistent with Win32 input focus. - if (!IsFocused(disabled_document_manager_)) - return false; - - return SUCCEEDED(thread_manager_->SetFocus(document_manager_)); - } - - // TsfBridge override. - virtual bool DisableIME() OVERRIDE { - DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); - DCHECK(IsInitialized()); - - // Do nothing unless the current focused document manager is - // |document_manager_|. See the comment in EnableIME. - if (!IsFocused(document_manager_)) - return false; + if (client != client_) { + // Called from not focusing client. Do nothing. + return; + } - return SUCCEEDED(thread_manager_->SetFocus(disabled_document_manager_)); + DCHECK(client_); + const TextInputType type = client_->GetTextInputType(); + const bool is_ime_enabled = + type != TEXT_INPUT_TYPE_NONE && type != TEXT_INPUT_TYPE_PASSWORD; + thread_manager_->SetFocus( + is_ime_enabled ? document_manager_ : disabled_document_manager_); } // TsfBridge override. @@ -156,17 +145,6 @@ class TsfBridgeDelegate : public TsfBridge { } // TsfBridge override. - virtual bool AssociateFocus(HWND window) { - DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); - DCHECK(IsInitialized()); - base::win::ScopedComPtr previous_manager; - return SUCCEEDED(thread_manager_->AssociateFocus( - window, - document_manager_, - previous_manager.Receive())); - } - - // TsfBridge override. virtual void SetFocusedClient(HWND focused_window, TextInputClient* client) OVERRIDE { DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); @@ -174,14 +152,19 @@ class TsfBridgeDelegate : public TsfBridge { DCHECK(IsInitialized()); client_ = client; text_store_->get()->SetFocusedTextInputClient(focused_window, client); + + // Synchronize text input type state. + OnTextInputTypeChanged(client); } // TsfBridge override. virtual void RemoveFocusedClient(TextInputClient* client) OVERRIDE { DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); DCHECK(IsInitialized()); - if (client_ == client) + if (client_ == client) { + client_ = NULL; text_store_->get()->SetFocusedTextInputClient(NULL, NULL); + } } private: diff --git a/ui/base/win/tsf_bridge.h b/ui/base/win/tsf_bridge.h index 23ae9f3..4a45f03 100644 --- a/ui/base/win/tsf_bridge.h +++ b/ui/base/win/tsf_bridge.h @@ -38,21 +38,15 @@ class TsfBridge { // Destroys the thread local instance. virtual void Shutdown() = 0; - // Enables the IME. - // Returns false if an error occures. - virtual bool EnableIME() = 0; - - // Diables the IME. - // Returns false if an error occures. - virtual bool DisableIME() = 0; + // Handles TextInputTypeChanged event. RWHVW is responsible for calling this + // handler whenever renderer's input text type is changed. Does nothing + // unless |client| is focused. + virtual void OnTextInputTypeChanged(TextInputClient* client) = 0; // Cancels the ongoing composition if exists. // Returns false if an error occures. virtual bool CancelComposition() = 0; - // Associates |window| with document manager. - virtual bool AssociateFocus(HWND window) = 0; - // Sets currently focused TextInputClient. // Caller must free |client|. virtual void SetFocusedClient(HWND focused_window, -- cgit v1.1