Mojo: Don't hold a watch context ref in SimpleWatcher posted task

Mojo: Don't hold a watch context ref in SimpleWatcher posted task

In test environments it's fairly common for task runners to be
torn down without flushing posted tasks. If there's e.g. a binding
object on the test function stack that gets destroyed on unwind,
this might end up posting a SimpleWatcher notification task that
then never runs.

Currently we disambiguate watch contexts in SimpleWatcher by
retaining a ref to the Context in said notification task and comparing
to the currently known Context instance on the SimpleWatcher's thread.
This can lead to indirect leaks in the aforementioned testing scenario.

This CL instead introduces a sequence number in SimpleWatcher which is
incremented every time a new watch is started. This sequence number is
used to disambiguate notifications which might have come from a previous
watch, thus avoiding the unnecessary ref count on Context.

BUG=702180
R=yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2754863002
Cr-Commit-Position: refs/heads/master@{#457455}
Committed: https://chromium.googlesource.com/chromium/src/+/58e6cc73277d3bc9e67687df2c1dd575019d3282

[Ken Rockot(use gerrit already), 2017-03-16 14:10:11]

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

[commit-bot: I haz the power, 2017-03-16 14:10:30]

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

[Ken Rockot(use gerrit already), 2017-03-16 14:14:44]

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

[Ken Rockot(use gerrit already), 2017-03-16 14:14:53]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-03-16 14:14:59]

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

[Ken Rockot(use gerrit already), 2017-03-16 14:17:58]

PTAL, thanks!

[yzshen1, 2017-03-16 15:50:16]

lgtm

[Ken Rockot(use gerrit already), 2017-03-16 15:50:32]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-03-16 15:50:33]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2017-03-16 15:50:43]

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-16 16:36:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489679433431280,
"parent_rev": "bb383d154b568ef7aaf4e8d7c4056eb4f71a4ce0", "commit_rev":
"58e6cc73277d3bc9e67687df2c1dd575019d3282"}

[commit-bot: I haz the power, 2017-03-16 16:37:31]

Description was changed from

==========
Mojo: Don't hold a watch context ref in SimpleWatcher posted task

In test environments it's fairly common for task runners to be
torn down without flushing posted tasks. If there's e.g. a binding
object on the test function stack that gets destroyed on unwind,
this might end up posting a SimpleWatcher notification task that
then never runs.

Currently we disambiguate watch contexts in SimpleWatcher by
retaining a ref to the Context in said notification task and comparing
to the currently known Context instance on the SimpleWatcher's thread.
This can lead to indirect leaks in the aforementioned testing scenario.

This CL instead introduces a sequence number in SimpleWatcher which is
incremented every time a new watch is started. This sequence number is
used to disambiguate notifications which might have come from a previous
watch, thus avoiding the unnecessary ref count on Context.

BUG=702180
R=yzshen@chromium.org
==========

to

==========
Mojo: Don't hold a watch context ref in SimpleWatcher posted task

In test environments it's fairly common for task runners to be
torn down without flushing posted tasks. If there's e.g. a binding
object on the test function stack that gets destroyed on unwind,
this might end up posting a SimpleWatcher notification task that
then never runs.

Currently we disambiguate watch contexts in SimpleWatcher by
retaining a ref to the Context in said notification task and comparing
to the currently known Context instance on the SimpleWatcher's thread.
This can lead to indirect leaks in the aforementioned testing scenario.

This CL instead introduces a sequence number in SimpleWatcher which is
incremented every time a new watch is started. This sequence number is
used to disambiguate notifications which might have come from a previous
watch, thus avoiding the unnecessary ref count on Context.

BUG=702180
R=yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2754863002
Cr-Commit-Position: refs/heads/master@{#457455}
Committed:
https://chromium.googlesource.com/chromium/src/+/58e6cc73277d3bc9e67687df2c1d...
==========

[commit-bot: I haz the power, 2017-03-16 16:37:32]

Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/58e6cc73277d3bc9e67687df2c1d...