Skip to content

Commit

Permalink
Refactoring for the InputMethod & InputMethodDelegate interfaces.
Browse files Browse the repository at this point in the history
1) DispatchKeyEvent returns void type instead of bool, as it is not used.
2) DispatchKeyEventPostIME returns EventDispatchDetails, so that the caller
   (InputMethod) can know whether its being destroyed after the call of
   DispatchKeyEventPostIME.
3) Both DispatchKeyEvent & DispatchKeyEventPostIME takes ui::KeyEvent* as
   the parameter, so that the stopped_propagation() state can be carried over.

Note: this cl doesn't try to change any existing behaviors/logics, except one thing:
The cl https://codereview.chromium.org/1236923003 may introduce a potential
regression which this cl tries to fix:
DesktopWindowTreeHostWin::DispatchKeyEventPostIME() always call
event->StopPropagation(). However, HWNDMessageHandler::OnKeyEvent() relies
on the event.handled() state to correctly call SetMsgHandled(FALSE).
This cl remove the StopPropagation() call in
DesktopWindowTreeHostWin::DispatchKeyEventPostIME() and let
InputMethodWin/RemoteInputMethodWin call it as appropriate.

TBR=sky@chromium.org
BUG=513917,474828
TEST=Trybots green.

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

Cr-Commit-Position: refs/heads/master@{#341704}
  • Loading branch information
shuchen authored and Commit bot committed Aug 4, 2015
1 parent d340429 commit 59c2a2c
Show file tree
Hide file tree
Showing 47 changed files with 430 additions and 381 deletions.
3 changes: 2 additions & 1 deletion ash/display/window_tree_host_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,8 @@ void WindowTreeHostManager::PostDisplayConfigurationChange() {
UpdateMouseLocationAfterDisplayChange();
}

bool WindowTreeHostManager::DispatchKeyEventPostIME(const ui::KeyEvent& event) {
ui::EventDispatchDetails WindowTreeHostManager::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
// Getting the active root window to dispatch the event. This isn't
// significant as the event will be sent to the window resolved by
// aura::client::FocusClient which is FocusController in ash.
Expand Down
3 changes: 2 additions & 1 deletion ash/display/window_tree_host_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ class ASH_EXPORT WindowTreeHostManager
void PostDisplayConfigurationChange() override;

// ui::internal::InputMethodDelegate overrides:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

InputMethodEventHandler* input_method_event_handler() {
return input_method_event_handler_.get();
Expand Down
14 changes: 6 additions & 8 deletions ash/host/ash_remote_window_tree_host_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,14 @@ void AshRemoteWindowTreeHostWin::UpdateRootWindowSize(
transformer_helper_.UpdateWindowSize(host_size);
}

bool AshRemoteWindowTreeHostWin::DispatchKeyEventPostIME(
const ui::KeyEvent& event) {
ui::KeyEvent event_copy(event);
ui::EventDispatchDetails AshRemoteWindowTreeHostWin::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
input_method_handler()->SetPostIME(true);
ui::EventDispatchDetails details =
event_processor()->OnEventFromSource(&event_copy);
if (details.dispatcher_destroyed)
return true;
input_method_handler()->SetPostIME(false);
return event_copy.stopped_propagation();
event_processor()->OnEventFromSource(event);
if (!details.dispatcher_destroyed)
input_method_handler()->SetPostIME(false);
return details;
}

} // namespace ash
3 changes: 2 additions & 1 deletion ash/host/ash_remote_window_tree_host_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class ASH_EXPORT AshRemoteWindowTreeHostWin
void UpdateRootWindowSize(const gfx::Size& host_size) override;

// ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

TransformerHelper transformer_helper_;

Expand Down
17 changes: 8 additions & 9 deletions ash/host/ash_window_tree_host_ozone.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class AshWindowTreeHostOzone : public AshWindowTreeHost,
void DispatchEvent(ui::Event* event) override;

// ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

// Temporarily disable the tap-to-click feature. Used on CrOS.
void SetTapToClickPaused(bool state);
Expand Down Expand Up @@ -137,16 +138,14 @@ void AshWindowTreeHostOzone::DispatchEvent(ui::Event* event) {
SendEventToProcessor(event);
}

bool AshWindowTreeHostOzone::DispatchKeyEventPostIME(
const ui::KeyEvent& event) {
ui::KeyEvent event_copy(event);
ui::EventDispatchDetails AshWindowTreeHostOzone::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
input_method_handler()->SetPostIME(true);
ui::EventDispatchDetails details =
event_processor()->OnEventFromSource(&event_copy);
if (details.dispatcher_destroyed)
return true;
input_method_handler()->SetPostIME(false);
return event_copy.stopped_propagation();
event_processor()->OnEventFromSource(event);
if (!details.dispatcher_destroyed)
input_method_handler()->SetPostIME(false);
return details;
}

void AshWindowTreeHostOzone::SetTapToClickPaused(bool state) {
Expand Down
14 changes: 6 additions & 8 deletions ash/host/ash_window_tree_host_unified.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,14 @@ void AshWindowTreeHostUnified::OnWindowDestroying(aura::Window* window) {
mirroring_hosts_.erase(iter);
}

bool AshWindowTreeHostUnified::DispatchKeyEventPostIME(
const ui::KeyEvent& event) {
ui::KeyEvent event_copy(event);
ui::EventDispatchDetails AshWindowTreeHostUnified::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
input_method_handler()->SetPostIME(true);
ui::EventDispatchDetails details =
event_processor()->OnEventFromSource(&event_copy);
if (details.dispatcher_destroyed)
return true;
input_method_handler()->SetPostIME(false);
return event_copy.stopped_propagation();
event_processor()->OnEventFromSource(event);
if (!details.dispatcher_destroyed)
input_method_handler()->SetPostIME(false);
return details;
}

} // namespace ash
3 changes: 2 additions & 1 deletion ash/host/ash_window_tree_host_unified.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ class AshWindowTreeHostUnified : public AshWindowTreeHost,
void OnWindowDestroying(aura::Window* window) override;

// ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

std::vector<AshWindowTreeHost*> mirroring_hosts_;

Expand Down
13 changes: 6 additions & 7 deletions ash/host/ash_window_tree_host_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,14 @@ class AshWindowTreeHostWin : public AshWindowTreeHost,
}

// ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override {
ui::KeyEvent event_copy(event);
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override {
input_method_handler()->SetPostIME(true);
ui::EventDispatchDetails details =
event_processor()->OnEventFromSource(&event_copy);
if (details.dispatcher_destroyed)
return true;
input_method_handler()->SetPostIME(false);
return event_copy.stopped_propagation();
event_processor()->OnEventFromSource(event);
if (!details.dispatcher_destroyed)
input_method_handler()->SetPostIME(false);
return details;
}

bool fullscreen_;
Expand Down
13 changes: 6 additions & 7 deletions ash/host/ash_window_tree_host_x11.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,14 @@ void AshWindowTreeHostX11::TranslateAndDispatchLocatedEvent(
SendEventToProcessor(event);
}

bool AshWindowTreeHostX11::DispatchKeyEventPostIME(const ui::KeyEvent& event) {
ui::KeyEvent event_copy(event);
ui::EventDispatchDetails AshWindowTreeHostX11::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
input_method_handler()->SetPostIME(true);
ui::EventDispatchDetails details =
event_processor()->OnEventFromSource(&event_copy);
if (details.dispatcher_destroyed)
return true;
input_method_handler()->SetPostIME(false);
return event_copy.stopped_propagation();
event_processor()->OnEventFromSource(event);
if (!details.dispatcher_destroyed)
input_method_handler()->SetPostIME(false);
return details;
}

#if defined(OS_CHROMEOS)
Expand Down
3 changes: 2 additions & 1 deletion ash/host/ash_window_tree_host_x11.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class ASH_EXPORT AshWindowTreeHostX11 : public AshWindowTreeHost,
void OnHostInitialized(aura::WindowTreeHost* host) override;

// ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

#if defined(OS_CHROMEOS)
// Set the CrOS touchpad "tap paused" property. It is used to temporarily
Expand Down
2 changes: 1 addition & 1 deletion ash/ime/input_method_event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void InputMethodEventHandler::OnKeyEvent(ui::KeyEvent* event) {
return;
if (post_ime_)
return;
input_method_->DispatchKeyEvent(*event);
input_method_->DispatchKeyEvent(event);
event->StopPropagation();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ bool InputMethodEngine::SendKeyEvents(
ui::EventTimeForNow());
base::AutoReset<const ui::KeyEvent*> reset_sent_key(&sent_key_event_,
&ui_event);
input_method->DispatchKeyEvent(ui_event);
input_method->DispatchKeyEvent(&ui_event);
}

return true;
Expand Down
22 changes: 12 additions & 10 deletions mandoline/ui/aura/input_method_mandoline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,27 @@ bool InputMethodMandoline::OnUntranslatedIMEMessage(
return false;
}

bool InputMethodMandoline::DispatchKeyEvent(const ui::KeyEvent& event) {
DCHECK(event.type() == ui::ET_KEY_PRESSED ||
event.type() == ui::ET_KEY_RELEASED);
void InputMethodMandoline::DispatchKeyEvent(ui::KeyEvent* event) {
DCHECK(event->type() == ui::ET_KEY_PRESSED ||
event->type() == ui::ET_KEY_RELEASED);

// If no text input client, do nothing.
if (!GetTextInputClient())
return DispatchKeyEventPostIME(event);
if (!GetTextInputClient()) {
ignore_result(DispatchKeyEventPostIME(event));
return;
}

// Here is where we change the differ from our base class's logic. Instead of
// always dispatching a key down event, and then sending a synthesized
// character event, we instead check to see if this is a character event and
// send out the key if it is. (We fallback to normal dispatch if it isn't.)
if (event.is_char()) {
GetTextInputClient()->InsertChar(event.GetCharacter(), event.flags());

return false;
if (event->is_char()) {
GetTextInputClient()->InsertChar(event->GetCharacter(), event->flags());
event->StopPropagation();
return;
}

return DispatchKeyEventPostIME(event);
ignore_result(DispatchKeyEventPostIME(event));
}

void InputMethodMandoline::OnCaretBoundsChanged(
Expand Down
2 changes: 1 addition & 1 deletion mandoline/ui/aura/input_method_mandoline.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class InputMethodMandoline : public ui::InputMethodBase {
// Overridden from ui::InputMethod:
bool OnUntranslatedIMEMessage(const base::NativeEvent& event,
NativeEventResult* result) override;
bool DispatchKeyEvent(const ui::KeyEvent& event) override;
void DispatchKeyEvent(ui::KeyEvent* event) override;
void OnCaretBoundsChanged(const ui::TextInputClient* client) override;
void CancelComposition(const ui::TextInputClient* client) override;
void OnInputLocaleChanged() override;
Expand Down
3 changes: 1 addition & 2 deletions mandoline/ui/aura/native_widget_view_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ void NativeWidgetViewManager::OnViewInputEvent(mojo::View* view,

if (ui_event->IsKeyEvent()) {
window_tree_host_->GetInputMethod()->DispatchKeyEvent(
*static_cast<ui::KeyEvent*>(ui_event.get()));
ui_event->StopPropagation();
static_cast<ui::KeyEvent*>(ui_event.get()));
} else {
window_tree_host_->SendEventToProcessor(ui_event.get());
}
Expand Down
8 changes: 3 additions & 5 deletions ui/aura/window_tree_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,9 @@ void WindowTreeHost::SetSharedInputMethod(ui::InputMethod* input_method) {
owned_input_method_ = false;
}

bool WindowTreeHost::DispatchKeyEventPostIME(const ui::KeyEvent& event) {
ui::KeyEvent copied_event(event);
ui::EventDispatchDetails details = SendEventToProcessor(&copied_event);
DCHECK(!details.dispatcher_destroyed);
return copied_event.stopped_propagation();
ui::EventDispatchDetails WindowTreeHost::DispatchKeyEventPostIME(
ui::KeyEvent* event) {
return SendEventToProcessor(event);
}

void WindowTreeHost::Show() {
Expand Down
3 changes: 2 additions & 1 deletion ui/aura/window_tree_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ class AURA_EXPORT WindowTreeHost : public ui::internal::InputMethodDelegate,
void SetSharedInputMethod(ui::InputMethod* input_method);

// Overridden from ui::internal::InputMethodDelegate:
bool DispatchKeyEventPostIME(const ui::KeyEvent& event) override;
ui::EventDispatchDetails DispatchKeyEventPostIME(
ui::KeyEvent* event) override;

// Returns the EventSource responsible for dispatching events to the window
// tree.
Expand Down
3 changes: 1 addition & 2 deletions ui/base/ime/dummy_input_method.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ TextInputClient* DummyInputMethod::GetTextInputClient() const {
return NULL;
}

bool DummyInputMethod::DispatchKeyEvent(const ui::KeyEvent& event) {
return false;
void DummyInputMethod::DispatchKeyEvent(ui::KeyEvent* event) {
}

void DummyInputMethod::OnTextInputTypeChanged(const TextInputClient* client) {
Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/dummy_input_method.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class DummyInputMethod : public InputMethod {
void SetFocusedTextInputClient(TextInputClient* client) override;
void DetachTextInputClient(TextInputClient* client) override;
TextInputClient* GetTextInputClient() const override;
bool DispatchKeyEvent(const ui::KeyEvent& event) override;
void DispatchKeyEvent(ui::KeyEvent* event) override;
void OnTextInputTypeChanged(const TextInputClient* client) override;
void OnCaretBoundsChanged(const TextInputClient* client) override;
void CancelComposition(const TextInputClient* client) override;
Expand Down
3 changes: 1 addition & 2 deletions ui/base/ime/input_method.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class InputMethod {
// dispatched back to the caller via
// ui::InputMethodDelegate::DispatchKeyEventPostIME(), once it's processed by
// the input method. It should only be called by a message dispatcher.
// Returns true if the event was processed.
virtual bool DispatchKeyEvent(const ui::KeyEvent& event) = 0;
virtual void DispatchKeyEvent(ui::KeyEvent* event) = 0;

// Called by the focused client whenever its text input type is changed.
// Before calling this method, the focused client must confirm or clear
Expand Down
Loading

0 comments on commit 59c2a2c

Please sign in to comment.