Add custom action support

components/arc/common/accessibility_helper.mojom

Index: components/arc/common/accessibility_helper.mojom
diff --git a/components/arc/common/accessibility_helper.mojom b/components/arc/common/accessibility_helper.mojom
index f3651b2356a5288163b4f727844bdd1ed15d7ecf..730bdda44107d83437dad8f6943eedf310f7ff7b 100644
--- a/components/arc/common/accessibility_helper.mojom
+++ b/components/arc/common/accessibility_helper.mojom
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Next MinVersion: 3
+// Next MinVersion: 4
 
 module arc.mojom;
 
@@ -69,7 +69,15 @@ enum AccessibilityActionType {
   EXPAND,
   COLLAPSE,
   DISMISS,
-  SET_TEXT
+  SET_TEXT,
+  CONTEXT_CLICK,
+  SCROLL_DOWN,
+  SCROLL_LEFT,
+  SCROLL_RIGHT,
+  SCROLL_TO_POSITION,
+  SCROLL_UP,
+  SET_PROGRESS,
+  SHOW_ON_SCREEN
 };
 
 // Possible boolean properties set on an AccessibilityNodeInfo.
@@ -130,7 +138,13 @@ enum AccessibilityIntProperty {
 [Extensible]
 enum AccessibilityIntListProperty {
   CHILD_NODE_IDS,
-  ACTIONS
+  ACTIONS,

[David Tseng, 2017/06/08 17:21:59]

This doesn't appear to be used. It might be safe to just rename it to
custom_action_ids.

[yawano, 2017/06/13 06:58:47]

Done.

+  CUSTOM_ACTION_IDS
+};
+
+[Extensible]
+enum AccessibilityStringListProperty {
+  CUSTOM_ACTION_DESCRIPTIONS
 };
 
 // AccessibilityNodeInfoData is a struct to contain info of
@@ -144,6 +158,7 @@ struct AccessibilityNodeInfoData {
   [MinVersion=1]map<AccessibilityIntProperty, int32>? int_properties;
   [MinVersion=1]
       map<AccessibilityIntListProperty, array<int32>>? int_list_properties;
+  [MinVersion=3]map<AccessibilityStringListProperty, array<string>>? string_list_properties;
 };
 
 // Filters the event type (and implicitly the data) sent by the ARC
@@ -182,7 +197,7 @@ interface AccessibilityHelperHost {
   OnAccessibilityEvent@1(AccessibilityEventData event_data);
 };
 
-// Next method ID: 3
+// Next method ID: 4
 interface AccessibilityHelperInstance {
   Init@0(AccessibilityHelperHost host);
 
@@ -191,4 +206,7 @@ interface AccessibilityHelperInstance {
 
   // Set a filter on the event types received.
   SetFilter@2(AccessibilityFilterType filter_type);
+
+  // Perform a custom action on a node requested by a Chrome client.
+  [MinVersion=3]PerformCustomAction@3(int32 id, int32 custom_action_id);

[David Tseng, 2017/06/08 17:21:59]

Considering we might want just one action list in the future, wouldn't it make
sense to try merging this with performAction (i.e. make performAction take an
ActionData)? On the Android end, therer isn't a distinction between a standard
action vs a custom action.

[yawano, 2017/06/09 08:03:01]

I would prefer to keep standard action and custom action separated in our mojo
interface. In Android, they are sharing ids with standard actions and custom
actions. But I'm afraid to share them in our mojo interface as well. It would be
difficult to keep maintaining our standard action ids in mojom file are synced
with those in Android framework.

If we need to manage those ids separately, we need to send them in the separated
int list from Android side to Chrome side. We will need to send descriptions of
standard actions as well in the future. Android supports to override standard
actions.

If we manage standard actions and custom actions separately for sending from
Android to Chrome, I think it makes sense to keep them separated in the other
way as well.

WDYT?

[David Tseng, 2017/06/09 18:01:32]

I should be more clear. I was thinking:
performAction(ActionData data);

// Passed to |performAction|. A caller must specify either standard_action_id or
custom_action_id.
struct ActionData {
  AccessibilityActionType standard_action_id;

  // Optional.
   int32 custom_id;

  ... (we can add additional args here).
};

[yawano, 2017/06/12 01:46:52]

My point was that it would make sense to keep standard actions and custom
actions separated if we treat them separated when sending them from Android to
Chrome side. It would be symmetric.

The pro of adding ActionData struct is that it makes easy to extend the API
later. Adding a field to the struct is much easier than changing the interface
of the method.

After a thought, I will change this to use ActionData. Some standard actions
take arguments, e.g. set text, text selection. We will need to make this
extensible in the future. This should be a good time to address the issue in
advance.

[David Tseng, 2017/06/12 16:55:01]

Totally understood. However, the automation api packs up actions into one data
structure and has one internal binding for it
(AutomationInternal::performAction). 
Yup; I was thinking of custom actions as a "standard" action with extra
arguments of action id. That could be one way to unify standard and custom --
add a custom action type to the AccessibilityActionType in the mojom as well as
in ax_enums.idl.

[yawano, 2017/06/13 06:58:47]

Changed PerformAction to take AccessibilityActionData.

 };