Recovery: Implement focus recovery across root AutomationNodes

Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918, 628912, 624586, 605377, 524673, 652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/7a99c7d3e7dc2ca71bbaa127099239355a41fd37
Cr-Commit-Position: refs/heads/master@{#424612}

[David Tseng, 2016-10-11 15:50:32]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape. Verify focus lands as expected.
BUG=
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape. Verify focus lands as expected.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 15:51:20]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape. Verify focus lands as expected.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 15:51:20]

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

[David Tseng, 2016-10-11 15:53:54]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 15:54:22]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 17:36:58]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2016-10-11 17:41:18]

lgtm

I love this idea overall, seems much cleaner
than excursions.

I think you may as well store a full Range in
the WeakMap rather than just the start node,
it will be a better experience.

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:159:
* @type {WeakMap<AutomationNode>}
I think it should be WeakMap<AutomationNode, AutomationNode>, right?

Please document the meaning of the key and value in the map

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:668:
this.focusRecoveryMap_.set(root, this.currentRange.start.node);
Why not store and recover the whole range?

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:622: var
onFocus = function(evt) {
Rather than add and remove a focus listener, what if there
was just a permanent focus listener that calls any pending
callbacks whenever focus lands somewhere other than the
panel?

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:627:
chrome.automation.getDesktop(function(node) {
How about: getDesktop(function(desktop) {

[David Tseng, 2016-10-11 17:43:14]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 17:48:55]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 21:19:52]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-10-11 21:19:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2412433004/#ps20001
(title: "Address feedback.")

[commit-bot: I haz the power, 2016-10-11 21:20:15]

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

[David Tseng, 2016-10-11 21:20:57]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673,652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-10-11 21:24:32]

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:159:
* @type {WeakMap<AutomationNode>}
On 2016/10/11 17:41:18, dmazzoni wrote:
> I think it should be WeakMap<AutomationNode, AutomationNode>, right?
> 
> Please document the meaning of the key and value in the map
Yup; you're right.
Done (Closure doesn't seem to care either way).

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:668:
this.focusRecoveryMap_.set(root, this.currentRange.start.node);
On 2016/10/11 17:41:18, dmazzoni wrote:
> Why not store and recover the whole range?

Done.

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:622: var
onFocus = function(evt) {
On 2016/10/11 17:41:18, dmazzoni wrote:
> Rather than add and remove a focus listener, what if there
> was just a permanent focus listener that calls any pending
> callbacks whenever focus lands somewhere other than the
> panel?

I'd like to limit the panel's global event listeners to make sure we never
move/steal focus when we shouldn't.

https://codereview.chromium.org/2412433004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:627:
chrome.automation.getDesktop(function(node) {
On 2016/10/11 17:41:18, dmazzoni wrote:
> How about: getDesktop(function(desktop) {

desktop is used above. I guess I could bind it to onFocus.

[commit-bot: I haz the power, 2016-10-11 21:32:14]

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

[commit-bot: I haz the power, 2016-10-11 21:32:15]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[David Tseng, 2016-10-11 22:10:05]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2016-10-11 22:11:08]

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, 2016-10-12 00:20:47]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673,652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673,652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-10-12 00:20:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-12 00:25:11]

Description was changed from

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673,652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Recovery: Implement focus recovery across root AutomationNodes

This cl maintains a WeakMap of AutomationRootNode objects to the last known node
that had ChromeVox range.

This map gets updated manually when marked or implicitly when focus crosses top
level root nodes.

TEST=manual; open context menu, open panel, activate menus that manipulate
focus, press escape; ctrl+tab, alt+tab. Verify focus lands as expected.
BUG=631918,628912,624586,605377,524673,652143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/7a99c7d3e7dc2ca71bbaa127099239355a41fd37
Cr-Commit-Position: refs/heads/master@{#424612}
==========

[commit-bot: I haz the power, 2016-10-12 00:25:12]

Patchset 2 (id:??) landed as
https://crrev.com/7a99c7d3e7dc2ca71bbaa127099239355a41fd37
Cr-Commit-Position: refs/heads/master@{#424612}