Set keys for SwitchAccessEventHandler to capture using accessibiltyPrivate API

chrome/browser/accessibility/accessibility_extension_api.cc

Index: chrome/browser/accessibility/accessibility_extension_api.cc
diff --git a/chrome/browser/accessibility/accessibility_extension_api.cc b/chrome/browser/accessibility/accessibility_extension_api.cc
index 62abc25566768286c5c722d2896334b09bbf0b2b..54903465516fb3449beb58174d52093a27b2ddfb 100644
--- a/chrome/browser/accessibility/accessibility_extension_api.cc
+++ b/chrome/browser/accessibility/accessibility_extension_api.cc
@@ -27,6 +27,7 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/image_util.h"
 #include "extensions/common/manifest_handlers/background_info.h"
+#include "ui/events/keycodes/keyboard_codes.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
@@ -164,3 +165,29 @@ AccessibilityPrivateDarkenScreenFunction::Run() {
 
   return RespondNow(Error(kErrorNotSupported));
 }
+
+ExtensionFunction::ResponseAction
+AccessibilityPrivateSetSwitchAccessKeysFunction::Run() {
+#if defined(OS_CHROMEOS)

[Devlin, 2017/05/30 19:22:03]

Is there a reason to have this function present on non-chromeos platforms,
rather than making it only available on ChromeOS?

[dmazzoni, 2017/05/30 19:34:02]

Good point. Eli, I think you want "platforms": ["chromeos"] in the .json file

[elichtenberg, 2017/05/31 00:58:43]

Done.

+  base::ListValue* key_codes_list = NULL;

[Devlin, 2017/05/30 19:22:03]

nit: nullptr in new code, though possibly moot with below.

[elichtenberg, 2017/05/31 00:58:43]

Done.

+  EXTENSION_FUNCTION_VALIDATE(args_->GetSize() == 1);

[Devlin, 2017/05/30 19:22:03]

Any reason to not use the generated extension parsing?

[elichtenberg, 2017/05/31 00:58:43]

What exactly do you mean by generated extension parsing? I'm not sure what to
change. Are you saying there's some other way of getting the size of args or
getting the list of key codes without using GetSize or GetList?

[Devlin, 2017/05/31 01:33:32]

We auto-generate typed structs and parsing code for extension APIs.  The
generated files are, in this case,
<out_dir>/gen/chrome/common/api/accessibility_private.[h|cc].  You can use those
to parse the input params, doing something like:
std::unique_ptr<accessibility_private::SetSwitchAccessKeys::Params> params =
    accessibility_private::SetSwitchAccessKeys::Params::Create(*args_);
EXTENSION_FUNCTION_VALIDATE(params);

and then params contains a std::vector<int>, rather than needing to iterate over
the list value.  This approach is generally less error-prone, since it doesn't
rely on argument indices, etc.

[elichtenberg, 2017/06/01 19:55:26]

Got it. Made the change.

+  EXTENSION_FUNCTION_VALIDATE(args_->GetList(0, &key_codes_list));
+
+  // For now, only accept key code if it represents an alphanumeric character.
+  std::unordered_set<int> key_codes;
+  for (size_t i = 0; i < key_codes_list->GetSize(); ++i) {
+    int key_code;

[Devlin, 2017/05/30 19:22:03]

nit: initialize

[elichtenberg, 2017/05/31 00:58:43]

Done.

+    EXTENSION_FUNCTION_VALIDATE(key_codes_list->GetInteger(i, &key_code));
+    if (key_code >= ui::VKEY_0 && key_code <= ui::VKEY_Z)
+      key_codes.insert(key_code);

[Devlin, 2017/05/30 19:22:03]

should there be an error if the key is out of bounds?  (See also comment in
.json)

[elichtenberg, 2017/05/31 00:58:43]

Changing it to have an error instead of just filtering out invalid keys.

+  }
+
+  chromeos::AccessibilityManager* manager =
+      chromeos::AccessibilityManager::Get();
+  if (manager)

[Devlin, 2017/05/30 19:22:03]

When can this be null?  Should we be returning an error?

[elichtenberg, 2017/05/31 00:58:43]

It can be null when the system is shutting down, but we shouldn't need to return
an error then.

+    manager->SetSwitchAccessKeys(key_codes);
+  return RespondNow(NoArguments());
+#endif  // defined (OS_CHROMEOS)
+
+  return RespondNow(Error(kErrorNotSupported));
+}