Add chrome flags entry for ChromeVox ARC support

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

Index: chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
diff --git a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
index 8959cb3fb70648cca7e4bae58b3fbd7eb33444bc..90f0b13425d68b3d817559f5541254d7c9180e80 100644
--- a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
+++ b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
@@ -152,10 +152,18 @@ class ArcSettingsServiceImpl
   // Returns the integer value of the pref.  pref_name must exist.
   int GetIntegerPref(const std::string& pref_name) const;
 
+  // Gets whether this is a managed pref.
+  bool IsPrefManaged(const std::string& pref_name) const;
+
   // Sends boolean pref broadcast to the delegate.
   void SendBoolPrefSettingsBroadcast(const std::string& pref_name,
                                      const std::string& action) const;
 
+  // Same as above, except sends a specific boolean value.
+  void SendBoolValueSettingsBroadcast(bool value,
+                                      bool managed,
+                                      const std::string& action) const;
+
   // Sends a broadcast to the delegate.
   void SendSettingsBroadcast(const std::string& action,
                              const base::DictionaryValue& extras) const;
@@ -490,14 +498,39 @@ void ArcSettingsServiceImpl::SyncReportingConsent() const {
 }
 
 void ArcSettingsServiceImpl::SyncSpokenFeedbackEnabled() const {
-  std::string setting =
+  // Chrome spoken feedback triggers enabling of Android spoken feedback.
+  // There are two types of spoken feedback from Android:
+  // 1. Talkback (default)
+  // 2. accessibility helper (experimental, works through ChromeVox).
+  // These two features are mutually exclusive.
+
+  int enabled = GetIntegerPref(prefs::kAccessibilitySpokenFeedbackEnabled);
+  bool managed = IsPrefManaged(prefs::kAccessibilitySpokenFeedbackEnabled);
+
+  std::string talkback_setting =
       "org.chromium.arc.intent_helper.SET_SPOKEN_FEEDBACK_ENABLED";
+  std::string accessibility_helper_setting =
+      "org.chromium.arc.intent_helper.SET_ACCESSIBILITY_HELPER_ENABLED";
+
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          chromeos::switches::kEnableChromeVoxArcSupport))
-    setting = "org.chromium.arc.intent_helper.SET_ACCESSIBILITY_HELPER_ENABLED";
+          chromeos::switches::kEnableChromeVoxArcSupport)) {
+    SendBoolValueSettingsBroadcast(enabled, managed,
+                                   accessibility_helper_setting);
+
+    // Make sure if ChromeVox is on, TalkBack is off.
+    if (enabled) {
+      SendBoolValueSettingsBroadcast(false, managed, talkback_setting);
+    }
+    return;
+  }
+
+  SendBoolValueSettingsBroadcast(enabled, managed, talkback_setting);
 
-  SendBoolPrefSettingsBroadcast(prefs::kAccessibilitySpokenFeedbackEnabled,
-                                setting);
+  // Makes sure if Talkback is on, accessibility helper is off.
+  if (enabled) {

[yawano, 2017/03/10 05:05:47]

I think this will disable focus highlight if user has enabled TalkBack as focus
highlight also relies on accessibility helper. We will need to have an
additional check here or ArcAccessibilityHelper side.

[David Tseng, 2017/03/10 17:51:38]

I'm not sure I follow. Are you saying if someone has ChromeVox enabled and focus
highlight enabled? In that case, removing this check is fine as accessibility
helper when loaded, sends a filter type to the service from the Chrome end.
Nothing else is needed (i.e. this would retain the previous behavior).

In general though, it doesn't make sense to have focus highlight and ChromeVox
enabled at the same time since both ChromeVox and Talkback do their own focus
highlighting, right?

[yawano, 2017/03/13 02:03:24]

Sorry, I missed the point that TalkBack also does focus highlight. Yes, we
should be okay to disable accessibility helper. I think both leaving
accessibility helper as is and disabling it should be okay. Thank you.

+    SendBoolValueSettingsBroadcast(false, managed,
+                                   accessibility_helper_setting);
+  }
 }
 
 void ArcSettingsServiceImpl::SyncTimeZone() const {
@@ -558,6 +591,15 @@ int ArcSettingsServiceImpl::GetIntegerPref(const std::string& pref_name) const {
   return val;
 }
 
+bool ArcSettingsServiceImpl::IsPrefManaged(const std::string& pref_name) const {
+  const PrefService::Preference* pref =
+      registrar_.prefs()->FindPreference(pref_name);
+  DCHECK(pref);
+  bool value_exists = pref->GetValue()->is_int();
+  DCHECK(value_exists);
+  return !pref->IsUserModifiable();
+}
+
 void ArcSettingsServiceImpl::SendBoolPrefSettingsBroadcast(
     const std::string& pref_name,
     const std::string& action) const {
@@ -567,9 +609,16 @@ void ArcSettingsServiceImpl::SendBoolPrefSettingsBroadcast(
   bool enabled = false;
   bool value_exists = pref->GetValue()->GetAsBoolean(&enabled);
   DCHECK(value_exists);
+  SendBoolValueSettingsBroadcast(enabled, !pref->IsUserModifiable(), action);
+}
+
+void ArcSettingsServiceImpl::SendBoolValueSettingsBroadcast(
+    bool enabled,
+    bool managed,
+    const std::string& action) const {
   base::DictionaryValue extras;
   extras.SetBoolean("enabled", enabled);
-  extras.SetBoolean("managed", !pref->IsUserModifiable());
+  extras.SetBoolean("managed", managed);
   SendSettingsBroadcast(action, extras);
 }