Allow ObjectWatcher to be used from sequenced tasks.

Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631
Committed: https://crrev.com/0838b883b2a89bbfa6b1f845e0dfd5765781bc07
Cr-Commit-Position: refs/heads/master@{#440519}

[fdoray, 2016-12-21 19:38:13]

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

[commit-bot: I haz the power, 2016-12-21 19:38:41]

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, 2016-12-21 20:42:08]

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

[commit-bot: I haz the power, 2016-12-21 20:42:09]

Dry run: This issue passed the CQ dry run.

[fdoray, 2016-12-22 13:09:27]

Description was changed from

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task,
this CL does not change the behavior of ObjectWatcher when used
from a single-threaded task.

BUG=675631
==========

to

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in 
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631
==========

[fdoray, 2016-12-22 13:16:42]

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

[commit-bot: I haz the power, 2016-12-22 13:16:57]

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

[fdoray, 2016-12-22 13:17:06]

fdoray@chromium.org changed reviewers:
+ grt@chromium.org

[fdoray, 2016-12-22 13:17:06]

PTAL

[commit-bot: I haz the power, 2016-12-22 14:19:36]

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

[commit-bot: I haz the power, 2016-12-22 14:19:37]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2016-12-22 14:25:07]

I only looked at one consumer of ObjectWatcher -- I think FilePathWatcher has a
one-thread assumption built-in. Have you surveyed the other callers to see if
they're all okay with this change?

[fdoray, 2016-12-22 14:48:34]

On 2016/12/22 14:25:07, grt (UTC plus 1) wrote:
> I only looked at one consumer of ObjectWatcher -- I think FilePathWatcher has
a
> one-thread assumption built-in. Have you surveyed the other callers to see if
> they're all okay with this change?

We don't need to look at callers.
- Before this CL, ObjectWatcher::StartWatching*() could only be used when
ThreadTaskRunnerHandle::IsSet()
https://cs.chromium.org/chromium/src/base/win/object_watcher.cc?sq=package:ch...
- SequencedTaskRunnerHandle behaves the same way as ThreadTaskRunnerHandle when
ThreadTaskRunnerHandle::IsSet()
https://cs.chromium.org/chromium/src/base/threading/sequenced_task_runner_han...
[1]
Conclusion: This change does not affect existing callers. It merely allows new
uses of ObjectWatcher from sequenced tasks.

For us (task scheduler team), it is clear a phrase such as "a callback will be
invoked on the origin sequence" can be interpreted as "a callback will be
invoked on the origin MessageLoop, TaskScheduler multi-threaded sequence [2] or
TaskScheduler single-threaded sequence". I'm not sure where we could document
this.

[1] The two ifs in SequencedTaskRunnerHandle::Get() are necessarily false when
ThreadTaskRunnerHandle::IsSet(). The first if has a DCHECK and I could add one
in the second if.
[2] "TaskScheduler multi-threaded sequence" refers to a sequence of tasks that
may be scheduled on any TaskScheduler thread, in posting order, one at a time.

[grt (UTC plus 2), 2016-12-22 20:44:18]

Thanks for the extra clarification. LGTM.

[fdoray, 2016-12-22 22:10:14]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-12-22 22:10:30]

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-12-22 22:14:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482444614259940,
"parent_rev": "1fad9c66dbc419a624f7bc548cfc6e4a20238554", "commit_rev":
"052082363cae8d6e1e78f96d97c261573bab93a2"}

[commit-bot: I haz the power, 2016-12-22 22:14:52]

Description was changed from

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in 
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631
==========

to

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in 
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631

Review-Url: https://codereview.chromium.org/2598753002
==========

[commit-bot: I haz the power, 2016-12-22 22:14:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-22 22:21:03]

Description was changed from

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in 
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631

Review-Url: https://codereview.chromium.org/2598753002
==========

to

==========
Allow ObjectWatcher to be used from sequenced tasks.

Previously, ObjectWatcher could only be used from single-threaded
tasks. Watchers were notified on the thread from which they were
registered.

With this CL, ObjectWatcher can also be used from sequenced tasks.
Watchers will be notified on the sequence from which they were
registered.

Note that since SequencedTaskRunnerHandle behaves the same way
as ThreadTaskRunnerHandle when used from a single-threaded task
or outside of any task, this CL does not change the behavior of
existing ObjectWatcher::StartWatching*() call sites (those are
necessarily in single-threaded tasks or outside of any task;
otherwise the DCHECK(ThreadTaskRunnerHandle::IsSet()) in
ObjectWatcher::StartWatchingInternal would fail).

BUG=675631

Committed: https://crrev.com/0838b883b2a89bbfa6b1f845e0dfd5765781bc07
Cr-Commit-Position: refs/heads/master@{#440519}
==========

[commit-bot: I haz the power, 2016-12-22 22:21:04]

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