Use FileDescriptorWatcher in FilePathWatcherKQueue.

Use FileDescriptorWatcher in FilePathWatcherKQueue.

This allows FilePathWatcherKQueue to be used from any thread that
instantiates a FileDescriptorWatcher (not just threads that run
a MessageLoopForIO). This will facilitate the migration of
BrowserThreads to base/task_scheduler.

BUG=645114
Committed: https://crrev.com/a0e4a7ead3bae88770f4352e2dd927998238b9ee
Cr-Commit-Position: refs/heads/master@{#422243}

[fdoray, 2016-09-20 18:07:58]

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

[commit-bot: I haz the power, 2016-09-20 18:08:19]

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-09-20 19:33:00]

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

[commit-bot: I haz the power, 2016-09-20 19:33:02]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[fdoray, 2016-09-20 19:41:16]

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

[fdoray, 2016-09-20 19:41:17]

PTAL

[dcheng, 2016-09-20 23:22:04]

LGTM with comment addressed.

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
File base/files/file_path_watcher_kqueue.cc (right):

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
base/files/file_path_watcher_kqueue.cc:275:
Bind(&FilePathWatcherKQueue::OnKQueueReadable, Unretained(this)));
|this| is refcounted, so maybe just use this (and the automatic refcounting)
directly?

[fdoray, 2016-09-26 14:32:02]

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
File base/files/file_path_watcher_kqueue.cc (right):

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
base/files/file_path_watcher_kqueue.cc:275:
Bind(&FilePathWatcherKQueue::OnKQueueReadable, Unretained(this)));
On 2016/09/20 23:22:04, dcheng wrote:
> |this| is refcounted, so maybe just use this (and the automatic refcounting)
> directly?

Done.

Note that this creates an ownership cycle (FilePathWatcherKQueue owns
FileDescriptorWatcher::Controller, which owns FilePathWatcherKQueue through the
callback). I think it's ok since the cycle is broken when
CancelOnMessageLoopThread() is called (line 368).

[fdoray, 2016-09-26 14:32:35]

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

[commit-bot: I haz the power, 2016-09-26 14:32:58]

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-09-26 14:43:11]

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

[commit-bot: I haz the power, 2016-09-26 14:43:12]

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

[fdoray, 2016-09-26 14:51:39]

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

[commit-bot: I haz the power, 2016-09-26 14:52:11]

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-09-26 15:04:29]

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

[commit-bot: I haz the power, 2016-09-26 15:04:29]

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

[fdoray, 2016-09-28 19:06:58]

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

[commit-bot: I haz the power, 2016-09-28 19:07:26]

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-09-28 20:05:01]

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

[commit-bot: I haz the power, 2016-09-28 20:05:03]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dcheng, 2016-09-29 18:06:53]

Still LGTM

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
File base/files/file_path_watcher_kqueue.cc (right):

https://codereview.chromium.org/2353103002/diff/1/base/files/file_path_watche...
base/files/file_path_watcher_kqueue.cc:275:
Bind(&FilePathWatcherKQueue::OnKQueueReadable, Unretained(this)));
On 2016/09/26 14:32:02, fdoray wrote:
> On 2016/09/20 23:22:04, dcheng wrote:
> > |this| is refcounted, so maybe just use this (and the automatic refcounting)
> > directly?
> 
> Done.
> 
> Note that this creates an ownership cycle (FilePathWatcherKQueue owns
> FileDescriptorWatcher::Controller, which owns FilePathWatcherKQueue through
the
> callback). I think it's ok since the cycle is broken when
> CancelOnMessageLoopThread() is called (line 368).

I'm OK with it either way, I guess. It's just what the original code did.

Either way is slightly surprising (at first glance), so maybe throw a comment on
here whichever way you choose.

[fdoray, 2016-09-29 18:20:49]

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

[fdoray, 2016-09-29 18:20:49]

pastarmovj@chromium.org: PTAL at components/policy

[pastarmovj, 2016-09-30 08:01:38]

lgtm

[fdoray, 2016-09-30 20:35:48]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-09-30 20:36:27]

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-09-30 21:07:39]

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

[commit-bot: I haz the power, 2016-09-30 21:07:40]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[fdoray, 2016-09-30 21:27:41]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-09-30 21:27:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
pastarmovj@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2353103002/#ps160001
(title: "fix windows build error")

[commit-bot: I haz the power, 2016-09-30 21:28:16]

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-09-30 22:27:51]

Committed patchset #9 (id:160001)

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

Description was changed from

==========
Use FileDescriptorWatcher in FilePathWatcherKQueue.

This allows FilePathWatcherKQueue to be used from any thread that
instantiates a FileDescriptorWatcher (not just threads that run
a MessageLoopForIO). This will facilitate the migration of
BrowserThreads to base/task_scheduler.

BUG=645114
==========

to

==========
Use FileDescriptorWatcher in FilePathWatcherKQueue.

This allows FilePathWatcherKQueue to be used from any thread that
instantiates a FileDescriptorWatcher (not just threads that run
a MessageLoopForIO). This will facilitate the migration of
BrowserThreads to base/task_scheduler.

BUG=645114

Committed: https://crrev.com/a0e4a7ead3bae88770f4352e2dd927998238b9ee
Cr-Commit-Position: refs/heads/master@{#422243}
==========

[commit-bot: I haz the power, 2016-09-30 22:31:13]

Patchset 9 (id:??) landed as
https://crrev.com/a0e4a7ead3bae88770f4352e2dd927998238b9ee
Cr-Commit-Position: refs/heads/master@{#422243}