Set keys for SwitchAccessEventHandler to capture using accessibiltyPrivate API

chrome/browser/accessibility/accessibility_extension_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/accessibility/accessibility_extension_api.h"
 
 #include <stddef.h>
 
 #include "base/json/json_writer.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
 #include "chrome/browser/extensions/chrome_extension_function_details.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/accessibility_private.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "components/infobars/core/infobar.h"
 #include "content/public/browser/browser_accessibility_state.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/lazy_background_task_queue.h"
 #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"
 #include "chrome/browser/chromeos/ui/accessibility_focus_ring_controller.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
 
 using chromeos::AccessibilityFocusRingController;
 #endif
 
@@ skipping 117 matching lines @@
 
   // Called twice to ensure the cros end of the dbus message is in a good
   // state.
   client->SetBacklightsForcedOff(!darken);
   client->SetBacklightsForcedOff(darken);
   return RespondNow(NoArguments());
 #endif  // defined OS_CHROMEOS
 
   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));
+}