Refactoring for the InputMethod & InputMethodDelegate interfaces.

ui/base/ime/input_method_auralinux.cc

 // Copyright 2013 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 "ui/base/ime/input_method_auralinux.h"
 
 #include "base/auto_reset.h"
 #include "base/environment.h"
 #include "ui/base/ime/linux/linux_input_method_context_factory.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/events/event.h"
 
 namespace ui {
 
 InputMethodAuraLinux::InputMethodAuraLinux(
     internal::InputMethodDelegate* delegate)
     : text_input_type_(TEXT_INPUT_TYPE_NONE),
       is_sync_mode_(false),
       composition_changed_(false),
-      suppress_next_result_(false),
-      destroyed_ptr_(nullptr) {
+      suppress_next_result_(false) {
   SetDelegate(delegate);
   context_ =
       LinuxInputMethodContextFactory::instance()->CreateInputMethodContext(
           this, false);
   context_simple_ =
       LinuxInputMethodContextFactory::instance()->CreateInputMethodContext(
           this, true);
 }
 
 InputMethodAuraLinux::~InputMethodAuraLinux() {
-  if (destroyed_ptr_)
-    *destroyed_ptr_ = true;
 }
 
 LinuxInputMethodContext* InputMethodAuraLinux::GetContextForTesting(
     bool is_simple) {
   return is_simple ? context_simple_.get() : context_.get();
 }
 
 // Overriden from InputMethod.
 
 bool InputMethodAuraLinux::OnUntranslatedIMEMessage(
     const base::NativeEvent& event,
     NativeEventResult* result) {
   return false;
 }
 
-bool InputMethodAuraLinux::DispatchKeyEvent(const ui::KeyEvent& event) {
-  DCHECK(event.type() == ET_KEY_PRESSED || event.type() == ET_KEY_RELEASED);
+void InputMethodAuraLinux::DispatchKeyEvent(ui::KeyEvent* event) {
+  DCHECK(event->type() == ET_KEY_PRESSED || event->type() == ET_KEY_RELEASED);
   DCHECK(system_toplevel_window_focused());
 
   // If no text input client, do nothing.
-  if (!GetTextInputClient())
-    return DispatchKeyEventPostIME(event);
+  if (!GetTextInputClient()) {
+    ignore_result(DispatchKeyEventPostIME(event));
+    return;
+  }
 
   suppress_next_result_ = false;
   composition_changed_ = false;
   result_text_.clear();
 
   bool filtered = false;
   {
     base::AutoReset<bool> flipper(&is_sync_mode_, true);
     if (text_input_type_ != TEXT_INPUT_TYPE_NONE &&
         text_input_type_ != TEXT_INPUT_TYPE_PASSWORD) {
-      filtered = context_->DispatchKeyEvent(event);
+      filtered = context_->DispatchKeyEvent(*event);
     } else {
-      filtered = context_simple_->DispatchKeyEvent(event);
+      filtered = context_simple_->DispatchKeyEvent(*event);
     }
   }
 
-  bool destroyed = false;
-  bool handled = false;
-  if (event.type() == ui::ET_KEY_PRESSED && filtered) {
-    {
-      base::AutoReset<bool*> auto_reset(&destroyed_ptr_, &destroyed);
-      if (NeedInsertChar())
-        handled = DispatchKeyEventPostIME(event);
-      else if (HasInputMethodResult())
-        handled = SendFakeProcessKeyEvent(event.flags());
-      if (destroyed)
-        return true;
-    }
+  ui::EventDispatchDetails details;
+  if (event->type() == ui::ET_KEY_PRESSED && filtered) {
+    if (NeedInsertChar())
+      details = DispatchKeyEventPostIME(event);
+    else if (HasInputMethodResult())
+      details = SendFakeProcessKeyEvent(event);
+    if (details.dispatcher_destroyed)
+      return;
     // If the KEYDOWN is stopped propagation (e.g. triggered an accelerator),
     // don't InsertChar/InsertText to the input field.
-    if (handled) {
+    if (event->stopped_propagation() || details.target_destroyed) {
       ResetContext();
-      return true;
+      return;
     }
 
     // Don't send VKEY_PROCESSKEY event if there is no result text or
     // composition. This is to workaround the weird behavior of IBus with US
     // keyboard, which mutes the keydown and later fake a new keydown with IME
     // result in sync mode. In that case, user would expect only
     // keydown/keypress/keyup event without an initial 229 keydown event.
   }
 
+  bool should_stop_propagation = false;
   TextInputClient* client = GetTextInputClient();
   // Processes the result text before composition for sync mode.
   if (!result_text_.empty()) {
     if (filtered && NeedInsertChar()) {
       for (const auto ch : result_text_)
-        client->InsertChar(ch, event.flags());
+        client->InsertChar(ch, event->flags());
     } else {
       // If |filtered| is false, that means the IME wants to commit some text
       // but still release the key to the application. For example, Korean IME
       // handles ENTER key to confirm its composition but still release it for
       // the default behavior (e.g. trigger search, etc.)
       // In such case, don't do InsertChar because a key should only trigger the
       // keydown event once.
       client->InsertText(result_text_);
     }
+    should_stop_propagation = true;
   }
 
   if (composition_changed_ && !IsTextInputTypeNone()) {
     // If composition changed, does SetComposition if composition is not empty.
     // And ClearComposition if composition is empty.
     if (!composition_.text.empty())
       client->SetCompositionText(composition_);
     else if (result_text_.empty())
       client->ClearCompositionText();
+    should_stop_propagation = true;
   }
 
   // Makes sure the cached composition is cleared after committing any text or
   // cleared composition.
   if (!client->HasCompositionText())
     composition_.Clear();
 
   if (!filtered) {
-    {
-      base::AutoReset<bool*> auto_reset(&destroyed_ptr_, &destroyed);
-      handled = DispatchKeyEventPostIME(event);
-      if (destroyed)
-        return true;
+    details = DispatchKeyEventPostIME(event);
+    if (details.dispatcher_destroyed)
+      goto Exit;
+    if (event->stopped_propagation() || details.target_destroyed) {
+      ResetContext();
+      goto Exit;

[sadrul, 2015/08/04 03:52:46]

I don't think this needs to use goto. Can we look at should_stop_propagation and
set StopPropagation() right after DispatchKeyEventPostIME()?

[Shu Chen, 2015/08/04 03:59:20]

Do you mean like below? Please see my inline comment with "BUG".
  details = DispatchKeyEventPostIME(event);
  if (should_stop_propagation)
    event->StopPropagation();
  ...
  // BUG: stopped_propagation() flag maybe set by should_stop_propagation
instead of DispatchKeyEventPostIME().
  if (event->stopped_propagation() || details.target_destroyed) {
  ...

[James Su, 2015/08/04 05:02:48]

How about:

if (details.dispatcher_destroyed) {
  // Do nothing
} else if (event->stopped_propagation() || details.target_destroyed) {
  ResetContext();
} else if (event->type() == ui::ET_KEY_PRESSED) {
  ...
}

On 2015/08/04 03:59:20, Shu Chen wrote:

[Shu Chen, 2015/08/04 05:09:31]

Done & thanks for your suggestion.

     }
-    if (handled) {
-      ResetContext();
-      return true;
-    }
-    if (event.type() == ui::ET_KEY_PRESSED) {
+    if (event->type() == ui::ET_KEY_PRESSED) {
       // If a key event was not filtered by |context_| or |context_simple_|,
       // then it means the key event didn't generate any result text. For some
       // cases, the key event may still generate a valid character, eg. a
       // control-key event (ctrl-a, return, tab, etc.). We need to send the
       // character to the focused text input client by calling
       // TextInputClient::InsertChar().
       // Note: don't use |client| and use GetTextInputClient() here because
       // DispatchKeyEventPostIME may cause the current text input client change.
-      base::char16 ch = event.GetCharacter();
+      base::char16 ch = event->GetCharacter();
       if (ch && GetTextInputClient())
-        GetTextInputClient()->InsertChar(ch, event.flags());
+        GetTextInputClient()->InsertChar(ch, event->flags());
+      should_stop_propagation = true;
     }
   }
 
-  return true;
+Exit:
+  if (should_stop_propagation)
+    event->StopPropagation();
 }
 
 void InputMethodAuraLinux::UpdateContextFocusState() {
   bool old_text_input_type = text_input_type_;
   text_input_type_ = GetTextInputType();
 
   // We only focus in |context_| when the focus is in a textfield.
   if (old_text_input_type != TEXT_INPUT_TYPE_NONE &&
       text_input_type_ == TEXT_INPUT_TYPE_NONE) {
     context_->Blur();
@@ skipping 75 matching lines @@
     return;
   }
 
   if (is_sync_mode_) {
     // Append the text to the buffer, because commit signal might be fired
     // multiple times when processing a key event.
     result_text_.append(text);
   } else if (!IsTextInputTypeNone()) {
     // If we are not handling key event, do not bother sending text result if
     // the focused text input client does not support text input.
-    SendFakeProcessKeyEvent(0);
-    GetTextInputClient()->InsertText(text);
+    ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, 0);
+    ui::EventDispatchDetails details = SendFakeProcessKeyEvent(&event);
+    if (details.dispatcher_destroyed)
+      return;
+    if (!event.stopped_propagation() && !details.target_destroyed)
+      GetTextInputClient()->InsertText(text);
     composition_.Clear();
   }
 }
 
 void InputMethodAuraLinux::OnPreeditChanged(
     const CompositionText& composition_text) {
   if (suppress_next_result_ || IsTextInputTypeNone())
     return;
 
   if (is_sync_mode_) {
     if (!composition_.text.empty() || !composition_text.text.empty())
       composition_changed_ = true;
   } else {
-    SendFakeProcessKeyEvent(0);
-    GetTextInputClient()->SetCompositionText(composition_text);
+    ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, 0);
+    ui::EventDispatchDetails details = SendFakeProcessKeyEvent(&event);
+    if (details.dispatcher_destroyed)
+      return;
+    if (!event.stopped_propagation() && !details.target_destroyed)
+      GetTextInputClient()->SetCompositionText(composition_text);
   }
 
   composition_ = composition_text;
 }
 
 void InputMethodAuraLinux::OnPreeditEnd() {
   if (suppress_next_result_ || IsTextInputTypeNone())
     return;
 
   if (is_sync_mode_) {
     if (!composition_.text.empty()) {
       composition_.Clear();
       composition_changed_ = true;
     }
   } else {
     TextInputClient* client = GetTextInputClient();
     if (client && client->HasCompositionText()) {
-      SendFakeProcessKeyEvent(0);
-      client->ClearCompositionText();
+      ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, 0);
+      ui::EventDispatchDetails details = SendFakeProcessKeyEvent(&event);
+      if (details.dispatcher_destroyed)
+        return;
+      if (!event.stopped_propagation() && !details.target_destroyed)
+        client->ClearCompositionText();
     }
     composition_.Clear();
   }
 }
 
 // Overridden from InputMethodBase.
 
 void InputMethodAuraLinux::OnFocus() {
   InputMethodBase::OnFocus();
   UpdateContextFocusState();
@@ skipping 29 matching lines @@
 bool InputMethodAuraLinux::HasInputMethodResult() {
   return !result_text_.empty() || composition_changed_;
 }
 
 bool InputMethodAuraLinux::NeedInsertChar() const {
   return IsTextInputTypeNone() ||
          (!composition_changed_ && composition_.text.empty() &&
           result_text_.length() == 1);
 }
 
-bool InputMethodAuraLinux::SendFakeProcessKeyEvent(int flags) const {
-  return DispatchKeyEventPostIME(
-      KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, flags));
+ui::EventDispatchDetails InputMethodAuraLinux::SendFakeProcessKeyEvent(
+    ui::KeyEvent* event) const {
+  KeyEvent key_event(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, event->flags());
+  ui::EventDispatchDetails details = DispatchKeyEventPostIME(&key_event);
+  if (key_event.stopped_propagation())
+    event->StopPropagation();
+  return details;
 }
 
 void InputMethodAuraLinux::ConfirmCompositionText() {
   TextInputClient* client = GetTextInputClient();
   if (client && client->HasCompositionText())
     client->ConfirmCompositionText();
 
   ResetContext();
 }
 
 }  // namespace ui