[Windows] Listen for sub directory changes. (issue 11415066)

3 views
Skip to first unread message

kmad...@chromium.org

unread,
Nov 19, 2012, 9:58:03 PM11/19/12
to mnis...@chromium.org, chromium...@chromium.org, erikwrig...@chromium.org, the...@chromium.org
Reviewers: Mattias Nissler,

Message:
mnissler@: Is there any specific reason why we are not listening for sub
tree
changes? If this change is fine, I will add a unit test in the next patch.

Thanks.

Description:
[Windows] Listen for sub directory changes.

BUG=144491
TEST=none


Please review this at https://codereview.chromium.org/11415066/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M base/files/file_path_watcher_win.cc


Index: base/files/file_path_watcher_win.cc
diff --git a/base/files/file_path_watcher_win.cc
b/base/files/file_path_watcher_win.cc
index
37b1e7b3f9feeaeef4d37558c8e01fa799f91dba..70374fb5b9cc46fcba76ea9de8a05f0a4e40559f
100644
--- a/base/files/file_path_watcher_win.cc
+++ b/base/files/file_path_watcher_win.cc
@@ -182,7 +182,7 @@ bool FilePathWatcherImpl::SetupWatchHandle(const
FilePath& dir,
HANDLE* handle) {
*handle = FindFirstChangeNotification(
dir.value().c_str(),
- false, // Don't watch subtrees
+ true, // Watch subtrees
FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_SIZE |
FILE_NOTIFY_CHANGE_LAST_WRITE | FILE_NOTIFY_CHANGE_DIR_NAME |
FILE_NOTIFY_CHANGE_ATTRIBUTES | FILE_NOTIFY_CHANGE_SECURITY);


the...@chromium.org

unread,
Nov 19, 2012, 10:48:13 PM11/19/12
to kmad...@chromium.org, mnis...@chromium.org, chromium...@chromium.org, erikwrig...@chromium.org
On 2012/11/20 02:58:03, kmadhusu wrote:
> mnissler@: Is there any specific reason why we are not listening for sub
> tree
> changes? If this change is fine, I will add a unit test in the next patch.

> Thanks.

Erm, that's because this code is used to implement FilePathWatcher, and
FilePathWatcher::Watch() is not recursive.

https://codereview.chromium.org/11415066/

the...@chromium.org

unread,
Nov 19, 2012, 10:53:03 PM11/19/12
to kmad...@chromium.org, mnis...@chromium.org, chromium...@chromium.org, erikwrig...@chromium.org
What we want to do is implement FilePathWatcher::RecursiveWatch() or change
FilePathWatcher::Watch() to take a recursive boolean. For unsupported
platforms
/ paths, we would just return false. Thus we can implement this
incrementally,
platform by platform.

On Windows, if someone calls "RecursiveWatch()", only then would we want to
call
FindFirstChangeNotification() and tell it to watch subtrees.

https://codereview.chromium.org/11415066/

mnis...@chromium.org

unread,
Nov 20, 2012, 4:25:17 AM11/20/12
to kmad...@chromium.org, chromium...@chromium.org, erikwrig...@chromium.org
Agree. Alternatively, you could pass a bool recursive flag on construction
time.
I don't care either way. Note that on windows, this is pretty simple (as you
figured), but on Linux and Mac it's going to be rather hard to get this
right
(there are previous failed attempts at that).

https://codereview.chromium.org/11415066/
Reply all
Reply to author
Forward
0 new messages