Fix ISearch regressions

Fix ISearch regressions

- use a cursors.Cursor rather than a node; sometimes, nodes are destroyed when the Panel receives focus. A cursor will resolve to the ancestors of the destroyed node
- ensure event handlers are always unregistered
- fix the pending callback to ensure we always set focus to the right search hit

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2621773003
Cr-Commit-Position: refs/heads/master@{#442655}
Committed: https://chromium.googlesource.com/chromium/src/+/780fd7effb09f3975ae14f7b6fc501ce56cf96e5

[David Tseng, 2017-01-09 23:45:02]

Description was changed from

==========
Fix ISearch regressions

- use a cursors.Cursor rather than a node; sometimes, nodes are destroyed when
the Panel receives focus. A cursor will resolve to the ancestors of the
destroyed node
- ensure event handlers are always unregistered
- fix the pending callback to ensure we always set focus to the right search hit

BUG=
==========

to

==========
Fix ISearch regressions

- use a cursors.Cursor rather than a node; sometimes, nodes are destroyed when
the Panel receives focus. A cursor will resolve to the ancestors of the
destroyed node
- ensure event handlers are always unregistered
- fix the pending callback to ensure we always set focus to the right search hit

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-01-09 23:46:36]

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

[David Tseng, 2017-01-10 00:28:27]

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

[commit-bot: I haz the power, 2017-01-10 00:29:05]

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

[dmazzoni, 2017-01-10 16:07:15]

lgtm with one suggestion

https://codereview.chromium.org/2621773003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/i_search.js
(right):

https://codereview.chromium.org/2621773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/i_search.js:118:
this.iSearch_ = new ISearch(this.background_.currentRange.start.node);
How about if you have the ISearch constructor takea  cursor, and pass
currentRange.start here, rather than going from a cursor to a node to a cursor.

[David Tseng, 2017-01-10 17:28:56]

On 2017/01/10 16:07:15, dmazzoni wrote:
> lgtm with one suggestion
> 
>
https://codereview.chromium.org/2621773003/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/chromeos/chromevox/cvox2/background/i_search.js
> (right):
> 
>
https://codereview.chromium.org/2621773003/diff/20001/chrome/browser/resource...
> chrome/browser/resources/chromeos/chromevox/cvox2/background/i_search.js:118:
> this.iSearch_ = new ISearch(this.background_.currentRange.start.node);
> How about if you have the ISearch constructor takea  cursor, and pass
> currentRange.start here, rather than going from a cursor to a node to a
cursor.

Done though going from Cursor to node to Cursor still has to happen because we
sync to a leaf.

[David Tseng, 2017-01-10 17:47:47]

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

[commit-bot: I haz the power, 2017-01-10 17:48:07]

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-01-10 19:03:42]

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

[commit-bot: I haz the power, 2017-01-10 19:03:43]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2017-01-10 19:22:27]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2017-01-10 19:22:27]

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/2621773003/#ps40001
(title: "Fix ISearch regressions")

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

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-01-10 19:27:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484076147008140,
"parent_rev": "616f5a726f4c8140d4d6e67d3353488d0a8f8b6e", "commit_rev":
"780fd7effb09f3975ae14f7b6fc501ce56cf96e5"}

[commit-bot: I haz the power, 2017-01-10 19:28:28]

Description was changed from

==========
Fix ISearch regressions

- use a cursors.Cursor rather than a node; sometimes, nodes are destroyed when
the Panel receives focus. A cursor will resolve to the ancestors of the
destroyed node
- ensure event handlers are always unregistered
- fix the pending callback to ensure we always set focus to the right search hit

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix ISearch regressions

- use a cursors.Cursor rather than a node; sometimes, nodes are destroyed when
the Panel receives focus. A cursor will resolve to the ancestors of the
destroyed node
- ensure event handlers are always unregistered
- fix the pending callback to ensure we always set focus to the right search hit

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2621773003
Cr-Commit-Position: refs/heads/master@{#442655}
Committed:
https://chromium.googlesource.com/chromium/src/+/780fd7effb09f3975ae14f7b6fc5...
==========

[commit-bot: I haz the power, 2017-01-10 19:28:29]

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