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

Reland: "Get an incognito flag of text fields of ARC++ apps."

The original change lacks of initialization of new member variable,
so it broke MSan tests.
It's fixed in this CL.

Original change's description:
> Get an incognito flag of text fields of ARC++ apps.
>
> This CL adds a new boolean argument to OnTextInputTypeChanged()
> to send the flag.
>
> Bug: 311180
> Test: Manual testing on the device.
> Change-Id: I130552f4657e0bdd97745fdeffe5e7088ff2583a

Bug: 311180
Test: Manual testing + MSan tests
Change-Id: I920e1e50a6bbd1bb183aa6385cd4701dd926e6f2
Reviewed-on: https://chromium-review.googlesource.com/1086550Reviewed-by: default avatarMattias Nissler <mnissler@chromium.org>
Reviewed-by: default avatarKazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565980}
parent 573e3ebe
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// Next MinVersion: 10 // Next MinVersion: 11
module arc.mojom; module arc.mojom;
...@@ -37,7 +37,9 @@ struct CompositionSegment { ...@@ -37,7 +37,9 @@ struct CompositionSegment {
// Next method ID: 6 // Next method ID: 6
interface ImeHost { interface ImeHost {
// Notifies Chrome that the text input focus is changed. // Notifies Chrome that the text input focus is changed.
OnTextInputTypeChanged@0(TextInputType type); OnTextInputTypeChanged@0(
TextInputType type,
[MinVersion=10] bool is_personalized_learning_allowed);
// Notifies Chrome that the cursor poisition has changed. // Notifies Chrome that the cursor poisition has changed.
// //
......
...@@ -29,7 +29,9 @@ class ArcImeBridge { ...@@ -29,7 +29,9 @@ class ArcImeBridge {
// Received IPCs are deserialized and passed to this delegate. // Received IPCs are deserialized and passed to this delegate.
class Delegate { class Delegate {
public: public:
virtual void OnTextInputTypeChanged(ui::TextInputType type) = 0; virtual void OnTextInputTypeChanged(
ui::TextInputType type,
bool is_personalized_learning_allowed) = 0;
virtual void OnCursorRectChanged(const gfx::Rect& rect, virtual void OnCursorRectChanged(const gfx::Rect& rect,
bool is_screen_cooridnates) = 0; bool is_screen_cooridnates) = 0;
virtual void OnCancelComposition() = 0; virtual void OnCancelComposition() = 0;
......
...@@ -134,8 +134,11 @@ void ArcImeBridgeImpl::SendOnKeyboardAppearanceChanging( ...@@ -134,8 +134,11 @@ void ArcImeBridgeImpl::SendOnKeyboardAppearanceChanging(
ime_instance->OnKeyboardAppearanceChanging(new_bounds, is_available); ime_instance->OnKeyboardAppearanceChanging(new_bounds, is_available);
} }
void ArcImeBridgeImpl::OnTextInputTypeChanged(mojom::TextInputType type) { void ArcImeBridgeImpl::OnTextInputTypeChanged(
delegate_->OnTextInputTypeChanged(ConvertTextInputType(type)); mojom::TextInputType type,
bool is_personalized_learning_allowed) {
delegate_->OnTextInputTypeChanged(ConvertTextInputType(type),
is_personalized_learning_allowed);
} }
void ArcImeBridgeImpl::OnCursorRectChanged(const gfx::Rect& rect, void ArcImeBridgeImpl::OnCursorRectChanged(const gfx::Rect& rect,
......
...@@ -38,7 +38,8 @@ class ArcImeBridgeImpl : public ArcImeBridge, public mojom::ImeHost { ...@@ -38,7 +38,8 @@ class ArcImeBridgeImpl : public ArcImeBridge, public mojom::ImeHost {
bool is_available) override; bool is_available) override;
// mojom::ImeHost overrides: // mojom::ImeHost overrides:
void OnTextInputTypeChanged(mojom::TextInputType type) override; void OnTextInputTypeChanged(mojom::TextInputType type,
bool is_personalized_learning_allowed) override;
void OnCursorRectChanged(const gfx::Rect& rect, void OnCursorRectChanged(const gfx::Rect& rect,
bool screen_coordinates) override; bool screen_coordinates) override;
void OnCancelComposition() override; void OnCancelComposition() override;
......
...@@ -139,6 +139,7 @@ ArcImeService::ArcImeService(content::BrowserContext* context, ...@@ -139,6 +139,7 @@ ArcImeService::ArcImeService(content::BrowserContext* context,
: ime_bridge_(new ArcImeBridgeImpl(this, bridge_service)), : ime_bridge_(new ArcImeBridgeImpl(this, bridge_service)),
arc_window_delegate_(new ArcWindowDelegateImpl(this)), arc_window_delegate_(new ArcWindowDelegateImpl(this)),
ime_type_(ui::TEXT_INPUT_TYPE_NONE), ime_type_(ui::TEXT_INPUT_TYPE_NONE),
is_personalized_learning_allowed_(false),
has_composition_text_(false), has_composition_text_(false),
is_focus_observer_installed_(false) { is_focus_observer_installed_(false) {
aura::Env* env = aura::Env::GetInstanceDontCreate(); aura::Env* env = aura::Env::GetInstanceDontCreate();
...@@ -268,10 +269,15 @@ void ArcImeService::OnWindowFocused(aura::Window* gained_focus, ...@@ -268,10 +269,15 @@ void ArcImeService::OnWindowFocused(aura::Window* gained_focus,
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// Overridden from arc::ArcImeBridge::Delegate // Overridden from arc::ArcImeBridge::Delegate
void ArcImeService::OnTextInputTypeChanged(ui::TextInputType type) { void ArcImeService::OnTextInputTypeChanged(
if (ime_type_ == type) ui::TextInputType type,
bool is_personalized_learning_allowed) {
if (ime_type_ == type &&
is_personalized_learning_allowed_ == is_personalized_learning_allowed) {
return; return;
}
ime_type_ = type; ime_type_ = type;
is_personalized_learning_allowed_ = is_personalized_learning_allowed;
ui::InputMethod* const input_method = GetInputMethod(); ui::InputMethod* const input_method = GetInputMethod();
if (input_method) if (input_method)
...@@ -524,9 +530,7 @@ const std::string& ArcImeService::GetClientSourceInfo() const { ...@@ -524,9 +530,7 @@ const std::string& ArcImeService::GetClientSourceInfo() const {
} }
bool ArcImeService::ShouldDoLearning() { bool ArcImeService::ShouldDoLearning() {
// TODO(https://crbug.com/311180): Implement this method. return is_personalized_learning_allowed_;
NOTIMPLEMENTED_LOG_ONCE();
return true;
} }
// static // static
......
...@@ -84,7 +84,8 @@ class ArcImeService : public KeyedService, ...@@ -84,7 +84,8 @@ class ArcImeService : public KeyedService,
aura::Window* lost_focus) override; aura::Window* lost_focus) override;
// Overridden from ArcImeBridge::Delegate: // Overridden from ArcImeBridge::Delegate:
void OnTextInputTypeChanged(ui::TextInputType type) override; void OnTextInputTypeChanged(ui::TextInputType type,
bool is_personalized_learning_allowed) override;
void OnCursorRectChanged(const gfx::Rect& rect, void OnCursorRectChanged(const gfx::Rect& rect,
bool is_screen_coordinates) override; bool is_screen_coordinates) override;
void OnCancelComposition() override; void OnCancelComposition() override;
...@@ -162,6 +163,7 @@ class ArcImeService : public KeyedService, ...@@ -162,6 +163,7 @@ class ArcImeService : public KeyedService,
std::unique_ptr<ArcImeBridge> ime_bridge_; std::unique_ptr<ArcImeBridge> ime_bridge_;
std::unique_ptr<ArcWindowDelegate> arc_window_delegate_; std::unique_ptr<ArcWindowDelegate> arc_window_delegate_;
ui::TextInputType ime_type_; ui::TextInputType ime_type_;
bool is_personalized_learning_allowed_;
gfx::Rect cursor_rect_; gfx::Rect cursor_rect_;
bool has_composition_text_; bool has_composition_text_;
gfx::Range text_range_; gfx::Range text_range_;
......
...@@ -61,10 +61,12 @@ class FakeArcImeBridge : public ArcImeBridge { ...@@ -61,10 +61,12 @@ class FakeArcImeBridge : public ArcImeBridge {
class FakeInputMethod : public ui::DummyInputMethod { class FakeInputMethod : public ui::DummyInputMethod {
public: public:
FakeInputMethod() : client_(nullptr), FakeInputMethod()
count_show_ime_if_needed_(0), : client_(nullptr),
count_cancel_composition_(0), count_show_ime_if_needed_(0),
count_set_focused_text_input_client_(0) {} count_cancel_composition_(0),
count_set_focused_text_input_client_(0),
count_on_text_input_type_changed_(0) {}
void SetFocusedTextInputClient(ui::TextInputClient* client) override { void SetFocusedTextInputClient(ui::TextInputClient* client) override {
count_set_focused_text_input_client_++; count_set_focused_text_input_client_++;
...@@ -89,6 +91,10 @@ class FakeInputMethod : public ui::DummyInputMethod { ...@@ -89,6 +91,10 @@ class FakeInputMethod : public ui::DummyInputMethod {
client_ = nullptr; client_ = nullptr;
} }
void OnTextInputTypeChanged(const ui::TextInputClient* client) override {
count_on_text_input_type_changed_++;
}
int count_show_ime_if_needed() const { int count_show_ime_if_needed() const {
return count_show_ime_if_needed_; return count_show_ime_if_needed_;
} }
...@@ -101,11 +107,16 @@ class FakeInputMethod : public ui::DummyInputMethod { ...@@ -101,11 +107,16 @@ class FakeInputMethod : public ui::DummyInputMethod {
return count_set_focused_text_input_client_; return count_set_focused_text_input_client_;
} }
int count_on_text_input_type_changed() const {
return count_on_text_input_type_changed_;
}
private: private:
ui::TextInputClient* client_; ui::TextInputClient* client_;
int count_show_ime_if_needed_; int count_show_ime_if_needed_;
int count_cancel_composition_; int count_cancel_composition_;
int count_set_focused_text_input_client_; int count_set_focused_text_input_client_;
int count_on_text_input_type_changed_;
}; };
// Helper class for testing the window focus tracking feature of ArcImeService, // Helper class for testing the window focus tracking feature of ArcImeService,
...@@ -227,11 +238,11 @@ TEST_F(ArcImeServiceTest, HasCompositionText) { ...@@ -227,11 +238,11 @@ TEST_F(ArcImeServiceTest, HasCompositionText) {
TEST_F(ArcImeServiceTest, ShowImeIfNeeded) { TEST_F(ArcImeServiceTest, ShowImeIfNeeded) {
instance_->OnWindowFocused(arc_win_.get(), nullptr); instance_->OnWindowFocused(arc_win_.get(), nullptr);
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE); instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE, false);
ASSERT_EQ(0, fake_input_method_->count_show_ime_if_needed()); ASSERT_EQ(0, fake_input_method_->count_show_ime_if_needed());
// Text input type change does not imply the show ime request. // Text input type change does not imply the show ime request.
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT); instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT, true);
EXPECT_EQ(0, fake_input_method_->count_show_ime_if_needed()); EXPECT_EQ(0, fake_input_method_->count_show_ime_if_needed());
instance_->ShowImeIfNeeded(); instance_->ShowImeIfNeeded();
...@@ -250,12 +261,12 @@ TEST_F(ArcImeServiceTest, InsertChar) { ...@@ -250,12 +261,12 @@ TEST_F(ArcImeServiceTest, InsertChar) {
instance_->OnWindowFocused(arc_win_.get(), nullptr); instance_->OnWindowFocused(arc_win_.get(), nullptr);
// When text input type is NONE, the event is not forwarded. // When text input type is NONE, the event is not forwarded.
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE); instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE, false);
instance_->InsertChar(ui::KeyEvent('a', ui::VKEY_A, 0)); instance_->InsertChar(ui::KeyEvent('a', ui::VKEY_A, 0));
EXPECT_EQ(0, fake_arc_ime_bridge_->count_send_insert_text()); EXPECT_EQ(0, fake_arc_ime_bridge_->count_send_insert_text());
// When the bridge is accepting text inputs, forward the event. // When the bridge is accepting text inputs, forward the event.
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT); instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT, true);
instance_->InsertChar(ui::KeyEvent('a', ui::VKEY_A, 0)); instance_->InsertChar(ui::KeyEvent('a', ui::VKEY_A, 0));
EXPECT_EQ(1, fake_arc_ime_bridge_->count_send_insert_text()); EXPECT_EQ(1, fake_arc_ime_bridge_->count_send_insert_text());
} }
...@@ -390,4 +401,21 @@ TEST_F(ArcImeServiceTest, GetCaretBounds) { ...@@ -390,4 +401,21 @@ TEST_F(ArcImeServiceTest, GetCaretBounds) {
instance_->GetCaretBounds()); instance_->GetCaretBounds());
} }
TEST_F(ArcImeServiceTest, ShouldDoLearning) {
instance_->OnWindowFocused(arc_win_.get(), nullptr);
ASSERT_NE(ui::TEXT_INPUT_TYPE_TEXT, instance_->GetTextInputType());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT, true);
EXPECT_TRUE(instance_->ShouldDoLearning());
EXPECT_EQ(1, fake_input_method_->count_on_text_input_type_changed());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_TEXT, false);
EXPECT_FALSE(instance_->ShouldDoLearning());
EXPECT_EQ(2, fake_input_method_->count_on_text_input_type_changed());
instance_->OnTextInputTypeChanged(ui::TEXT_INPUT_TYPE_URL, false);
EXPECT_FALSE(instance_->ShouldDoLearning());
EXPECT_EQ(3, fake_input_method_->count_on_text_input_type_changed());
}
} // namespace arc } // 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