Commit cb4796d8 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

Revert "Disable Android IMEs according to given ImeInfo."

This reverts commit 2687587b.

Reason for revert: This triggers a use of initialized value on the msan bots. I believe you forgot to initialize is_removing_imm_entry_

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8929963931985091872/+/steps/unit_tests/0/logs/ArcInputMethodManagerServiceTest.EnableIme/0

==14387==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5638655272d9 in arc::ArcInputMethodManagerService::ImeMenuListChanged() ./../../chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.cc:394:7
    #1 0x56385b307d20 in arc::ArcInputMethodManagerServiceTest_EnableIme_Test::TestBody() ./../../chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc:313:14
    #2 0x56385dbb3cb2 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #3 0x56385dbb3cb2 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2522:0
    #4 0x56385dbb7a6b in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2698:11
    #5 0x56385dbb9559 in testing::TestCase::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2816:28
    #6 0x56385dbf2614 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5182:43
    #7 0x56385dbf0ee7 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #8 0x56385dbf0ee7 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:4791:0
    #9 0x56386b81af20 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2333:46
    #10 0x56386b81af20 in base::TestSuite::Run() ./../../base/test/test_suite.cc:294:0
    #11 0x56386b822e8a in Run ./../../base/callback.h:99:12
    #12 0x56386b822e8a in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:225:0
    #13 0x56386b822607 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) ./../../base/test/launcher/unit_test_launcher.cc:575:10
    #14 0x56386b7e9154 in main ./../../chrome/test/base/run_all_unittests.cc:30:10
    #15 0x7fd6f88f4f44 in __libc_start_main ??:0:0
    #16 0x5638521e2b09 in _start ??:0:0

Original change's description:
> Disable Android IMEs according to given ImeInfo.
> 
> Android IMEs can be disabled in Android side by using 'ime' command.
> This CL ensures that disabled IMEs are also disabled in Chrome OS's
> InputMethodManager.
> 
> Bug: b/119274469
> Change-Id: I46c2996a41327221470d69b778da2b7270c73cd2
> Reviewed-on: https://chromium-review.googlesource.com/c/1331294
> Reviewed-by: Yusuke Sato <yusukes@chromium.org>
> Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607473}

TBR=yusukes@chromium.org,yhanada@chromium.org

Change-Id: I5fd93607e711cd90edc4eb0ccf656b4c82ad5555
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/119274469
Reviewed-on: https://chromium-review.googlesource.com/c/1333892Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607610}
parent 4d10954f
...@@ -328,12 +328,10 @@ void ArcInputMethodManagerService::OnImeInfoChanged( ...@@ -328,12 +328,10 @@ void ArcInputMethodManagerService::OnImeInfoChanged(
if (!base::FeatureList::IsEnabled(kEnableInputMethodFeature)) if (!base::FeatureList::IsEnabled(kEnableInputMethodFeature))
return; return;
is_removing_imm_entry_ = true;
scoped_refptr<InputMethodManager::State> state = scoped_refptr<InputMethodManager::State> state =
InputMethodManager::Get()->GetActiveIMEState(); InputMethodManager::Get()->GetActiveIMEState();
// Remove the old registered entry. // Remove the old registered entry.
state->RemoveInputMethodExtension(proxy_ime_extension_id_); state->RemoveInputMethodExtension(proxy_ime_extension_id_);
is_removing_imm_entry_ = false;
// Convert ime_info_array to InputMethodDescriptors. // Convert ime_info_array to InputMethodDescriptors.
InputMethodDescriptors descriptors; InputMethodDescriptors descriptors;
...@@ -365,11 +363,6 @@ void ArcInputMethodManagerService::OnImeInfoChanged( ...@@ -365,11 +363,6 @@ void ArcInputMethodManagerService::OnImeInfoChanged(
if (!base::ContainsValue(active_ime_list, input_method_id)) if (!base::ContainsValue(active_ime_list, input_method_id))
active_ime_list.push_back(input_method_id); active_ime_list.push_back(input_method_id);
} }
// Disable IMEs that are already disable in the container.
base::EraseIf(active_ime_list, [&enabled_input_method_ids](const auto& id) {
return chromeos::extension_ime_util::IsArcIME(id) &&
!base::ContainsValue(enabled_input_method_ids, id);
});
profile_->GetPrefs()->SetString(prefs::kLanguageEnabledImes, profile_->GetPrefs()->SetString(prefs::kLanguageEnabledImes,
base::JoinString(active_ime_list, ",")); base::JoinString(active_ime_list, ","));
...@@ -389,11 +382,6 @@ void ArcInputMethodManagerService::OnConnectionClosed() { ...@@ -389,11 +382,6 @@ void ArcInputMethodManagerService::OnConnectionClosed() {
} }
void ArcInputMethodManagerService::ImeMenuListChanged() { void ArcInputMethodManagerService::ImeMenuListChanged() {
// Ignore ime menu list change while removing the old entry in
// |OnImeInfoChanged| not to expose temporary state to ARC++ container.
if (is_removing_imm_entry_)
return;
auto* manager = chromeos::input_method::InputMethodManager::Get(); auto* manager = chromeos::input_method::InputMethodManager::Get();
auto new_active_ime_ids = auto new_active_ime_ids =
manager->GetActiveIMEState()->GetActiveInputMethodIds(); manager->GetActiveIMEState()->GetActiveInputMethodIds();
......
...@@ -119,7 +119,6 @@ class ArcInputMethodManagerService ...@@ -119,7 +119,6 @@ class ArcInputMethodManagerService
std::unique_ptr<ArcInputMethodManagerBridge> imm_bridge_; std::unique_ptr<ArcInputMethodManagerBridge> imm_bridge_;
std::set<std::string> active_arc_ime_ids_; std::set<std::string> active_arc_ime_ids_;
bool is_virtual_keyboard_shown_; bool is_virtual_keyboard_shown_;
bool is_removing_imm_entry_;
// ArcInputMethodManager installs a proxy IME to redirect IME related events // ArcInputMethodManager installs a proxy IME to redirect IME related events
// from/to ARC IMEs in the container. The below two variables are for the // from/to ARC IMEs in the container. The below two variables are for the
......
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