Skip to content

Commit

Permalink
FilePathWatcher: Make UpdateWatch call AddWatcher
Browse files Browse the repository at this point in the history
Every call to UpdateWatch is paired with an AddWatcher. Move the call
into UpdateWatch and get rid of the `watched_handle_` member.

This is in preparation to support WatchWithChangeInfo on Windows.

Bug: 321980447
Change-Id: Idf6985f1bfeb4fb9c28661a4ad0a955524482c0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5479668
Commit-Queue: Nathan Memmott <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1299319}
  • Loading branch information
Nathan Memmott authored and Chromium LUCI CQ committed May 10, 2024
1 parent 9791d5e commit 85226c1
Showing 1 changed file with 11 additions and 27 deletions.
38 changes: 11 additions & 27 deletions base/files/file_path_watcher_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
private:
friend CompletionIOPortThread;

// Sets up a watch handle in `watched_handle_` for either `target_` or one of
// its ancestors. Returns true on success.
// Sets up a watch handle for either `target_` or one of its ancestors.
// Returns true on success.
[[nodiscard]] bool SetupWatchHandleForTarget();

void CloseWatchHandle();
Expand All @@ -217,9 +217,6 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
// Path we're supposed to watch (passed to callback).
FilePath target_;

// Handle for FindFirstChangeNotification.
base::win::ScopedHandle watched_handle_;

std::optional<CompletionIOPortThread::WatcherEntryId> watcher_id_;

// The type of watch requested.
Expand Down Expand Up @@ -397,14 +394,7 @@ bool FilePathWatcherImpl::WatchWithChangeInfo(
first_notification_ = Time::Now();
}

if (!SetupWatchHandleForTarget()) {
return false;
}

watcher_id_ = CompletionIOPortThread::Get()->AddWatcher(
*this, std::move(watched_handle_));

return watcher_id_.has_value();
return SetupWatchHandleForTarget();
}

void FilePathWatcherImpl::Cancel() {
Expand Down Expand Up @@ -486,16 +476,6 @@ void FilePathWatcherImpl::OnObjectSignaled() {
// `this` may be deleted after `callback_` is run.
callback_.Run(FilePathWatcher::ChangeInfo(), target_, /*error=*/false);
}

// The watch may have been cancelled by the callback.
if (self) {
watcher_id_ = CompletionIOPortThread::Get()->AddWatcher(
*this, std::move(watched_handle_));
if (!watcher_id_.has_value()) {
// `this` may be deleted after `callback_` is run.
callback_.Run(FilePathWatcher::ChangeInfo(), target_, /*error=*/true);
}
}
}

bool FilePathWatcherImpl::SetupWatchHandleForTarget() {
Expand All @@ -508,12 +488,13 @@ bool FilePathWatcherImpl::SetupWatchHandleForTarget() {
// child directories stripped from target, in reverse order.
std::vector<FilePath> child_dirs;
FilePath path_to_watch(target_);
base::win::ScopedHandle watched_handle;
while (true) {
auto result = CreateDirectoryHandle(path_to_watch);

// Break if a valid handle is returned.
if (result.has_value()) {
watched_handle_ = std::move(result.value());
watched_handle = std::move(result.value());
break;
}

Expand All @@ -533,7 +514,7 @@ bool FilePathWatcherImpl::SetupWatchHandleForTarget() {
path_to_watch = parent;
}

// At this point, `watched_handle_` is valid. However, the bottom-up search
// At this point, `watched_handle` is valid. However, the bottom-up search
// that the above code performs races against directory creation. So try to
// walk back down and see whether any children appeared in the mean time.
while (!child_dirs.empty()) {
Expand All @@ -549,10 +530,13 @@ bool FilePathWatcherImpl::SetupWatchHandleForTarget() {
// Otherwise go with the current `watched_handle`.
break;
}
watched_handle_ = std::move(result.value());
watched_handle = std::move(result.value());
}

return true;
watcher_id_ = CompletionIOPortThread::Get()->AddWatcher(
*this, std::move(watched_handle));

return watcher_id_.has_value();
}

void FilePathWatcherImpl::CloseWatchHandle() {
Expand Down

0 comments on commit 85226c1

Please sign in to comment.