AccessibilityCommon extension is an IME when Dictation is activated.

Dictation adds AccessibilityCommon extension as an IME when Dictation is activated, and removes itself when Dictation is deactivated. Design: go/cros-dictation-extension Test: dictation_test.js, manual testing Bug: 1216111 Change-Id: Ic6bd9d53bda5bb3e863153cb7ec3c4c4274460e7 AX-Relnotes: N/A Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2945333 Reviewed-by: dpapad <dpapad@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: My Nguyen <myy@chromium.org> Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org> Reviewed-by: Akihiro Ota <akihiroota@chromium.org> Commit-Queue: Katie Dektar <katie@chromium.org> Cr-Commit-Position: refs/heads/master@{#893100}
FairyWorld · Jun 16, 2021 · 9a44cc6 · 9a44cc6
1 parent 1518adb
commit 9a44cc6
Show file tree

Hide file tree

Showing 24 changed files with 678 additions and 37 deletions.
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
@@ -143,13 +143,27 @@ void AddTestImes() {
   ime1.id = "id1";
   ImeInfo ime2;
   ime2.id = "id2";
-  std::vector<ImeInfo> available_imes;
-  available_imes.push_back(std::move(ime1));
-  available_imes.push_back(std::move(ime2));
-  Shell::Get()->ime_controller()->RefreshIme("id1", std::move(available_imes),
+  std::vector<ImeInfo> visible_imes;
+  visible_imes.push_back(std::move(ime1));
+  visible_imes.push_back(std::move(ime2));
+  Shell::Get()->ime_controller()->RefreshIme("id1", std::move(visible_imes),
                                              std::vector<ImeMenuItem>());
 }
 
+void AddNotVisibleTestIme() {
+  ImeInfo dictation;
+  dictation.id = "_ext_ime_egfdjlfmgnehecnclamagfafdccgfndpdictation";
+  const std::vector<ImeInfo> visible_imes =
+      Shell::Get()->ime_controller()->GetVisibleImes();
+  std::vector<ImeInfo> available_imes;
+  for (auto ime : visible_imes) {
+    available_imes.push_back(ime);
+  }
+  available_imes.push_back(dictation);
+  Shell::Get()->ime_controller()->RefreshIme(
+      dictation.id, std::move(available_imes), std::vector<ImeMenuItem>());
+}
+
 ui::Accelerator CreateReleaseAccelerator(ui::KeyboardCode key_code,
                                          int modifiers) {
   ui::Accelerator accelerator(key_code, modifiers);
@@ -1426,7 +1440,7 @@ TEST_F(AcceleratorControllerTest, GlobalAcceleratorsToggleAppListFullscreen) {
 }
 
 TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) {
-  ASSERT_EQ(0u, Shell::Get()->ime_controller()->available_imes().size());
+  ASSERT_EQ(0u, Shell::Get()->ime_controller()->GetVisibleImes().size());
 
   // Cycling IME is blocked because there is nothing to switch to.
   ui::Accelerator control_space_down(ui::VKEY_SPACE, ui::EF_CONTROL_DOWN);
@@ -1438,11 +1452,24 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) {
   EXPECT_FALSE(ProcessInController(control_space_up));
   EXPECT_FALSE(ProcessInController(control_shift_space));
 
+  // Adding only a not visible IME doesn't make IME accelerators available.
+  AddNotVisibleTestIme();
+  ASSERT_EQ(0u, Shell::Get()->ime_controller()->GetVisibleImes().size());
+  EXPECT_FALSE(ProcessInController(control_space_down));
+  EXPECT_FALSE(ProcessInController(control_space_up));
+  EXPECT_FALSE(ProcessInController(control_shift_space));
+
   // Cycling IME works when there are IMEs available.
   AddTestImes();
   EXPECT_TRUE(ProcessInController(control_space_down));
   EXPECT_TRUE(ProcessInController(control_space_up));
   EXPECT_TRUE(ProcessInController(control_shift_space));
+
+  // Adding the not visible IME back doesn't block cycling.
+  AddNotVisibleTestIme();
+  EXPECT_TRUE(ProcessInController(control_space_down));
+  EXPECT_TRUE(ProcessInController(control_space_up));
+  EXPECT_TRUE(ProcessInController(control_shift_space));
 }
 
 // TODO(nona|mazda): Remove this when crbug.com/139556 in a better way.

diff --git a/ash/ime/ime_controller_impl.cc b/ash/ime/ime_controller_impl.cc
@@ -31,6 +31,10 @@ enum class ModeChangeKeyAction {
   kMaxValue = kSwitchIme
 };
 
+// The ID for the Accessibility Common IME (used for Dictation).
+const char* kAccessibilityCommonIMEId =
+    "_ext_ime_egfdjlfmgnehecnclamagfafdccgfndpdictation";
+
 }  // namespace
 
 ImeControllerImpl::ImeControllerImpl()
@@ -48,6 +52,14 @@ void ImeControllerImpl::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+const std::vector<ImeInfo>& ImeControllerImpl::GetVisibleImes() const {
+  return visible_imes_;
+}
+
+bool ImeControllerImpl::IsCurrentImeVisible() const {
+  return current_ime_.id != kAccessibilityCommonIMEId;
+}
+
 void ImeControllerImpl::SetClient(ImeControllerClient* client) {
   if (client_) {
     if (CastConfigController::Get())
@@ -71,7 +83,7 @@ bool ImeControllerImpl::CanSwitchIme() const {
 
   // Do not consume key event if there is only one input method is enabled.
   // Ctrl+Space or Alt+Shift may be used by other application.
-  return available_imes_.size() > 1;
+  return GetVisibleImes().size() > 1;
 }
 
 void ImeControllerImpl::SwitchToNextIme() {
@@ -131,12 +143,17 @@ void ImeControllerImpl::RefreshIme(const std::string& current_ime_id,
 
   available_imes_.clear();
   available_imes_.reserve(available_imes.size());
+  visible_imes_.clear();
+  visible_imes_.reserve(visible_imes_.size());
   for (const auto& ime : available_imes) {
     if (ime.id.empty()) {
       DLOG(ERROR) << "Received IME with invalid ID.";
       continue;
     }
     available_imes_.push_back(ime);
+    if (ime.id != kAccessibilityCommonIMEId) {
+      visible_imes_.push_back(ime);
+    }
     if (ime.id == current_ime_id)
       current_ime_ = ime;
   }

diff --git a/ash/ime/ime_controller_impl.h b/ash/ime/ime_controller_impl.h
@@ -52,9 +52,10 @@ class ASH_EXPORT ImeControllerImpl : public ImeController,
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  const ImeInfo& current_ime() const { return current_ime_; }
+  const std::vector<ImeInfo>& GetVisibleImes() const;
+  bool IsCurrentImeVisible() const;
 
-  const std::vector<ImeInfo>& available_imes() const { return available_imes_; }
+  const ImeInfo& current_ime() const { return current_ime_; }
 
   bool is_extra_input_options_enabled() const {
     return is_extra_input_options_enabled_;
@@ -147,6 +148,10 @@ class ASH_EXPORT ImeControllerImpl : public ImeController,
   // "Available" IMEs are both installed and enabled by the user in settings.
   std::vector<ImeInfo> available_imes_;
 
+  // "Visible" IMEs are installed, enabled, and don't include built-in IMEs that
+  // shouldn't be shown to the user, like Dictation.
+  std::vector<ImeInfo> visible_imes_;
+
   // True if the available IMEs are currently managed by enterprise policy.
   // For example, can occur at the login screen with device-level policy.
   bool managed_by_policy_ = false;

diff --git a/ash/ime/ime_controller_impl_unittest.cc b/ash/ime/ime_controller_impl_unittest.cc
@@ -102,9 +102,9 @@ TEST_F(ImeControllerImplTest, RefreshIme) {
 
   // Cached data was updated.
   EXPECT_EQ("ime1", controller->current_ime().id);
-  ASSERT_EQ(2u, controller->available_imes().size());
-  EXPECT_EQ("ime1", controller->available_imes()[0].id);
-  EXPECT_EQ("ime2", controller->available_imes()[1].id);
+  ASSERT_EQ(2u, controller->GetVisibleImes().size());
+  EXPECT_EQ("ime1", controller->GetVisibleImes()[0].id);
+  EXPECT_EQ("ime2", controller->GetVisibleImes()[1].id);
   ASSERT_EQ(1u, controller->current_ime_menu_items().size());
   EXPECT_EQ("menu1", controller->current_ime_menu_items()[0].key);
 
@@ -118,13 +118,38 @@ TEST_F(ImeControllerImplTest, NoCurrentIme) {
   // Set up a single IME.
   RefreshImes("ime1", {"ime1"});
   EXPECT_EQ("ime1", controller->current_ime().id);
+  EXPECT_TRUE(controller->IsCurrentImeVisible());
 
   // When there is no current IME the cached current IME is empty.
   const std::string empty_ime_id;
   RefreshImes(empty_ime_id, {"ime1"});
   EXPECT_TRUE(controller->current_ime().id.empty());
 }
 
+TEST_F(ImeControllerImplTest, CurrentImeNotVisible) {
+  ImeControllerImpl* controller = Shell::Get()->ime_controller();
+
+  // Add only Dictation.
+  std::string dictation_id =
+      "_ext_ime_egfdjlfmgnehecnclamagfafdccgfndpdictation";
+  RefreshImes(dictation_id, {dictation_id});
+  EXPECT_EQ(dictation_id, controller->current_ime().id);
+  EXPECT_FALSE(controller->IsCurrentImeVisible());
+  EXPECT_EQ(0u, controller->GetVisibleImes().size());
+
+  // Add something else too, but Dictation is active.
+  RefreshImes(dictation_id, {dictation_id, "ime1"});
+  EXPECT_EQ(dictation_id, controller->current_ime().id);
+  EXPECT_FALSE(controller->IsCurrentImeVisible());
+  EXPECT_EQ(1u, controller->GetVisibleImes().size());
+
+  // Inactivate the other IME, leave Dictation in the list.
+  RefreshImes("ime1", {dictation_id, "ime1"});
+  EXPECT_EQ("ime1", controller->current_ime().id);
+  EXPECT_TRUE(controller->IsCurrentImeVisible());
+  EXPECT_EQ(1u, controller->GetVisibleImes().size());
+}
+
 TEST_F(ImeControllerImplTest, SetImesManagedByPolicy) {
   ImeControllerImpl* controller = Shell::Get()->ime_controller();
   TestImeObserver observer;
@@ -152,7 +177,7 @@ TEST_F(ImeControllerImplTest, CanSwitchIme) {
   ImeControllerImpl* controller = Shell::Get()->ime_controller();
 
   // Can't switch IMEs when none are available.
-  ASSERT_EQ(0u, controller->available_imes().size());
+  ASSERT_EQ(0u, controller->GetVisibleImes().size());
   EXPECT_FALSE(controller->CanSwitchIme());
 
   // Can't switch with only 1 IME.
@@ -208,7 +233,7 @@ TEST_F(ImeControllerImplTest, SwitchImeWithAccelerator) {
   const ui::Accelerator wide_half_2(ui::VKEY_DBE_DBCSCHAR, ui::EF_NONE);
 
   // When there are no IMEs available switching by accelerator does not work.
-  ASSERT_EQ(0u, controller->available_imes().size());
+  ASSERT_EQ(0u, controller->GetVisibleImes().size());
   EXPECT_FALSE(controller->CanSwitchImeWithAccelerator(convert));
   EXPECT_FALSE(controller->CanSwitchImeWithAccelerator(non_convert));
   EXPECT_FALSE(controller->CanSwitchImeWithAccelerator(wide_half_1));

diff --git a/ash/login/ui/lock_contents_view.cc b/ash/login/ui/lock_contents_view.cc
@@ -2131,7 +2131,7 @@ void LockContentsView::ShowAuthErrorMessage() {
   int bold_length = 0;
   // Display a hint to switch keyboards if there are other active input
   // methods in clamshell mode.
-  if (ime_controller->available_imes().size() > 1 && !IsTabletMode()) {
+  if (ime_controller->GetVisibleImes().size() > 1 && !IsTabletMode()) {
     error_text += u" ";
     bold_start = error_text.length();
     std::u16string shortcut =

diff --git a/ash/system/ime/ime_feature_pod_controller.cc b/ash/system/ime/ime_feature_pod_controller.cc
@@ -21,15 +21,15 @@ namespace {
 bool IsButtonVisible() {
   DCHECK(Shell::Get());
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
-  size_t ime_count = ime_controller->available_imes().size();
+  size_t ime_count = ime_controller->GetVisibleImes().size();
   return !ime_controller->is_menu_active() &&
          (ime_count > 1 || ime_controller->managed_by_policy());
 }
 
 std::u16string GetLabelString() {
   DCHECK(Shell::Get());
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
-  size_t ime_count = ime_controller->available_imes().size();
+  size_t ime_count = ime_controller->GetVisibleImes().size();
   if (ime_count > 1) {
     return ime_controller->current_ime().short_name;
   } else {
@@ -42,7 +42,7 @@ std::u16string GetLabelString() {
 std::u16string GetTooltipString() {
   DCHECK(Shell::Get());
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
-  size_t ime_count = ime_controller->available_imes().size();
+  size_t ime_count = ime_controller->GetVisibleImes().size();
   if (ime_count > 1) {
     return l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_IME_TOOLTIP_WITH_NAME,
                                       ime_controller->current_ime().name);

diff --git a/ash/system/ime/unified_ime_detailed_view_controller.cc b/ash/system/ime/unified_ime_detailed_view_controller.cc
@@ -75,7 +75,7 @@ void UnifiedIMEDetailedViewController::OnIMEMenuActivationChanged(
 void UnifiedIMEDetailedViewController::Update() {
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
   view_->Update(ime_controller->current_ime().id,
-                ime_controller->available_imes(),
+                ime_controller->GetVisibleImes(),
                 ime_controller->current_ime_menu_items(),
                 ShouldShowKeyboardToggle(), GetSingleImeBehavior());
 }

diff --git a/ash/system/ime_menu/ime_list_view.cc b/ash/system/ime_menu/ime_list_view.cc
@@ -209,7 +209,7 @@ ImeListView::~ImeListView() = default;
 void ImeListView::Init(bool show_keyboard_toggle,
                        SingleImeBehavior single_ime_behavior) {
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
-  Update(ime_controller->current_ime().id, ime_controller->available_imes(),
+  Update(ime_controller->current_ime().id, ime_controller->GetVisibleImes(),
          ime_controller->current_ime_menu_items(), show_keyboard_toggle,
          single_ime_behavior);
 }

diff --git a/ash/system/ime_menu/ime_menu_tray.cc b/ash/system/ime_menu/ime_menu_tray.cc
@@ -486,7 +486,7 @@ void ImeMenuTray::OnIMERefresh() {
   UpdateTrayLabel();
   if (bubble_ && ime_list_view_) {
     ime_list_view_->Update(
-        ime_controller_->current_ime().id, ime_controller_->available_imes(),
+        ime_controller_->current_ime().id, ime_controller_->GetVisibleImes(),
         ime_controller_->current_ime_menu_items(), ShouldShowKeyboardToggle(),
         ImeListView::SHOW_SINGLE_IME);
   }

diff --git a/ash/system/ime_menu/ime_menu_tray_unittest.cc b/ash/system/ime_menu/ime_menu_tray_unittest.cc
@@ -164,6 +164,39 @@ TEST_F(ImeMenuTrayTest, TrayLabelTest) {
   EXPECT_EQ(u"UK*", GetTrayText());
 }
 
+TEST_F(ImeMenuTrayTest, TrayLabelExludesDictation) {
+  Shell::Get()->ime_controller()->ShowImeMenuOnShelf(true);
+  ASSERT_TRUE(IsVisible());
+
+  ImeInfo info1;
+  info1.id = "ime1";
+  info1.name = u"English";
+  info1.short_name = u"US";
+  info1.third_party = false;
+
+  ImeInfo info2;
+  info2.id = "ime2";
+  info2.name = u"English UK";
+  info2.short_name = u"UK";
+  info2.third_party = true;
+
+  ImeInfo dictation;
+  dictation.id = "_ext_ime_egfdjlfmgnehecnclamagfafdccgfndpdictation";
+  dictation.name = u"Dictation";
+
+  // Changes the input method to "ime1".
+  SetCurrentIme("ime1", {info1, dictation, info2});
+  EXPECT_EQ(u"US", GetTrayText());
+
+  // Changes the input method to a third-party IME extension.
+  SetCurrentIme("ime2", {info1, dictation, info2});
+  EXPECT_EQ(u"UK*", GetTrayText());
+
+  // Sets to "dictation", which shouldn't be shown.
+  SetCurrentIme(dictation.id, {info1, dictation, info2});
+  EXPECT_EQ(u"", GetTrayText());
+}
+
 // Tests that IME menu tray changes background color when tapped/clicked. And
 // tests that the background color becomes 'inactive' when disabling the IME
 // menu feature. Also makes sure that the shelf won't autohide as long as the

diff --git a/ash/system/unified/ime_mode_view.cc b/ash/system/unified/ime_mode_view.cc
@@ -92,7 +92,12 @@ void ImeModeView::Update() {
 
   ImeControllerImpl* ime_controller = Shell::Get()->ime_controller();
 
-  size_t ime_count = ime_controller->available_imes().size();
+  if (!ime_controller->IsCurrentImeVisible()) {
+    SetVisible(false);
+    return;
+  }
+
+  size_t ime_count = ime_controller->GetVisibleImes().size();
   SetVisible(!ime_menu_on_shelf_activated_ &&
              (ime_count > 1 || ime_controller->managed_by_policy()));
 

diff --git a/chrome/browser/media/extension_media_access_handler.cc b/chrome/browser/media/extension_media_access_handler.cc
@@ -26,15 +26,17 @@ namespace {
 // 5. Hotwording component extension.
 // 6. XKB input method component extension.
 // 7. M17n/T13n/CJK input method component extension.
-// Once http://crbug.com/292856 is fixed, remove this whitelist.
+// 8. Accessibility Common extension (used for Dictation)
+// Once http://crbug.com/292856 is fixed, remove this allowlist.
 bool IsMediaRequestAllowedForExtension(const extensions::Extension* extension) {
   return extension->id() == "mppnpdlheglhdfmldimlhpnegondlapf" ||
          extension->id() == "jokbpnebhdcladagohdnfgjcpejggllo" ||
          extension->id() == "clffjmdilanldobdnedchkdbofoimcgb" ||
          extension->id() == "nnckehldicaciogcbchegobnafnjkcne" ||
          extension->id() == "nbpagnldghgfoolbancepceaanlmhfmd" ||
          extension->id() == "jkghodnilhceideoidjikpgommlajknk" ||
-         extension->id() == "gjaehgfemfahhmlgpdfknkhdnemmolop";
+         extension->id() == "gjaehgfemfahhmlgpdfknkhdnemmolop" ||
+         extension->id() == "egfdjlfmgnehecnclamagfafdccgfndp";
 }
 
 }  // namespace

diff --git a/chrome/browser/resources/chromeos/accessibility/accessibility_common/BUILD.gn b/chrome/browser/resources/chromeos/accessibility/accessibility_common/BUILD.gn
@@ -71,6 +71,9 @@ js2gtest("accessibility_common_extjs_tests") {
     "../common/testing/callback_helper.js",
     "../common/testing/e2e_test_base.js",
     "../common/testing/mock_accessibility_private.js",
+    "../common/testing/mock_input_ime.js",
+    "../common/testing/mock_input_method_private.js",
+    "../common/testing/mock_language_settings_private.js",
   ]
 
   # The test base classes generate C++ code with these deps.
@@ -103,6 +106,8 @@ js_library("accessibility_common") {
     "$externs_path/accessibility_private.js",
     "$externs_path/automation.js",
     "$externs_path/command_line_private.js",
+    "$externs_path/input_method_private.js",
+    "$externs_path/language_settings_private.js",
   ]
 }
 
@@ -122,4 +127,9 @@ js_library("magnifier") {
 
 js_library("dictation") {
   sources = [ "dictation/dictation.js" ]
+  externs_list = [
+    "$externs_path/accessibility_private.js",
+    "$externs_path/input_method_private.js",
+    "$externs_path/language_settings_private.js",
+  ]
 }
diff --git a/...wser/resources/chromeos/accessibility/accessibility_common/accessibility_common_loader.js b/...wser/resources/chromeos/accessibility/accessibility_common/accessibility_common_loader.js
@@ -60,6 +60,11 @@ export class AccessibilityCommon {
         {}, this.onDictationUpdated_.bind(this));
     chrome.accessibilityFeatures.dictation.onChange.addListener(
         this.onDictationUpdated_.bind(this));
+
+    // AccessibilityCommon is an IME so it shows in the input methods list
+    // when it starts up. Remove from this list, Dictation will add it back
+    // whenever needed.
+    Dictation.removeAsInputMethod();
   }
 
   /**