Use three way flag for ChromeVox arc support

- Use three way flag for ChromeVox arc support. This allows developers
  to easily switch between TalkBack on arc and ChromeVox arc support.
- This CL also changes default to disabled (TalkBack on arc).

Bug: 855310
Test: None
Change-Id: I301657798247da20eaf9ad734987b8d40790af1d
Reviewed-on: https://chromium-review.googlesource.com/1130963
Commit-Queue: Yuki Awano <yawano@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574411}

chrome/browser/about_flags.cc

@@ -3055,10 +3055,9 @@ const FeatureEntry kFeatureEntries[] = {
 #endif
 
 #if defined(OS_CHROMEOS)
-    {"enable-chromevox-arc-support",
+    {"ChromeVoxArcSupport", flag_descriptions::kChromeVoxArcSupportName,
-     flag_descriptions::kEnableChromevoxArcSupportName,
+     flag_descriptions::kChromeVoxArcSupportDescription, kOsCrOS,
-     flag_descriptions::kEnableChromevoxArcSupportDescription, kOsCrOS,
+     FEATURE_VALUE_TYPE(chromeos::features::kChromeVoxArcSupport)},
-     SINGLE_VALUE_TYPE(chromeos::switches::kEnableChromeVoxArcSupport)},
 #endif  // defined(OS_CHROMEOS)
 
 #if defined(OS_CHROMEOS)

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc

@@ -12,7 +12,7 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs_factory.h"
-#include "chromeos/chromeos_switches.h"
+#include "chromeos/chromeos_features.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/arc_service_manager.h"
@@ -80,33 +80,6 @@ void DispatchFocusChange(arc::mojom::AccessibilityNodeInfoData* node_data,
   accessibility_manager->OnViewFocusedInArc(bounds_in_screen);
 }
 
-arc::mojom::AccessibilityFilterType GetFilterTypeForProfile(Profile* profile) {
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          chromeos::switches::kEnableChromeVoxArcSupport)) {
-    return arc::mojom::AccessibilityFilterType::ALL;
-  }
-
-  chromeos::AccessibilityManager* accessibility_manager =
-      chromeos::AccessibilityManager::Get();
-  if (!accessibility_manager)
-    return arc::mojom::AccessibilityFilterType::OFF;
-
-  // TODO(yawano): Support the case where primary user is in background.
-  if (accessibility_manager->profile() != profile)
-    return arc::mojom::AccessibilityFilterType::OFF;
-
-  if (accessibility_manager->IsSpokenFeedbackEnabled() ||
-      accessibility_manager->IsSelectToSpeakEnabled() ||
-      accessibility_manager->IsSwitchAccessEnabled()) {
-    return arc::mojom::AccessibilityFilterType::ALL;
-  }
-
-  if (accessibility_manager->IsFocusHighlightEnabled())
-    return arc::mojom::AccessibilityFilterType::FOCUS;
-
-  return arc::mojom::AccessibilityFilterType::OFF;
-}
-
 }  // namespace
 
 namespace arc {
@@ -508,6 +481,38 @@ void ArcAccessibilityHelperBridge::OnAccessibilityStatusChanged(
   UpdateWindowProperties(GetActiveWindow());
 }
 
+arc::mojom::AccessibilityFilterType
+ArcAccessibilityHelperBridge::GetFilterTypeForProfile(Profile* profile) {
+  if (use_filter_type_all_for_test_)
+    return arc::mojom::AccessibilityFilterType::ALL;
+
+  chromeos::AccessibilityManager* accessibility_manager =
+      chromeos::AccessibilityManager::Get();
+  if (!accessibility_manager)
+    return arc::mojom::AccessibilityFilterType::OFF;
+
+  // TODO(yawano): Support the case where primary user is in background.
+  if (accessibility_manager->profile() != profile)
+    return arc::mojom::AccessibilityFilterType::OFF;
+
+  if (accessibility_manager->IsSelectToSpeakEnabled() ||
+      accessibility_manager->IsSwitchAccessEnabled()) {
+    return arc::mojom::AccessibilityFilterType::ALL;
+  }
+
+  if (accessibility_manager->IsSpokenFeedbackEnabled()) {
+    return base::FeatureList::IsEnabled(
+               chromeos::features::kChromeVoxArcSupport)
+               ? arc::mojom::AccessibilityFilterType::ALL
+               : arc::mojom::AccessibilityFilterType::WHITELISTED_PACKAGE_NAME;
+  }
+
+  if (accessibility_manager->IsFocusHighlightEnabled())
+    return arc::mojom::AccessibilityFilterType::FOCUS;
+
+  return arc::mojom::AccessibilityFilterType::OFF;
+}
+
 void ArcAccessibilityHelperBridge::UpdateFilterType() {
   arc::mojom::AccessibilityFilterType filter_type =
       GetFilterTypeForProfile(profile_);

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h

@@ -97,6 +97,8 @@ class ArcAccessibilityHelperBridge
     return notification_key_to_tree_;
   }
 
+  void set_filter_type_all_for_test() { use_filter_type_all_for_test_ = true; }
+
  protected:
   virtual aura::Window* GetActiveWindow();
 
@@ -110,6 +112,7 @@ class ArcAccessibilityHelperBridge
 
   void OnAccessibilityStatusChanged(
       const chromeos::AccessibilityStatusEventDetails& event_details);
+  arc::mojom::AccessibilityFilterType GetFilterTypeForProfile(Profile* profile);
   void UpdateFilterType();
   void UpdateWindowProperties(aura::Window* window);
   void UpdateTreeIdOfNotificationSurface(const std::string& notification_key,
@@ -130,6 +133,7 @@ class ArcAccessibilityHelperBridge
       notification_key_to_tree_;
   std::unique_ptr<chromeos::AccessibilityStatusSubscription>
       accessibility_status_subscription_;
+  bool use_filter_type_all_for_test_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(ArcAccessibilityHelperBridge);
 };

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_browsertest.cc

@@ -93,7 +93,7 @@ IN_PROC_BROWSER_TEST_F(ArcAccessibilityHelperBridgeBrowserTest,
               aura::client::kAccessibilityTouchExplorationPassThrough));
 
   chromeos::AccessibilityManager::Get()->EnableSpokenFeedback(true);
-  EXPECT_EQ(mojom::AccessibilityFilterType::ALL,
+  EXPECT_EQ(mojom::AccessibilityFilterType::WHITELISTED_PACKAGE_NAME,
             fake_accessibility_helper_instance_->filter_type());
 
   // Touch exploration pass through of test_window_1 (current active window)

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc

@@ -183,11 +183,10 @@ class ArcAccessibilityHelperBridgeTest : public ChromeViewsTestBase {
 };
 
 TEST_F(ArcAccessibilityHelperBridgeTest, TaskAndAXTreeLifecycle) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      chromeos::switches::kEnableChromeVoxArcSupport);
-
   TestArcAccessibilityHelperBridge* helper_bridge =
       accessibility_helper_bridge();
+  helper_bridge->set_filter_type_all_for_test();
+
   const auto& task_id_to_tree = helper_bridge->task_id_to_tree_for_test();
   ASSERT_EQ(0U, task_id_to_tree.size());
 
@@ -268,9 +267,6 @@ TEST_F(ArcAccessibilityHelperBridgeTest, TaskAndAXTreeLifecycle) {
 // mojo: notification 2 removed
 // wayland: surface 2 removed
 TEST_F(ArcAccessibilityHelperBridgeTest, NotificationEventArriveFirst) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      chromeos::switches::kEnableChromeVoxArcSupport);
-
   TestArcAccessibilityHelperBridge* helper_bridge =
       accessibility_helper_bridge();
   arc_notification_surface_manager_->AddObserver(helper_bridge);
@@ -364,9 +360,6 @@ TEST_F(ArcAccessibilityHelperBridgeTest, NotificationEventArriveFirst) {
 // mojo: notification 1 created
 // mojo: notification 1 removed
 TEST_F(ArcAccessibilityHelperBridgeTest, NotificationSurfaceArriveFirst) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      chromeos::switches::kEnableChromeVoxArcSupport);
-
   TestArcAccessibilityHelperBridge* helper_bridge =
       accessibility_helper_bridge();
   arc_notification_surface_manager_->AddObserver(helper_bridge);
@@ -404,8 +397,7 @@ TEST_F(ArcAccessibilityHelperBridgeTest, NotificationSurfaceArriveFirst) {
 
 TEST_F(ArcAccessibilityHelperBridgeTest,
        TextSelectionChangeActivateNotificationWidget) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+  accessibility_helper_bridge()->set_filter_type_all_for_test();
-      chromeos::switches::kEnableChromeVoxArcSupport);
 
   // Prepare notification surface.
   std::unique_ptr<MockArcNotificationSurface> surface =
@@ -460,8 +452,7 @@ TEST_F(ArcAccessibilityHelperBridgeTest,
 }
 
 TEST_F(ArcAccessibilityHelperBridgeTest, TextSelectionChangedFocusContentView) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+  accessibility_helper_bridge()->set_filter_type_all_for_test();
-      chromeos::switches::kEnableChromeVoxArcSupport);
 
   // Prepare notification surface.
   std::unique_ptr<MockArcNotificationSurface> surface =

chrome/browser/flag_descriptions.cc

@@ -2950,6 +2950,10 @@ const char kCaptivePortalBypassProxyDescription[] =
     "different captive portals. This enables opening captive portal "
     "authorization dialog in a separate window, which ignores proxy settings.";
 
+const char kChromeVoxArcSupportName[] = "ChromeVox ARC support";
+const char kChromeVoxArcSupportDescription[] =
+    "Use ChromeVox screen reader features in ARC";
+
 const char kCrOSComponentName[] = "Chrome OS Component";
 const char kCrOSComponentDescription[] =
     "Disable the use of componentized escpr CUPS filter.";
@@ -3001,10 +3005,6 @@ const char kEnableBackgroundBlurName[] = "Enable background blur.";
 const char kEnableBackgroundBlurDescription[] =
     "Enables background blur for the Peeking Launcher and Tab Switcher.";
 
-const char kEnableChromevoxArcSupportName[] = "ChromeVox ARC support";
-const char kEnableChromevoxArcSupportDescription[] =
-    "Enable ChromeVox screen reader features in ARC";
-
 const char kEnableDisplayZoomSettingName[] = "Enable display zoom settings";
 const char kEnableDisplayZoomSettingDescription[] =
     "Allows the user to modify the display size or zoom via the chrome display "

chrome/browser/flag_descriptions.h

@@ -1808,6 +1808,9 @@ extern const char kBulkPrintersDescription[];
 extern const char kCaptivePortalBypassProxyName[];
 extern const char kCaptivePortalBypassProxyDescription[];
 
+extern const char kChromeVoxArcSupportName[];
+extern const char kChromeVoxArcSupportDescription[];
+
 extern const char kCrOSComponentName[];
 extern const char kCrOSComponentDescription[];
 
@@ -1841,9 +1844,6 @@ extern const char kEnableAppsGridGapFeatureDescription[];
 extern const char kEnableBackgroundBlurName[];
 extern const char kEnableBackgroundBlurDescription[];
 
-extern const char kEnableChromevoxArcSupportName[];
-extern const char kEnableChromevoxArcSupportDescription[];
-
 extern const char kEnableDisplayZoomSettingName[];
 extern const char kEnableDisplayZoomSettingDescription[];
 

chromeos/chromeos_features.cc

@@ -12,6 +12,10 @@ namespace features {
 const base::Feature kAndroidMessagesIntegration{
     "AndroidMessagesIntegration", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables or disables native ChromeVox support for Arc.
+const base::Feature kChromeVoxArcSupport{"ChromeVoxArcSupport",
+                                         base::FEATURE_DISABLED_BY_DEFAULT};
+
 // If enabled, DriveFS will be used for Drive sync.
 const base::Feature kDriveFs{"DriveFS", base::FEATURE_DISABLED_BY_DEFAULT};
 

chromeos/chromeos_features.h

@@ -16,6 +16,7 @@ namespace features {
 // alongside the definition of their values in the .cc file.
 
 CHROMEOS_EXPORT extern const base::Feature kAndroidMessagesIntegration;
+CHROMEOS_EXPORT extern const base::Feature kChromeVoxArcSupport;
 CHROMEOS_EXPORT extern const base::Feature kDriveFs;
 CHROMEOS_EXPORT extern const base::Feature kEnableUnifiedMultiDeviceSettings;
 CHROMEOS_EXPORT extern const base::Feature kEnableUnifiedMultiDeviceSetup;

chromeos/chromeos_switches.cc

@@ -313,9 +313,6 @@ const char kEnableCaptivePortalRandomUrl[] = "enable-captive-portal-random-url";
 // Enables the Cast Receiver.
 const char kEnableCastReceiver[] = "enable-cast-receiver";
 
-// Enables native ChromeVox support for Arc.
-const char kEnableChromeVoxArcSupport[] = "enable-chromevox-arc-support";
-
 // Enables consumer kiosk mode for Chrome OS.
 const char kEnableConsumerKiosk[] = "enable-consumer-kiosk";
 

chromeos/chromeos_switches.h

@@ -92,7 +92,6 @@ CHROMEOS_EXPORT extern const char kEnableArc[];
 CHROMEOS_EXPORT extern const char kEnableArcOobeOptinNoSkip[];
 CHROMEOS_EXPORT extern const char kEnableCaptivePortalRandomUrl[];
 CHROMEOS_EXPORT extern const char kEnableCastReceiver[];
-CHROMEOS_EXPORT extern const char kEnableChromeVoxArcSupport[];
 CHROMEOS_EXPORT extern const char kEnableConsumerKiosk[];
 CHROMEOS_EXPORT extern const char kEnableDataSaverPrompt[];
 CHROMEOS_EXPORT extern const char kEnableDemoMode[];

tools/metrics/histograms/enums.xml

@@ -27784,6 +27784,7 @@ from previous Chrome versions.
   <int value="-1255427595" label="HomePageButtonForceEnabled:enabled"/>
   <int value="-1254070521" label="enable-slimming-paint-invalidation"/>
   <int value="-1251411236" label="disable-new-md-input-view"/>
+  <int value="-1250611337" label="ChromeVoxArcSupport:disabled"/>
   <int value="-1248478422" label="enable-zip-archiver-packer"/>
   <int value="-1246840031" label="OptInImeMenu:disabled"/>
   <int value="-1241747717" label="enable-android-password-link"/>
@@ -28325,6 +28326,7 @@ from previous Chrome versions.
   <int value="-68877684" label="BackgroundVideoTrackOptimization:enabled"/>
   <int value="-68225452" label="enable-translate-new-ux"/>
   <int value="-67297229" label="OfflinePagesDescriptivePendingStatus:disabled"/>
+  <int value="-59530055" label="ChromeVoxArcSupport:enabled"/>
   <int value="-59401847" label="ContentSuggestionsLargeThumbnail:disabled"/>
   <int value="-58242474" label="ash-disable-swipe-to-close-in-overview-mode"/>
   <int value="-57986995" label="DisablePostScriptPrinting:enabled"/>