Commit 33790d39 authored by kinaba's avatar kinaba Committed by Commit bot

ARC: Show virtual keyboard when necessary on text input type change events.

ShowImeIfNeeded() must be explicitly called to bring up the virtual keyboard
when the device is in touch mode.

This is a short-term solution covering only basic use cases. Notably,
it doesn't handle the case when the user dismissed the keyboard manually and
then re-clicked an ARC text field yet.

BUG=581282
BUG=b/25865847

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

Cr-Commit-Position: refs/heads/master@{#371481}
parent 2174bee7
......@@ -22,6 +22,7 @@
'../ui/aura/aura.gyp:aura',
'../ui/base/ime/ui_base_ime.gyp:ui_base_ime',
'../ui/base/ui_base.gyp:ui_base',
'../ui/base/ui_base.gyp:ui_base_test_support',
'../ui/events/events.gyp:events_base',
],
'sources': [
......
......@@ -108,6 +108,7 @@ source_set("unit_tests") {
"//mojo/public/cpp/system:system",
"//testing/gtest",
"//ui/aura",
"//ui/base:test_support",
"//ui/base/ime",
"//ui/events",
"//ui/events:dom_keycode_converter",
......
......@@ -35,7 +35,8 @@ ArcImeBridge::ArcImeBridge(ArcBridgeService* bridge_service)
: ArcService(bridge_service),
ipc_host_(new ArcImeIpcHostImpl(this, bridge_service)),
ime_type_(ui::TEXT_INPUT_TYPE_NONE),
has_composition_text_(false) {
has_composition_text_(false),
test_input_method_(nullptr) {
aura::Env* env = aura::Env::GetInstanceDontCreate();
if (env)
env->AddObserver(this);
......@@ -60,7 +61,14 @@ void ArcImeBridge::SetIpcHostForTesting(
ipc_host_ = std::move(test_ipc_host);
}
void ArcImeBridge::SetInputMethodForTesting(
ui::InputMethod* test_input_method) {
test_input_method_ = test_input_method;
}
ui::InputMethod* ArcImeBridge::GetInputMethod() {
if (test_input_method_)
return test_input_method_;
if (!focused_arc_window_.has_windows())
return nullptr;
return focused_arc_window_.windows().front()->GetHost()->GetInputMethod();
......@@ -114,8 +122,18 @@ void ArcImeBridge::OnTextInputTypeChanged(ui::TextInputType type) {
ime_type_ = type;
ui::InputMethod* const input_method = GetInputMethod();
if (input_method)
if (input_method) {
input_method->OnTextInputTypeChanged(this);
if (input_method->GetTextInputClient() == this &&
ime_type_ != ui::TEXT_INPUT_TYPE_NONE) {
// TODO(kinaba): crbug.com/581282. This is tentative short-term solution.
//
// For fully correct implementation, rather than to piggyback the "show"
// request with the input type change, we need dedicated IPCs to share the
// virtual keyboard show/hide states between Chromium and ARC.
input_method->ShowImeIfNeeded();
}
}
}
void ArcImeBridge::OnCursorRectChanged(const gfx::Rect& rect) {
......
......@@ -45,6 +45,9 @@ class ArcImeBridge : public ArcService,
// Injects the custom IPC host object for testing purpose only.
void SetIpcHostForTesting(scoped_ptr<ArcImeIpcHost> test_ipc_host);
// Injects the custom IME for testing purpose only.
void SetInputMethodForTesting(ui::InputMethod* test_input_method);
// Overridden from aura::EnvObserver:
void OnWindowInitialized(aura::Window* new_window) override;
......@@ -104,6 +107,8 @@ class ArcImeBridge : public ArcService,
aura::WindowTracker arc_windows_;
aura::WindowTracker focused_arc_window_;
ui::InputMethod* test_input_method_;
DISALLOW_COPY_AND_ASSIGN(ArcImeBridge);
};
......
......@@ -11,6 +11,7 @@
#include "components/arc/test/fake_arc_bridge_service.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/ime/composition_text.h"
#include "ui/base/ime/dummy_input_method.h"
namespace arc {
......@@ -26,6 +27,31 @@ class FakeArcImeIpcHost : public ArcImeIpcHost {
}
};
class FakeInputMethod : public ui::DummyInputMethod {
public:
FakeInputMethod() : client_(nullptr), count_show_ime_if_needed_(0) {}
void SetFocusedTextInputClient(ui::TextInputClient* client) override {
client_ = client;
}
ui::TextInputClient* GetTextInputClient() const override {
return client_;
}
void ShowImeIfNeeded() override {
count_show_ime_if_needed_++;
}
int count_show_ime_if_needed() const {
return count_show_ime_if_needed_;
}
private:
ui::TextInputClient* client_;
int count_show_ime_if_needed_;
};
} // namespace
class ArcImeBridgeTest : public testing::Test {
......@@ -34,6 +60,7 @@ class ArcImeBridgeTest : public testing::Test {
protected:
scoped_ptr<FakeArcBridgeService> fake_arc_bridge_service_;
scoped_ptr<FakeInputMethod> fake_input_method_;
scoped_ptr<ArcImeBridge> instance_;
private:
......@@ -41,6 +68,9 @@ class ArcImeBridgeTest : public testing::Test {
fake_arc_bridge_service_.reset(new FakeArcBridgeService);
instance_.reset(new ArcImeBridge(fake_arc_bridge_service_.get()));
instance_->SetIpcHostForTesting(make_scoped_ptr(new FakeArcImeIpcHost));
fake_input_method_.reset(new FakeInputMethod);
instance_->SetInputMethodForTesting(fake_input_method_.get());
}
void TearDown() override {
......@@ -76,4 +106,24 @@ TEST_F(ArcImeBridgeTest, HasCompositionText) {
EXPECT_FALSE(instance_->HasCompositionText());
}
TEST_F(ArcImeBridgeTest, ShowImeIfNeeded) {
fake_input_method_->SetFocusedTextInputClient(instance_.get());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE);
ASSERT_EQ(0, fake_input_method_->count_show_ime_if_needed());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT);
EXPECT_EQ(1, fake_input_method_->count_show_ime_if_needed());
// The type is not changing, hence no call.
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT);
EXPECT_EQ(1, fake_input_method_->count_show_ime_if_needed());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_SEARCH);
EXPECT_EQ(2, fake_input_method_->count_show_ime_if_needed());
// Change to NONE should not trigger the showing event.
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE);
EXPECT_EQ(2, fake_input_method_->count_show_ime_if_needed());
}
} // namespace arc
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