Skip to content

Commit

Permalink
IME for Mus: Send TextInputClient information to IMEDriver.
Browse files Browse the repository at this point in the history
Client sends type, mode, direction, flags, and caret bounds to IMEDriver
when initializing a session. It can update type and caret bounds later
using methods in InputMethod.

Almost all other NOTIMPLEMENED() functions in RemoteTextInputClient are
used solely by InputMethodChromeOS::OnCaretBoundsChanged() or its
callees, which we might be able to avoid implementing in the IMEDriver
side and handle in the client side. So we don't touch them in this CL
and leave them to other CLs.

BUG=631527

Review-Url: https://codereview.chromium.org/2626983003
Cr-Commit-Position: refs/heads/master@{#443929}
  • Loading branch information
moshayedi authored and Commit bot committed Jan 16, 2017
1 parent 43732b2 commit 0869ab1
Show file tree
Hide file tree
Showing 15 changed files with 115 additions and 67 deletions.
16 changes: 10 additions & 6 deletions chrome/browser/ui/views/ime_driver/ime_driver_mus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/ui/views/ime_driver/ime_driver_mus.h"

#include "base/memory/ptr_util.h"
#include "chrome/browser/ui/views/ime_driver/remote_text_input_client.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/common/service_manager_connection.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
Expand Down Expand Up @@ -38,15 +39,18 @@ void IMEDriver::Register() {
ime_registrar->RegisterDriver(std::move(ime_driver_ptr));
}

void IMEDriver::StartSession(
int32_t session_id,
ui::mojom::TextInputClientPtr client,
ui::mojom::InputMethodRequest input_method_request) {
void IMEDriver::StartSession(int32_t session_id,
ui::mojom::StartSessionDetailsPtr details) {
#if defined(OS_CHROMEOS)
std::unique_ptr<RemoteTextInputClient> remote_client =
base::MakeUnique<RemoteTextInputClient>(
std::move(details->client), details->text_input_type,
details->text_input_mode, details->text_direction,
details->text_input_flags, details->caret_bounds);
input_method_bindings_[session_id] =
base::MakeUnique<mojo::Binding<ui::mojom::InputMethod>>(
new InputMethodBridge(std::move(client)),
std::move(input_method_request));
new InputMethodBridge(std::move(remote_client)),
std::move(details->input_method_request));
#else
input_method_bindings_[session_id] =
base::MakeUnique<mojo::Binding<ui::mojom::InputMethod>>(
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/ui/views/ime_driver/ime_driver_mus.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ class IMEDriver : public ui::mojom::IMEDriver {

private:
// ui::mojom::IMEDriver:
void StartSession(
int32_t session_id,
ui::mojom::TextInputClientPtr client,
ui::mojom::InputMethodRequest input_method_request) override;
void StartSession(int32_t session_id,
ui::mojom::StartSessionDetailsPtr details) override;
void CancelSession(int32_t session_id) override;

std::map<int32_t, std::unique_ptr<mojo::Binding<ui::mojom::InputMethod>>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
#include "base/memory/ptr_util.h"
#include "chrome/browser/ui/views/ime_driver/remote_text_input_client.h"

InputMethodBridge::InputMethodBridge(ui::mojom::TextInputClientPtr client)
: client_(base::MakeUnique<RemoteTextInputClient>(std::move(client))),
InputMethodBridge::InputMethodBridge(
std::unique_ptr<RemoteTextInputClient> client)
: client_(std::move(client)),
input_method_chromeos_(
base::MakeUnique<ui::InputMethodChromeOS>(nullptr)) {
input_method_chromeos_->SetFocusedTextInputClient(client_.get());
Expand All @@ -18,10 +19,12 @@ InputMethodBridge::~InputMethodBridge() {}

void InputMethodBridge::OnTextInputTypeChanged(
ui::TextInputType text_input_type) {
client_->SetTextInputType(text_input_type);
input_method_chromeos_->OnTextInputTypeChanged(client_.get());
}

void InputMethodBridge::OnCaretBoundsChanged(const gfx::Rect& caret_bounds) {
client_->SetCaretBounds(caret_bounds);
input_method_chromeos_->OnCaretBoundsChanged(client_.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
#ifndef CHROME_BROWSER_UI_VIEWS_IME_DRIVER_INPUT_METHOD_BRIDGE_CHROMEOS_H_
#define CHROME_BROWSER_UI_VIEWS_IME_DRIVER_INPUT_METHOD_BRIDGE_CHROMEOS_H_

#include "chrome/browser/ui/views/ime_driver/remote_text_input_client.h"
#include "services/ui/public/interfaces/ime/ime.mojom.h"
#include "ui/base/ime/input_method_chromeos.h"

// This bridges between mojo InputMethod API and ui::InputMethodChromeOS. It
// forwards the received events to an instance of ui::InputMethodChromeOS.
class InputMethodBridge : public ui::mojom::InputMethod {
public:
explicit InputMethodBridge(ui::mojom::TextInputClientPtr client);
explicit InputMethodBridge(std::unique_ptr<RemoteTextInputClient> client);
~InputMethodBridge() override;

// ui::mojom::InputMethod:
Expand All @@ -23,7 +24,7 @@ class InputMethodBridge : public ui::mojom::InputMethod {
void CancelComposition() override;

private:
std::unique_ptr<ui::TextInputClient> client_;
std::unique_ptr<RemoteTextInputClient> client_;
std::unique_ptr<ui::InputMethodChromeOS> input_method_chromeos_;

DISALLOW_COPY_AND_ASSIGN(InputMethodBridge);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ class InputMethodBridgeChromeOSTest : public testing::Test {

ui::mojom::TextInputClientPtr client_ptr;
client_ = base::MakeUnique<TestTextInputClient>(MakeRequest(&client_ptr));
input_method_ = base::MakeUnique<InputMethodBridge>(std::move(client_ptr));
input_method_ = base::MakeUnique<InputMethodBridge>(
base::MakeUnique<RemoteTextInputClient>(
std::move(client_ptr), ui::TEXT_INPUT_TYPE_TEXT,
ui::TEXT_INPUT_MODE_DEFAULT, base::i18n::LEFT_TO_RIGHT, 0,
gfx::Rect()));
}

bool ProcessKeyEvent(std::unique_ptr<ui::Event> event) {
Expand Down
43 changes: 26 additions & 17 deletions chrome/browser/ui/views/ime_driver/remote_text_input_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,30 @@
#include "base/strings/utf_string_conversions.h"

RemoteTextInputClient::RemoteTextInputClient(
ui::mojom::TextInputClientPtr remote_client)
: remote_client_(std::move(remote_client)) {}
ui::mojom::TextInputClientPtr remote_client,
ui::TextInputType text_input_type,
ui::TextInputMode text_input_mode,
base::i18n::TextDirection text_direction,
int text_input_flags,
gfx::Rect caret_bounds)
: remote_client_(std::move(remote_client)),
text_input_type_(text_input_type),
text_input_mode_(text_input_mode),
text_direction_(text_direction),
text_input_flags_(text_input_flags),
caret_bounds_(caret_bounds) {}

RemoteTextInputClient::~RemoteTextInputClient() {}

void RemoteTextInputClient::SetTextInputType(
ui::TextInputType text_input_type) {
text_input_type_ = text_input_type;
}

void RemoteTextInputClient::SetCaretBounds(const gfx::Rect& caret_bounds) {
caret_bounds_ = caret_bounds;
}

void RemoteTextInputClient::SetCompositionText(
const ui::CompositionText& composition) {
remote_client_->SetCompositionText(composition);
Expand All @@ -39,27 +58,19 @@ void RemoteTextInputClient::InsertChar(const ui::KeyEvent& event) {
}

ui::TextInputType RemoteTextInputClient::GetTextInputType() const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED();
return ui::TEXT_INPUT_TYPE_TEXT;
return text_input_type_;
}

ui::TextInputMode RemoteTextInputClient::GetTextInputMode() const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED();
return ui::TEXT_INPUT_MODE_DEFAULT;
return text_input_mode_;
}

base::i18n::TextDirection RemoteTextInputClient::GetTextDirection() const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED();
return base::i18n::UNKNOWN_DIRECTION;
return text_direction_;
}

int RemoteTextInputClient::GetTextInputFlags() const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED();
return 0;
return text_input_flags_;
}

bool RemoteTextInputClient::CanComposeInline() const {
Expand All @@ -70,9 +81,7 @@ bool RemoteTextInputClient::CanComposeInline() const {
}

gfx::Rect RemoteTextInputClient::GetCaretBounds() const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED();
return gfx::Rect();
return caret_bounds_;
}

bool RemoteTextInputClient::GetCompositionCharacterBounds(
Expand Down
15 changes: 14 additions & 1 deletion chrome/browser/ui/views/ime_driver/remote_text_input_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,17 @@
// ui::InputMethod::SetFocusedTextInputClient().
class RemoteTextInputClient : public ui::TextInputClient {
public:
explicit RemoteTextInputClient(ui::mojom::TextInputClientPtr remote_client);
RemoteTextInputClient(ui::mojom::TextInputClientPtr remote_client,
ui::TextInputType text_input_type,
ui::TextInputMode text_input_mode,
base::i18n::TextDirection text_direction,
int text_input_flags,
gfx::Rect caret_bounds);
~RemoteTextInputClient() override;

void SetTextInputType(ui::TextInputType text_input_type);
void SetCaretBounds(const gfx::Rect& caret_bounds);

private:
// ui::TextInputClient:
void SetCompositionText(const ui::CompositionText& composition) override;
Expand Down Expand Up @@ -48,6 +56,11 @@ class RemoteTextInputClient : public ui::TextInputClient {
void SetTextEditCommandForNextKeyEvent(ui::TextEditCommand command) override;

ui::mojom::TextInputClientPtr remote_client_;
ui::TextInputType text_input_type_;
ui::TextInputMode text_input_mode_;
base::i18n::TextDirection text_direction_;
int text_input_flags_;
gfx::Rect caret_bounds_;

DISALLOW_COPY_AND_ASSIGN(RemoteTextInputClient);
};
Expand Down
15 changes: 5 additions & 10 deletions services/ui/ime/ime_server_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "services/ui/ime/ime_server_impl.h"

#include "base/memory/ptr_util.h"
#include "services/catalog/public/interfaces/constants.mojom.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/ui/ime/ime_registrar_impl.h"
Expand Down Expand Up @@ -46,26 +47,20 @@ void IMEServerImpl::OnDriverChanged(mojom::IMEDriverPtr driver) {
driver_ = std::move(driver);

while (!pending_requests_.empty()) {
driver_->StartSession(current_id_++,
std::move(pending_requests_.front().first),
std::move(pending_requests_.front().second));
driver_->StartSession(current_id_++, std::move(pending_requests_.front()));
pending_requests_.pop();
}
}

void IMEServerImpl::StartSession(
mojom::TextInputClientPtr client,
mojom::InputMethodRequest input_method_request) {
void IMEServerImpl::StartSession(mojom::StartSessionDetailsPtr details) {
if (driver_.get()) {
// TODO(moshayedi): crbug.com/634431. This will forward all calls from
// clients to the driver as they are. We may need to check |caret_bounds|
// parameter of InputMethod::OnCaretBoundsChanged() here and limit them to
// client's focused window.
driver_->StartSession(current_id_++, std::move(client),
std::move(input_method_request));
driver_->StartSession(current_id_++, std::move(details));
} else {
pending_requests_.push(
std::make_pair(std::move(client), std::move(input_method_request)));
pending_requests_.push(std::move(details));
}
}

Expand Down
7 changes: 2 additions & 5 deletions services/ui/ime/ime_server_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ class IMEServerImpl : public mojom::IMEServer {

private:
// mojom::IMEServer:
void StartSession(mojom::TextInputClientPtr client,
mojom::InputMethodRequest input_method) override;
void StartSession(mojom::StartSessionDetailsPtr details) override;

void OnGotCatalogEntries(std::vector<catalog::mojom::EntryPtr> entries);

Expand All @@ -39,9 +38,7 @@ class IMEServerImpl : public mojom::IMEServer {
mojom::IMEDriverPtr driver_;
int current_id_;

using PendingRequest =
std::pair<mojom::TextInputClientPtr, mojom::InputMethodRequest>;
std::queue<PendingRequest> pending_requests_;
std::queue<mojom::StartSessionDetailsPtr> pending_requests_;

DISALLOW_COPY_AND_ASSIGN(IMEServerImpl);
};
Expand Down
6 changes: 5 additions & 1 deletion services/ui/ime/ime_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ TEST_F(IMEAppTest, ProcessKeyEvent) {
TestTextInputClient client(MakeRequest(&client_ptr));

ui::mojom::InputMethodPtr input_method;
ime_server_->StartSession(std::move(client_ptr), MakeRequest(&input_method));
ui::mojom::StartSessionDetailsPtr details =
ui::mojom::StartSessionDetails::New();
details->client = std::move(client_ptr);
details->input_method_request = MakeRequest(&input_method);
ime_server_->StartSession(std::move(details));

// Send character key event.
ui::KeyEvent char_event('A', ui::VKEY_A, 0);
Expand Down
10 changes: 4 additions & 6 deletions services/ui/ime/test_ime_driver/test_ime_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ TestIMEDriver::TestIMEDriver() {}

TestIMEDriver::~TestIMEDriver() {}

void TestIMEDriver::StartSession(
int32_t session_id,
mojom::TextInputClientPtr client,
mojom::InputMethodRequest input_method_request) {
void TestIMEDriver::StartSession(int32_t session_id,
mojom::StartSessionDetailsPtr details) {
input_method_bindings_[session_id].reset(
new mojo::Binding<mojom::InputMethod>(
new TestInputMethod(std::move(client)),
std::move(input_method_request)));
new TestInputMethod(std::move(details->client)),
std::move(details->input_method_request)));
}

void TestIMEDriver::CancelSession(int32_t session_id) {
Expand Down
6 changes: 2 additions & 4 deletions services/ui/ime/test_ime_driver/test_ime_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ class TestIMEDriver : public ui::mojom::IMEDriver {

private:
// ui::mojom::IMEDriver:
void StartSession(
int32_t session_id,
ui::mojom::TextInputClientPtr client,
ui::mojom::InputMethodRequest input_method_request) override;
void StartSession(int32_t session_id,
ui::mojom::StartSessionDetailsPtr details) override;
void CancelSession(int32_t session_id) override;

std::map<int32_t, std::unique_ptr<mojo::Binding<mojom::InputMethod>>>
Expand Down
1 change: 1 addition & 0 deletions services/ui/public/interfaces/ime/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mojom("ime") {
]

public_deps = [
"//mojo/common:common_custom_types",
"//ui/events/mojo:interfaces",
"//ui/gfx/geometry/mojo",
"//ui/gfx/range/mojo",
Expand Down
20 changes: 16 additions & 4 deletions services/ui/public/interfaces/ime/ime.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

module ui.mojom;

import "mojo/common/text_direction.mojom";
import "ui/events/mojo/event.mojom";
import "ui/gfx/geometry/mojo/geometry.mojom";
import "ui/gfx/range/mojo/range.mojom";
Expand Down Expand Up @@ -64,23 +65,34 @@ enum TextInputMode {
URL,
};

// Parameters needed to start an IME session.
struct StartSessionDetails {
TextInputClient client;
InputMethod& input_method_request;

// Initial details about |client| required by IMEDriver.
TextInputType text_input_type;
TextInputMode text_input_mode;
mojo.common.mojom.TextDirection text_direction;
int32 text_input_flags;
gfx.mojom.Rect caret_bounds;
};

// A service which provides the IMEDriver interface is responsible for doing
// the composition logic. After starting a session, it receives events from
// the client via the InputMethod interface, and sends composition events to
// the client via the TextInputClient.
interface IMEDriver {
// session_id is unique and generated by Mus.
StartSession(int32 session_id, TextInputClient client,
InputMethod& input_method);
StartSession(int32 session_id, StartSessionDetails details);
CancelSession(int32 session_id);
};

// Clients use IME using the IMEServer interface which is provided by Mus. Mus
// does minimal processing and mostly just acts as lightweight proxy between
// the client app and the registered IME driver.
interface IMEServer {
StartSession(TextInputClient client,
InputMethod& input_method);
StartSession(StartSessionDetails details);
};

// An IME driver register should register itself to Mus using the IMERegistrar
Expand Down
Loading

0 comments on commit 0869ab1

Please sign in to comment.