Commit b019381c authored by yusukes's avatar yusukes Committed by Commit Bot

Do not disable ARC IMEs when they are still in the prefs

BUG=879365,845079
TEST=Sign in, enter tablet mode, install gboard, enable gboard,
 use it, enter laptop mode, enter tablet mode again, use gboard,
 confirm the IME confirmation dialog is not shown.

Change-Id: I8eb9ca17433f79fd76e7c471bb6c7929cad68150
Reviewed-on: https://chromium-review.googlesource.com/1199501Reviewed-by: default avatarYuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588419}
parent 5b1b388e
......@@ -301,6 +301,19 @@ void ArcInputMethodManagerService::ImeMenuListChanged() {
return chromeos::extension_ime_util::IsArcIME(id);
});
// TODO(yhanada|yusukes): Instead of observing ImeMenuListChanged(), it's
// probably better to just observe the pref (and not disabling ones still
// in the prefs.) See also the comment below in the second for-loop.
std::set<std::string> active_ime_ids_on_prefs;
{
const std::string active_ime_ids =
profile_->GetPrefs()->GetString(prefs::kLanguageEnabledImes);
std::vector<base::StringPiece> active_ime_list = base::SplitStringPiece(
active_ime_ids, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
for (const auto& id : active_ime_list)
active_ime_ids_on_prefs.insert(id.as_string());
}
for (const auto& id : new_arc_active_ime_ids) {
// Enable the IME which is not currently enabled.
if (!active_arc_ime_ids_.count(id))
......@@ -308,9 +321,22 @@ void ArcInputMethodManagerService::ImeMenuListChanged() {
}
for (const auto& id : active_arc_ime_ids_) {
// Disable the IME which is currently enabled.
if (!new_arc_active_ime_ids.count(id))
if (!new_arc_active_ime_ids.count(id) &&
!active_ime_ids_on_prefs.count(id)) {
// This path is taken in the following two cases:
// 1) The device is in tablet mode, and the user disabled the IME via
// chrome://settings.
// 2) The device was just switched to laptop mode, and this service
// disallowed Android IMEs.
// In the former case, |active_ime_ids_on_prefs| doesn't have the IME,
// but in the latter case, the set still has it. Here, disable the IME
// only for the former case so that the temporary deactivation of the
// IME on laptop mode wouldn't be propagated to the container. Otherwise,
// the IME confirmation dialog will be shown again next time when you
// use the IME in tablet mode.
// TODO(yhanada|yusukes): Only observe the prefs and remove the hack.
EnableIme(id, false /* enable */);
}
}
active_arc_ime_ids_.swap(new_arc_active_ime_ids);
}
......
......@@ -273,12 +273,49 @@ TEST_F(ArcInputMethodManagerServiceTest, EnableIme) {
std::get<std::string>(bridge()->enable_ime_calls_[1]));
EXPECT_FALSE(std::get<bool>(bridge()->enable_ime_calls_[1]));
// EnableIme is not called when non ARC IME is disable.
// EnableIme is not called when non ARC IME is disabled.
imm()->state()->RemoveActiveInputMethodId(extension_ime_id);
service()->ImeMenuListChanged();
EXPECT_EQ(2u, bridge()->enable_ime_calls_.size());
}
TEST_F(ArcInputMethodManagerServiceTest, EnableIme_WithPrefs) {
namespace ceiu = chromeos::extension_ime_util;
using crx_file::id_util::GenerateId;
base::test::ScopedFeatureList feature;
feature.InitAndEnableFeature(kEnableInputMethodFeature);
ToggleTabletMode(true);
ASSERT_EQ(0u, bridge()->enable_ime_calls_.size());
const std::string component_extension_ime_id =
ceiu::GetComponentInputMethodID(
GenerateId("test.component.extension.ime"), "us");
const std::string arc_ime_id =
ceiu::GetArcInputMethodID(GenerateId("test.arc.ime"), "us");
imm()->state()->AddActiveInputMethodId(component_extension_ime_id);
service()->ImeMenuListChanged();
EXPECT_EQ(0u, bridge()->enable_ime_calls_.size());
imm()->state()->AddActiveInputMethodId(arc_ime_id);
service()->ImeMenuListChanged();
ASSERT_EQ(1u, bridge()->enable_ime_calls_.size());
// Test the case where |arc_ime_id| is temporarily disallowed because of the
// toggling to the laptop mode. In that case, the prefs still have the IME's
// ID.
profile()->GetPrefs()->SetString(
prefs::kLanguageEnabledImes,
base::StringPrintf("%s,%s", component_extension_ime_id.c_str(),
arc_ime_id.c_str()));
imm()->state()->RemoveActiveInputMethodId(arc_ime_id);
service()->ImeMenuListChanged();
// Verify that EnableIme(id, false) is NOT called.
EXPECT_EQ(1u, bridge()->enable_ime_calls_.size()); // still 1u, not 2u.
}
TEST_F(ArcInputMethodManagerServiceTest, SwitchImeTo) {
namespace ceiu = chromeos::extension_ime_util;
using crx_file::id_util::GenerateId;
......
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