Generalize the HIT_TEST accessibility action so that it can send any event.

Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2748463003
Cr-Commit-Position: refs/heads/master@{#460192}
Committed: https://chromium.googlesource.com/chromium/src/+/7a44adcc7ca0d47f73d8cbc286f540cbc65e5859

[dmazzoni, 2017-03-10 22:37:14]

Description was changed from

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
==========

to

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dmazzoni, 2017-03-10 22:39:59]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org, nasko@chromium.org

[dmazzoni, 2017-03-10 22:39:59]

dtseng: primary review
nasko: owners review for content/ and *messages*

[nasko, 2017-03-13 21:28:46]

content/ plumbing and IPC LGTM

[David Tseng, 2017-03-13 21:43:47]

You can also hook up the automation bindings to include the new action argument.

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
File content/browser/accessibility/hit_testing_browsertest.cc (left):

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
content/browser/accessibility/hit_testing_browsertest.cc:45:
BrowserAccessibility* hovered_node =
nit: hit_node

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
File content/browser/accessibility/hit_testing_browsertest.cc (right):

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
content/browser/accessibility/hit_testing_browsertest.cc:167:
ASSERT_TRUE(hovered_node != NULL);
nit: ASSERT_NE

https://codereview.chromium.org/2748463003/diff/1/ui/accessibility/ax_action_...
File ui/accessibility/ax_action_data.cc (right):

https://codereview.chromium.org/2748463003/diff/1/ui/accessibility/ax_action_...
ui/accessibility/ax_action_data.cc:26: hit_test_event_to_fire(AX_EVENT_NONE) {}
Seems like this should default to hover. Alternatively, CHECK if HIT_TEST gets
called without an event to fire set.

[dmazzoni, 2017-03-20 05:40:37]

Description was changed from

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:linux_site_isolation
==========

[dmazzoni, 2017-03-20 05:40:40]

I added an automation API binding and a test too.

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
File content/browser/accessibility/hit_testing_browsertest.cc (left):

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
content/browser/accessibility/hit_testing_browsertest.cc:45:
BrowserAccessibility* hovered_node =
On 2017/03/13 21:43:47, David Tseng wrote:
> nit: hit_node

Done.

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
File content/browser/accessibility/hit_testing_browsertest.cc (right):

https://codereview.chromium.org/2748463003/diff/1/content/browser/accessibili...
content/browser/accessibility/hit_testing_browsertest.cc:167:
ASSERT_TRUE(hovered_node != NULL);
On 2017/03/13 21:43:47, David Tseng wrote:
> nit: ASSERT_NE

Done.

https://codereview.chromium.org/2748463003/diff/1/ui/accessibility/ax_action_...
File ui/accessibility/ax_action_data.cc (right):

https://codereview.chromium.org/2748463003/diff/1/ui/accessibility/ax_action_...
ui/accessibility/ax_action_data.cc:26: hit_test_event_to_fire(AX_EVENT_NONE) {}
On 2017/03/13 21:43:47, David Tseng wrote:
> Seems like this should default to hover. Alternatively, CHECK if HIT_TEST gets
> called without an event to fire set.

I added a DCHECK. I'd rather we make it explicit.

[David Tseng, 2017-03-22 15:27:57]

lgtm; thanks for the automation bindings. We can do focus follows mouse
trivially in cvox with this change.

[David Tseng, 2017-03-22 15:40:28]

Actually, I think you need to hook up hit testing on the desktop tree still,
right?

https://codereview.chromium.org/2748463003/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
(right):

https://codereview.chromium.org/2748463003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:362:
if (ui::ToString(event) == hit_test_params.event_to_fire)
Pretty sure you can do ParseEvent here.

[dmazzoni, 2017-03-23 16:29:30]

On 2017/03/22 15:40:28, David Tseng wrote:
> Actually, I think you need to hook up hit testing on the desktop tree still,
> right?

Yes, that's coming in a follow-up, it depends on
https://codereview.chromium.org/2774683002/ landing
first and then it will be possible to initiate a hit test
in the desktop and have it forward to the web.

https://codereview.chromium.org/2748463003/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
(right):

https://codereview.chromium.org/2748463003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:362:
if (ui::ToString(event) == hit_test_params.event_to_fire)
On 2017/03/22 15:40:28, David Tseng wrote:
> Pretty sure you can do ParseEvent here.

Done.

[dmazzoni, 2017-03-23 16:58:05]

dmazzoni@chromium.org changed reviewers:
+ dbeam@chromium.org

[dmazzoni, 2017-03-23 16:58:06]

+dbeam for third_party/closure_compiler/externs/automation.js

[dmazzoni, 2017-03-23 16:58:12]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-23 16:58:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-23 17:23:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 17:23:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-03-25 06:47:46]

lgtm

[dmazzoni, 2017-03-27 16:55:49]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2017-03-27 16:55:50]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org,
dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2748463003/#ps40001
(title: "Use ParseAXEvent")

[commit-bot: I haz the power, 2017-03-27 16:56:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 18:07:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 18:08:00]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2017-03-28 17:30:02]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2017-03-28 17:30:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 19:55:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490722202680050,
"parent_rev": "e774f07c97a47852ad648879cba168424df0bf08", "commit_rev":
"7a44adcc7ca0d47f73d8cbc286f540cbc65e5859"}

[commit-bot: I haz the power, 2017-03-28 19:55:48]

Description was changed from

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Generalize the HIT_TEST accessibility action so that it can send any event.

To determine what object is under a certain location on the screen we
have an accessibility action from the browser to renderer and then a
response event fired on the correct accessibility object. It gets more
complicated and bounces around a few times if there are out-of-process
iframes.

For historical reasons we fire a HOVER event on the hit object, but
this is somewhat arbitrary. Generalize this by letting the caller
specify what event to fire. That will be useful in improving the
select-to-speak feature on Chrome OS.

In this change, just generalize the HIT_TEST to fire any specified event
and add test coverage.

BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2748463003
Cr-Commit-Position: refs/heads/master@{#460192}
Committed:
https://chromium.googlesource.com/chromium/src/+/7a44adcc7ca0d47f73d8cbc286f5...
==========

[commit-bot: I haz the power, 2017-03-28 19:55:50]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7a44adcc7ca0d47f73d8cbc286f5...