Commit 6e2b3399 authored by Leo Zhang's avatar Leo Zhang Committed by Commit Bot

Convert IME service to new Mojo types

Before a large code refactoring in IME service, convert it to new Mojo
types first in order to avoid any conflict during the refactoring with
an ongoing Mojo types converting.

TEST= /out/ime/chromeos_unittests --gtest_filter=ImeServiceTest*

Bug: 837156
Change-Id: I8ae669cb2fe9da980364696675278d3dc1e9516b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632131
Commit-Queue: Leo Zhang <googleo@chromium.org>
Reviewed-by: default avatarSam McNally <sammc@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#664149}
parent a1124fea
......@@ -15,16 +15,17 @@
namespace chromeos {
namespace ime {
ImeService::ImeService(service_manager::mojom::ServiceRequest request)
: service_binding_(this, std::move(request)) {}
ImeService::ImeService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver)
: service_binding_(this, std::move(receiver)) {}
ImeService::~ImeService() = default;
void ImeService::OnStart() {
binder_registry_.AddInterface<mojom::InputEngineManager>(base::BindRepeating(
&ImeService::BindInputEngineManagerRequest, base::Unretained(this)));
binders_.Add(base::BindRepeating(&ImeService::AddInputEngineManagerReceiver,
base::Unretained(this)));
engine_manager_bindings_.set_connection_error_handler(base::BindRepeating(
manager_receivers_.set_disconnect_handler(base::BindRepeating(
&ImeService::OnConnectionLost, base::Unretained(this)));
#if BUILDFLAG(ENABLE_CROS_IME_DECODER)
......@@ -38,14 +39,14 @@ void ImeService::OnStart() {
void ImeService::OnBindInterface(
const service_manager::BindSourceInfo& source_info,
const std::string& interface_name,
mojo::ScopedMessagePipeHandle interface_pipe) {
binder_registry_.BindInterface(interface_name, std::move(interface_pipe));
mojo::ScopedMessagePipeHandle receiver_pipe) {
binders_.TryBind(interface_name, &receiver_pipe);
}
void ImeService::ConnectToImeEngine(
const std::string& ime_spec,
mojom::InputChannelRequest to_engine_request,
mojom::InputChannelPtr from_engine,
mojo::PendingReceiver<mojom::InputChannel> to_engine_request,
mojo::PendingRemote<mojom::InputChannel> from_engine,
const std::vector<uint8_t>& extra,
ConnectToImeEngineCallback callback) {
DCHECK(input_engine_);
......@@ -54,14 +55,14 @@ void ImeService::ConnectToImeEngine(
std::move(callback).Run(bound);
}
void ImeService::BindInputEngineManagerRequest(
mojom::InputEngineManagerRequest request) {
engine_manager_bindings_.AddBinding(this, std::move(request));
void ImeService::AddInputEngineManagerReceiver(
mojo::PendingReceiver<mojom::InputEngineManager> receiver) {
manager_receivers_.Add(this, std::move(receiver));
// TODO(https://crbug.com/837156): Reset the cleanup timer.
}
void ImeService::OnConnectionLost() {
if (engine_manager_bindings_.empty()) {
if (manager_receivers_.empty()) {
service_binding_.RequestClose();
// TODO(https://crbug.com/837156): Set a timer to start a cleanup.
}
......
......@@ -7,8 +7,10 @@
#include "chromeos/services/ime/input_engine.h"
#include "chromeos/services/ime/public/mojom/input_engine.mojom.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "services/service_manager/public/cpp/binder_map.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/mojom/service.mojom.h"
......@@ -19,7 +21,8 @@ namespace ime {
class ImeService : public service_manager::Service,
public mojom::InputEngineManager {
public:
explicit ImeService(service_manager::mojom::ServiceRequest request);
explicit ImeService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver);
~ImeService() override;
private:
......@@ -30,14 +33,16 @@ class ImeService : public service_manager::Service,
mojo::ScopedMessagePipeHandle interface_pipe) override;
// mojom::InputEngineManager overrides:
void ConnectToImeEngine(const std::string& ime_spec,
mojom::InputChannelRequest to_engine_request,
mojom::InputChannelPtr from_engine,
const std::vector<uint8_t>& extra,
ConnectToImeEngineCallback callback) override;
void ConnectToImeEngine(
const std::string& ime_spec,
mojo::PendingReceiver<mojom::InputChannel> to_engine_request,
mojo::PendingRemote<mojom::InputChannel> from_engine,
const std::vector<uint8_t>& extra,
ConnectToImeEngineCallback callback) override;
// Binds the mojom::InputEngineManager interface to this object.
void BindInputEngineManagerRequest(mojom::InputEngineManagerRequest request);
// Adds a mojom::InputEngineManager receiver to this object.
void AddInputEngineManagerReceiver(
mojo::PendingReceiver<mojom::InputEngineManager> receiver);
void OnConnectionLost();
......@@ -47,9 +52,9 @@ class ImeService : public service_manager::Service,
// input engine instance.
std::unique_ptr<InputEngine> input_engine_;
mojo::BindingSet<mojom::InputEngineManager> engine_manager_bindings_;
mojo::ReceiverSet<mojom::InputEngineManager> manager_receivers_;
service_manager::BinderRegistry binder_registry_;
service_manager::BinderMap binders_;
DISALLOW_COPY_AND_ASSIGN(ImeService);
};
......
......@@ -8,9 +8,10 @@
#include "base/test/scoped_task_environment.h"
#include "chromeos/services/ime/public/mojom/constants.mojom.h"
#include "chromeos/services/ime/public/mojom/input_engine.mojom.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/cpp/test/test_connector_factory.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -37,12 +38,10 @@ void TestProcessTextCallback(std::string* res_out,
class TestClientChannel : mojom::InputChannel {
public:
TestClientChannel() : binding_(this) {}
TestClientChannel() : receiver_(this) {}
mojom::InputChannelPtr CreateInterfacePtrAndBind() {
mojom::InputChannelPtr ptr;
binding_.Bind(mojo::MakeRequest(&ptr));
return ptr;
mojo::PendingRemote<mojom::InputChannel> CreatePendingRemote() {
return receiver_.BindNewPipeAndPassRemote();
}
// mojom::InputChannel implementation.
......@@ -52,7 +51,7 @@ class TestClientChannel : mojom::InputChannel {
ProcessMessageCallback));
private:
mojo::Binding<mojom::InputChannel> binding_;
mojo::Receiver<mojom::InputChannel> receiver_;
DISALLOW_COPY_AND_ASSIGN(TestClientChannel);
};
......@@ -60,8 +59,8 @@ class TestClientChannel : mojom::InputChannel {
class ImeServiceTest : public testing::Test {
public:
ImeServiceTest()
: service_(test_connector_factory_.RegisterInstance(mojom::kServiceName)),
connector_(test_connector_factory_.CreateConnector()) {}
: service_(
test_connector_factory_.RegisterInstance(mojom::kServiceName)) {}
~ImeServiceTest() override = default;
MOCK_METHOD1(SentTextCallback, void(const std::string&));
......@@ -69,20 +68,16 @@ class ImeServiceTest : public testing::Test {
protected:
void SetUp() override {
connector_->BindInterface(mojom::kServiceName,
mojo::MakeRequest(&ime_manager_));
// TODO(https://crbug.com/837156): Start or bind other services used.
// Eg. connector()->StartService(mojom::kSomeServiceName);
test_connector_factory_.GetDefaultConnector()->Connect(
mojom::kServiceName, remote_manager_.BindNewPipeAndPassReceiver());
}
mojom::InputEngineManagerPtr ime_manager_;
mojo::Remote<mojom::InputEngineManager> remote_manager_;
private:
base::test::ScopedTaskEnvironment task_environment_;
service_manager::TestConnectorFactory test_connector_factory_;
ImeService service_;
std::unique_ptr<service_manager::Connector> connector_;
DISALLOW_COPY_AND_ASSIGN(ImeServiceTest);
};
......@@ -94,58 +89,58 @@ class ImeServiceTest : public testing::Test {
TEST_F(ImeServiceTest, ConnectInvalidImeEngine) {
bool success = true;
TestClientChannel test_channel;
mojom::InputChannelPtr to_engine_ptr;
mojo::Remote<mojom::InputChannel> remote_engine;
ime_manager_->ConnectToImeEngine(
kInvalidImeSpec, mojo::MakeRequest(&to_engine_ptr),
test_channel.CreateInterfacePtrAndBind(), extra,
remote_manager_->ConnectToImeEngine(
kInvalidImeSpec, remote_engine.BindNewPipeAndPassReceiver(),
test_channel.CreatePendingRemote(), extra,
base::BindOnce(&ConnectCallback, &success));
ime_manager_.FlushForTesting();
remote_manager_.FlushForTesting();
EXPECT_FALSE(success);
}
TEST_F(ImeServiceTest, MultipleClients) {
bool success = false;
TestClientChannel test_channel1;
TestClientChannel test_channel2;
mojom::InputChannelPtr to_engine_ptr1;
mojom::InputChannelPtr to_engine_ptr2;
ime_manager_->ConnectToImeEngine(
"m17n:ar", mojo::MakeRequest(&to_engine_ptr1),
test_channel1.CreateInterfacePtrAndBind(), extra,
TestClientChannel test_channel_1;
TestClientChannel test_channel_2;
mojo::Remote<mojom::InputChannel> remote_engine_1;
mojo::Remote<mojom::InputChannel> remote_engine_2;
remote_manager_->ConnectToImeEngine(
"m17n:ar", remote_engine_1.BindNewPipeAndPassReceiver(),
test_channel_1.CreatePendingRemote(), extra,
base::BindOnce(&ConnectCallback, &success));
ime_manager_.FlushForTesting();
remote_manager_.FlushForTesting();
ime_manager_->ConnectToImeEngine(
"m17n:ar", mojo::MakeRequest(&to_engine_ptr2),
test_channel2.CreateInterfacePtrAndBind(), extra,
remote_manager_->ConnectToImeEngine(
"m17n:ar", remote_engine_2.BindNewPipeAndPassReceiver(),
test_channel_2.CreatePendingRemote(), extra,
base::BindOnce(&ConnectCallback, &success));
ime_manager_.FlushForTesting();
remote_manager_.FlushForTesting();
std::string response;
std::string process_text_key =
"{\"method\":\"keyEvent\",\"type\":\"keydown\""
",\"code\":\"KeyA\",\"shift\":true,\"altgr\":false,\"caps\":false}";
to_engine_ptr1->ProcessText(
remote_engine_1->ProcessText(
process_text_key, base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr1.FlushForTesting();
remote_engine_1.FlushForTesting();
to_engine_ptr2->ProcessText(
remote_engine_2->ProcessText(
process_text_key, base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr2.FlushForTesting();
remote_engine_2.FlushForTesting();
std::string process_text_key_count = "{\"method\":\"countKey\"}";
to_engine_ptr1->ProcessText(
remote_engine_1->ProcessText(
process_text_key_count,
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr1.FlushForTesting();
remote_engine_1.FlushForTesting();
EXPECT_EQ("1", response);
to_engine_ptr2->ProcessText(
remote_engine_2->ProcessText(
process_text_key_count,
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr2.FlushForTesting();
remote_engine_2.FlushForTesting();
EXPECT_EQ("1", response);
}
......@@ -153,66 +148,66 @@ TEST_F(ImeServiceTest, MultipleClients) {
TEST_F(ImeServiceTest, RuleBasedArabic) {
bool success = false;
TestClientChannel test_channel;
mojom::InputChannelPtr to_engine_ptr;
mojo::Remote<mojom::InputChannel> remote_engine;
ime_manager_->ConnectToImeEngine("m17n:ar", mojo::MakeRequest(&to_engine_ptr),
test_channel.CreateInterfacePtrAndBind(),
extra,
base::BindOnce(&ConnectCallback, &success));
ime_manager_.FlushForTesting();
remote_manager_->ConnectToImeEngine(
"m17n:ar", remote_engine.BindNewPipeAndPassReceiver(),
test_channel.CreatePendingRemote(), extra,
base::BindOnce(&ConnectCallback, &success));
remote_manager_.FlushForTesting();
EXPECT_TRUE(success);
// Test Shift+KeyA.
std::string response;
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"KeyA\","
"\"shift\":true,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
const wchar_t* expected_response =
L"{\"result\":true,\"operations\":[{\"method\":\"commitText\","
L"\"arguments\":[\"\u0650\"]}]}";
EXPECT_EQ(base::WideToUTF8(expected_response), response);
// Test KeyB.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"KeyB\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
expected_response =
L"{\"result\":true,\"operations\":[{\"method\":\"commitText\","
L"\"arguments\":[\"\u0644\u0627\"]}]}";
EXPECT_EQ(base::WideToUTF8(expected_response), response);
// Test unhandled key.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"Enter\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
EXPECT_EQ("{\"result\":false}", response);
// Test keyup.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keyup\",\"code\":\"Enter\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
EXPECT_EQ("{\"result\":false}", response);
// Test reset.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"reset\"}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
EXPECT_EQ("{\"result\":true}", response);
// Test invalid request.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\"}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
EXPECT_EQ("{\"result\":false}", response);
}
......@@ -220,60 +215,60 @@ TEST_F(ImeServiceTest, RuleBasedArabic) {
TEST_F(ImeServiceTest, RuleBasedDevaPhone) {
bool success = false;
TestClientChannel test_channel;
mojom::InputChannelPtr to_engine_ptr;
mojo::Remote<mojom::InputChannel> remote_engine;
ime_manager_->ConnectToImeEngine(
"m17n:deva_phone", mojo::MakeRequest(&to_engine_ptr),
test_channel.CreateInterfacePtrAndBind(), extra,
remote_manager_->ConnectToImeEngine(
"m17n:deva_phone", remote_engine.BindNewPipeAndPassReceiver(),
test_channel.CreatePendingRemote(), extra,
base::BindOnce(&ConnectCallback, &success));
ime_manager_.FlushForTesting();
remote_manager_.FlushForTesting();
EXPECT_TRUE(success);
std::string response;
// KeyN.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"KeyN\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
const char* expected_response =
u8"{\"result\":true,\"operations\":[{\"method\":\"setComposition\","
u8"\"arguments\":[\"\u0928\"]}]}";
EXPECT_EQ(expected_response, response);
// Backspace.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"Backspace\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
expected_response =
u8"{\"result\":true,\"operations\":[{\"method\":\"setComposition\","
u8"\"arguments\":[\"\"]}]}";
EXPECT_EQ(expected_response, response);
// KeyN + KeyC.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"KeyN\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"KeyC\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
expected_response =
u8"{\"result\":true,\"operations\":[{\"method\":\"setComposition\","
u8"\"arguments\":[\"\u091e\u094d\u091a\"]}]}";
EXPECT_EQ(expected_response, response);
// Space.
to_engine_ptr->ProcessText(
remote_engine->ProcessText(
"{\"method\":\"keyEvent\",\"type\":\"keydown\",\"code\":\"Space\","
"\"shift\":false,\"altgr\":false,\"caps\":false}",
base::BindOnce(&TestProcessTextCallback, &response));
to_engine_ptr.FlushForTesting();
remote_engine.FlushForTesting();
expected_response =
u8"{\"result\":true,\"operations\":[{\"method\":\"commitText\","
u8"\"arguments\":[\"\u091e\u094d\u091a \"]}]}";
......
......@@ -41,15 +41,16 @@ InputEngine::InputEngine() {}
InputEngine::~InputEngine() {}
bool InputEngine::BindRequest(const std::string& ime_spec,
mojom::InputChannelRequest request,
mojom::InputChannelPtr client,
const std::vector<uint8_t>& extra) {
bool InputEngine::BindRequest(
const std::string& ime_spec,
mojo::PendingReceiver<mojom::InputChannel> receiver,
mojo::PendingRemote<mojom::InputChannel> remote,
const std::vector<uint8_t>& extra) {
if (!IsImeSupported(ime_spec))
return false;
channel_bindings_.AddBinding(this, std::move(request),
std::make_unique<InputEngineContext>(ime_spec));
channel_receivers_.Add(this, std::move(receiver),
std::make_unique<InputEngineContext>(ime_spec));
return true;
// TODO(https://crbug.com/837156): Registry connection error handler.
......@@ -61,7 +62,7 @@ bool InputEngine::IsImeSupported(const std::string& ime_spec) {
void InputEngine::ProcessText(const std::string& message,
ProcessTextCallback callback) {
auto& context = channel_bindings_.dispatch_context();
auto& context = channel_receivers_.current_context();
std::string result = Process(message, context.get());
std::move(callback).Run(result);
}
......
......@@ -6,7 +6,9 @@
#define CHROMEOS_SERVICES_IME_INPUT_ENGINE_H_
#include "chromeos/services/ime/public/mojom/input_engine.mojom.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
namespace chromeos {
namespace ime {
......@@ -36,8 +38,8 @@ class InputEngine : public mojom::InputChannel {
// Binds the mojom::InputChannel interface to this object and returns true if
// the given ime_spec is supported by the engine.
virtual bool BindRequest(const std::string& ime_spec,
mojom::InputChannelRequest request,
mojom::InputChannelPtr client,
mojo::PendingReceiver<mojom::InputChannel> receiver,
mojo::PendingRemote<mojom::InputChannel> remote,
const std::vector<uint8_t>& extra);
// Returns whether the given ime_spec is supported by this engine.
......@@ -55,8 +57,8 @@ class InputEngine : public mojom::InputChannel {
std::string Process(const std::string& message,
const InputEngineContext* context);
mojo::BindingSet<mojom::InputChannel, std::unique_ptr<InputEngineContext>>
channel_bindings_;
mojo::ReceiverSet<mojom::InputChannel, std::unique_ptr<InputEngineContext>>
channel_receivers_;
DISALLOW_COPY_AND_ASSIGN(InputEngine);
};
......
......@@ -14,8 +14,8 @@ interface InputEngineManager {
// is implmented on the client and used to receive data sent from the engine.
// On failure (e.g. input engine is not found), |success| is false.
ConnectToImeEngine(string ime_spec,
InputChannel& to_engine_request,
InputChannel from_engine,
pending_receiver<InputChannel> to_engine_request,
pending_remote<InputChannel> from_engine,
array<uint8> extra)
=> (bool success);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment