Commit b52d57e9 authored by Yuichiro Hanada's avatar Yuichiro Hanada Committed by Commit Bot

Remove |composing_text_| field from InputConnectionImpl.

Android side should wait for completion of all operations, so there is
no need to cache the value of composing text.

Test: unit_tests --gtest_filter=InputConnectionImpl.*
Bug: b/118364941
Change-Id: I87beee3ba6b39618f705aa734974a460a3f9c214
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1681624
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: default avatarTetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673285}
parent c87d16d4
......@@ -65,7 +65,6 @@ InputConnectionImpl::InputConnectionImpl(
imm_bridge_(imm_bridge),
input_context_id_(input_context_id),
binding_(this),
composing_text_(),
state_update_timer_() {}
InputConnectionImpl::~InputConnectionImpl() = default;
......@@ -130,7 +129,6 @@ void InputConnectionImpl::CommitText(const base::string16& text,
if (!ime_engine_->CommitText(input_context_id_,
base::UTF16ToUTF8(text).c_str(), &error))
LOG(ERROR) << "CommitText failed: error=\"" << error << "\"";
composing_text_.clear();
}
void InputConnectionImpl::DeleteSurroundingText(int before, int after) {
......@@ -159,12 +157,6 @@ void InputConnectionImpl::DeleteSurroundingText(int before, int after) {
void InputConnectionImpl::FinishComposingText() {
StartStateUpdateTimer();
if (composing_text_.empty()) {
// There is no ongoing composing. Do nothing.
UpdateTextInputState(true);
return;
}
ui::TextInputClient* client = GetTextInputClient();
if (!client)
return;
......@@ -172,14 +164,22 @@ void InputConnectionImpl::FinishComposingText() {
client->GetEditableSelectionRange(&selection_range);
client->GetCompositionTextRange(&composition_range);
if (composition_range.is_empty()) {
// There is no ongoing composing. Do nothing.
UpdateTextInputState(true);
return;
}
base::string16 composing_text;
client->GetTextFromRange(composition_range, &composing_text);
std::string error;
if (!ime_engine_->CommitText(input_context_id_,
base::UTF16ToUTF8(composing_text_).c_str(),
base::UTF16ToUTF8(composing_text).c_str(),
&error)) {
LOG(ERROR) << "FinishComposingText: CommitText() failed, error=\"" << error
<< "\"";
}
composing_text_.clear();
if (selection_range.start() == selection_range.end() &&
selection_range.start() == composition_range.end()) {
......@@ -229,7 +229,6 @@ void InputConnectionImpl::SetComposingText(
<< ", error=\"" << error << "\"";
return;
}
composing_text_ = text;
}
void InputConnectionImpl::RequestTextInputState(
......
......@@ -65,8 +65,6 @@ class InputConnectionImpl : public mojom::InputConnection {
mojo::Binding<mojom::InputConnection> binding_;
base::string16 composing_text_;
base::OneShotTimer state_update_timer_;
DISALLOW_COPY_AND_ASSIGN(InputConnectionImpl);
......
......@@ -104,6 +104,43 @@ class TestIMEInputContextHandler : public ui::MockIMEInputContextHandler {
DISALLOW_COPY_AND_ASSIGN(TestIMEInputContextHandler);
};
class MockTextInputClient : public ui::DummyTextInputClient {
public:
void SetText(const std::string& text) { text_ = text; }
void SetCursorPos(int pos) { cursor_pos_ = pos; }
void SetCompositionRange(const gfx::Range& range) {
composition_range_ = range;
}
bool GetTextRange(gfx::Range* range) const override {
*range = gfx::Range(0, base::ASCIIToUTF16(text_).length());
return true;
}
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override {
*text = base::ASCIIToUTF16(text_.substr(range.start(), range.end()));
return true;
}
bool GetEditableSelectionRange(gfx::Range* range) const override {
*range = gfx::Range(cursor_pos_, cursor_pos_);
return true;
}
bool GetCompositionTextRange(gfx::Range* range) const override {
*range = composition_range_;
return true;
}
private:
std::string text_;
int cursor_pos_ = 0;
gfx::Range composition_range_ = gfx::Range(0, 0);
};
class InputConnectionImplTest : public testing::Test {
public:
InputConnectionImplTest() = default;
......@@ -118,7 +155,7 @@ class InputConnectionImplTest : public testing::Test {
TestIMEInputContextHandler* context_handler() { return &context_handler_; }
ui::DummyTextInputClient* client() { return &text_input_client_; }
MockTextInputClient* client() { return &text_input_client_; }
ui::IMEEngineHandlerInterface::InputContext context() {
return ui::IMEEngineHandlerInterface::InputContext{
......@@ -160,7 +197,7 @@ class InputConnectionImplTest : public testing::Test {
base::test::ScopedTaskEnvironment scoped_task_environment_;
std::unique_ptr<TestInputMethodManagerBridge> bridge_;
std::unique_ptr<chromeos::InputMethodEngine> engine_;
ui::DummyTextInputClient text_input_client_;
MockTextInputClient text_input_client_;
ui::MockInputMethod input_method_{nullptr};
TestIMEInputContextHandler context_handler_{&input_method_};
std::unique_ptr<ChromeKeyboardControllerClientTestHelper>
......@@ -218,11 +255,17 @@ TEST_F(InputConnectionImplTest, FinishComposingText) {
context_handler()->Reset();
connection->SetComposingText(base::ASCIIToUTF16("composing"), 0,
base::nullopt);
client()->SetText("composing");
client()->SetCompositionRange(gfx::Range(0, 9));
EXPECT_EQ(0, context_handler()->commit_text_call_count());
connection->FinishComposingText();
EXPECT_EQ(1, context_handler()->commit_text_call_count());
EXPECT_EQ("composing", context_handler()->last_commit_text());
client()->SetCompositionRange(gfx::Range(0, 0));
connection->FinishComposingText();
EXPECT_EQ(1, context_handler()->commit_text_call_count());
engine()->FocusOut();
}
......
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