Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2391673002:
Use FileDescriptorWatcher in UserInputMonitorLinuxCore. (Closed)
|
Created:
4 years, 2 months ago by fdoray Modified:
4 years, 2 months ago Reviewers:
liberato (no reviews please) CC:
chromium-reviews, feature-media-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse FileDescriptorWatcher in UserInputMonitorLinuxCore.
This allows UserInputMonitorLinuxCore 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/38bc36cf16d7a663619670854d0c3123a4302102
Cr-Commit-Position: refs/heads/master@{#423159}
Patch Set 1 #Patch Set 2 : fix build error #Patch Set 3 : fix test error #
Total comments: 4
Patch Set 4 : CR liberato #15 #
Messages
Total messages: 26 (19 generated)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
fdoray@chromium.org changed reviewers: + liberato@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... File media/base/user_input_monitor_linux.cc (right): https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... media/base/user_input_monitor_linux.cc:223: watch_controller_ = base::FileDescriptorWatcher::WatchReadable( file_descriptor_watcher_posix mentions that one must have instantiated a FileDescriptorWatcher before calling this. where does that happen? i'm still wrapping my head around FileDescriptorWatcher, so sorry if i've missed the obvious. https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... File media/base/user_input_monitor_unittest.cc (right): https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... media/base/user_input_monitor_unittest.cc:18: #if defined(OS_POSIX) line 55 checks OS_LINUX. probably this should be OS_LINUX as well.
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAnL https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... File media/base/user_input_monitor_linux.cc (right): https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... media/base/user_input_monitor_linux.cc:223: watch_controller_ = base::FileDescriptorWatcher::WatchReadable( On 2016/10/04 17:29:20, liberato wrote: > file_descriptor_watcher_posix mentions that one must have instantiated a > FileDescriptorWatcher before calling this. where does that happen? > > i'm still wrapping my head around FileDescriptorWatcher, so sorry if i've missed > the obvious. A base::Thread that runs a MessageLoopForIO instantiates a FileDescriptorWatcher (e.g. the IO thread). All TaskScheduler threads will instantiate a FileDescriptorWatcher. Unit tests which manage their own MessageLoopForIO may instantiate their own FileDescriptorWatcher (e.g. in this CL: https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m...) https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... File media/base/user_input_monitor_unittest.cc (right): https://codereview.chromium.org/2391673002/diff/40001/media/base/user_input_m... media/base/user_input_monitor_unittest.cc:18: #if defined(OS_POSIX) On 2016/10/04 17:29:20, liberato wrote: > line 55 checks OS_LINUX. probably this should be OS_LINUX as well. Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Use FileDescriptorWatcher in UserInputMonitorLinuxCore. This allows UserInputMonitorLinuxCore 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 UserInputMonitorLinuxCore. This allows UserInputMonitorLinuxCore 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/38bc36cf16d7a663619670854d0c3123a4302102 Cr-Commit-Position: refs/heads/master@{#423159} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/38bc36cf16d7a663619670854d0c3123a4302102 Cr-Commit-Position: refs/heads/master@{#423159} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
