 Chromium Code Reviews
 Chromium Code Reviews Issue 1779673003:
  Added network components for blimp text input feature  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1779673003:
  Added network components for blimp text input feature  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: blimp/engine/session/blimp_engine_session.cc | 
| diff --git a/blimp/engine/session/blimp_engine_session.cc b/blimp/engine/session/blimp_engine_session.cc | 
| index 5ba532c29f82f2bfc0700a8037d76d8f9c254718..81672b10bf0d992a8ea8a187d17cbecb2505d20f 100644 | 
| --- a/blimp/engine/session/blimp_engine_session.cc | 
| +++ b/blimp/engine/session/blimp_engine_session.cc | 
| @@ -38,6 +38,8 @@ | 
| #include "ui/aura/env.h" | 
| #include "ui/aura/window.h" | 
| #include "ui/aura/window_tree_host.h" | 
| +#include "ui/base/ime/input_method.h" | 
| +#include "ui/base/ime/text_input_client.h" | 
| #include "ui/gfx/geometry/size.h" | 
| #include "ui/wm/core/base_focus_rules.h" | 
| #include "ui/wm/core/default_activation_client.h" | 
| @@ -170,6 +172,8 @@ void BlimpEngineSession::Initialize() { | 
| capture_client_.reset( | 
| new aura::client::DefaultCaptureClient(window_tree_host_->window())); | 
| + window_tree_host_->GetInputMethod()->AddObserver(this); | 
| 
nyquist
2016/03/10 00:57:13
Does this observer need to be removed? Also, this
 
shaktisahu
2016/03/15 23:44:13
Done.
 | 
| + | 
| window_tree_host_->SetBounds(gfx::Rect(screen_->GetPrimaryDisplay().size())); | 
| RegisterFeatures(); | 
| @@ -205,6 +209,8 @@ void BlimpEngineSession::RegisterFeatures() { | 
| render_widget_feature_.set_compositor_message_sender( | 
| thread_pipe_manager_->RegisterFeature(BlimpMessage::COMPOSITOR, | 
| &render_widget_feature_)); | 
| + ime_message_sender_ = | 
| + thread_pipe_manager_->RegisterFeature(BlimpMessage::IME, this); | 
| 
nyquist
2016/03/10 00:57:13
I was hoping we wouldn't have to pass in |this| he
 
shaktisahu
2016/03/15 23:44:13
Done.
 | 
| } | 
| bool BlimpEngineSession::CreateWebContents(const int target_tab_id) { | 
| @@ -289,12 +295,65 @@ void BlimpEngineSession::OnCompositorMessageReceived( | 
| render_widget_host->HandleCompositorProto(message); | 
| } | 
| +void BlimpEngineSession::OnTextInputTypeChanged( | 
| + const ui::TextInputClient* client) {} | 
| + | 
| +void BlimpEngineSession::OnFocus() {} | 
| + | 
| +void BlimpEngineSession::OnBlur() {} | 
| + | 
| +void BlimpEngineSession::OnCaretBoundsChanged( | 
| + const ui::TextInputClient* client) {} | 
| + | 
| +// Called when either: | 
| +// - the TextInputClient is changed (e.g. by a change of focus) | 
| +// - the TextInputType of the TextInputClient changes | 
| +void BlimpEngineSession::OnTextInputStateChanged( | 
| 
nyquist
2016/03/10 00:58:48
And this should hoepfully be tested as well.
 | 
| + const ui::TextInputClient* client) { | 
| 
Khushal
2016/03/10 07:54:10
The RenderWidgetHostView is also a text input clie
 | 
| + ui::TextInputType type = | 
| + client ? client->GetTextInputType() : ui::TEXT_INPUT_TYPE_NONE; | 
| + | 
| + // Hide IME if text input is out of focus | 
| + if (type == ui::TEXT_INPUT_TYPE_NONE) { | 
| 
nyquist
2016/03/10 00:57:13
Nit: Braces are optional for single-line statement
 | 
| + ShowIme(false); | 
| + } | 
| +} | 
| + | 
| +void BlimpEngineSession::OnInputMethodDestroyed( | 
| + const ui::InputMethod* input_method) {} | 
| + | 
| +// Called when a user input should trigger showing the IME. | 
| +void BlimpEngineSession::OnShowImeIfNeeded() { | 
| + ShowIme(true); | 
| +} | 
| + | 
| +void BlimpEngineSession::ShowIme(bool show) { | 
| 
nyquist
2016/03/10 00:58:48
It feels like this should be tested.
 | 
| + ImeMessage* ime_message; | 
| + scoped_ptr<BlimpMessage> blimp_message = CreateBlimpMessage(&ime_message); | 
| 
David Trainor- moved to gerrit
2016/03/14 17:59:42
We probably also want to attach the tab id and the
 
shaktisahu
2016/03/15 23:44:13
Done.
 | 
| + | 
| + ime_message->set_type(show ? ImeMessage::SHOW_IME : ImeMessage::HIDE_IME); | 
| + | 
| + ui::TextInputClient* client = | 
| + window_tree_host_->GetInputMethod()->GetTextInputClient(); | 
| + | 
| + gfx::Range text_range; | 
| + base::string16 existing_text; | 
| + client->GetTextRange(&text_range); | 
| + client->GetTextFromRange(text_range, &existing_text); | 
| + | 
| + ime_message->set_text_input_type(client->GetTextInputType()); | 
| 
David Trainor- moved to gerrit
2016/03/14 17:59:42
We should pull this out to some protocol format an
 
shaktisahu
2016/03/15 23:44:13
Done.
 | 
| + ime_message->set_ime_text(base::UTF16ToUTF8(existing_text)); | 
| + ime_message_sender_->ProcessMessage(std::move(blimp_message), | 
| + net::CompletionCallback()); | 
| +} | 
| + | 
| void BlimpEngineSession::ProcessMessage( | 
| scoped_ptr<BlimpMessage> message, | 
| const net::CompletionCallback& callback) { | 
| DCHECK(!callback.is_null()); | 
| DCHECK(message->type() == BlimpMessage::TAB_CONTROL || | 
| - message->type() == BlimpMessage::NAVIGATION); | 
| + message->type() == BlimpMessage::NAVIGATION || | 
| + message->type() == BlimpMessage::IME); | 
| net::Error result = net::OK; | 
| if (message->type() == BlimpMessage::TAB_CONTROL) { | 
| @@ -333,6 +392,23 @@ void BlimpEngineSession::ProcessMessage( | 
| NOTIMPLEMENTED(); | 
| result = net::ERR_NOT_IMPLEMENTED; | 
| } | 
| + } else if (message->type() == BlimpMessage::IME) { | 
| 
nyquist
2016/03/10 00:58:48
And this should hopefully be tested?
 | 
| + const ImeMessage& ime = message->ime(); | 
| + DCHECK(ime.type() == ImeMessage::SHOW_TEXT); | 
| + | 
| + ui::TextInputClient* client = | 
| + window_tree_host_->GetInputMethod()->GetTextInputClient(); | 
| + | 
| + if (client && client->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE) { | 
| + // Clear out any existing text first and | 
| + // then insert new text entered through IME. | 
| + gfx::Range text_range; | 
| + client->GetTextRange(&text_range); | 
| + client->ExtendSelectionAndDelete(text_range.length(), | 
| + text_range.length()); | 
| + | 
| + client->InsertText(base::UTF8ToUTF16(ime.ime_text())); | 
| + } | 
| } else { | 
| DVLOG(1) << "No WebContents for navigation control"; | 
| result = net::ERR_FAILED; |