Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2764)

Unified Diff: base/files/file_path_watcher_linux.cc

Issue 2596273003: Remove ref-counting from FilePathWatcher. (Closed)
Patch Set: fix mac build error Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/files/file_path_watcher_kqueue.cc ('k') | base/files/file_path_watcher_mac.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/files/file_path_watcher_linux.cc
diff --git a/base/files/file_path_watcher_linux.cc b/base/files/file_path_watcher_linux.cc
index d060e49136cbc88dc095b5872d148607af501d39..9589e9b788dce7807ac0b97a011e6012e6b47860 100644
--- a/base/files/file_path_watcher_linux.cc
+++ b/base/files/file_path_watcher_linux.cc
@@ -28,6 +28,8 @@
#include "base/location.h"
#include "base/logging.h"
#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
#include "base/posix/eintr_wrapper.h"
#include "base/single_thread_task_runner.h"
#include "base/stl_util.h"
@@ -91,6 +93,7 @@ class InotifyReader {
class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
public:
FilePathWatcherImpl();
+ ~FilePathWatcherImpl() override;
// Called for each event coming from the watch. |fired_watch| identifies the
// watch that fired, |child| indicates what has changed, and is relative to
@@ -105,13 +108,13 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
bool deleted,
bool is_dir);
- protected:
- ~FilePathWatcherImpl() override {
- in_destructor_ = true;
- CancelOnMessageLoopThreadOrInDestructor();
- }
-
private:
+ void OnFilePathChangedOnOriginSequence(InotifyReader::Watch fired_watch,
+ const FilePath::StringType& child,
+ bool created,
+ bool deleted,
+ bool is_dir);
+
// Start watching |path| for changes and notify |delegate| on each change.
// Returns true if watch for |path| has been added successfully.
bool Watch(const FilePath& path,
@@ -120,7 +123,6 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
// Cancel the watch. This unregisters the instance with InotifyReader.
void Cancel() override;
- void CancelOnMessageLoopThreadOrInDestructor();
// Inotify watches are installed for all directory components of |target_|.
// A WatchEntry instance holds:
@@ -185,7 +187,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
hash_map<InotifyReader::Watch, FilePath> recursive_paths_by_watch_;
std::map<FilePath, InotifyReader::Watch> recursive_watches_by_path_;
- bool in_destructor_ = false;
+ WeakPtrFactory<FilePathWatcherImpl> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl);
};
@@ -312,7 +314,10 @@ void InotifyReader::OnInotifyEvent(const inotify_event* event) {
}
FilePathWatcherImpl::FilePathWatcherImpl()
- : recursive_(false) {
+ : recursive_(false), weak_factory_(this) {}
+
+FilePathWatcherImpl::~FilePathWatcherImpl() {
+ DCHECK(!task_runner() || task_runner()->RunsTasksOnCurrentThread());
}
void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
@@ -320,22 +325,25 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
bool created,
bool deleted,
bool is_dir) {
- if (!task_runner()->RunsTasksOnCurrentThread()) {
- // Switch to task_runner() to access |watches_| safely.
- task_runner()->PostTask(FROM_HERE,
- Bind(&FilePathWatcherImpl::OnFilePathChanged, this,
- fired_watch, child, created, deleted, is_dir));
- return;
- }
-
- // Check to see if CancelOnMessageLoopThreadOrInDestructor() has already been
- // called. May happen when code flow reaches here from the PostTask() above.
- if (watches_.empty()) {
- DCHECK(target_.empty());
- return;
- }
+ DCHECK(!task_runner()->RunsTasksOnCurrentThread());
+
+ // This method is invoked on the Inotify thread. Switch to task_runner() to
+ // access |watches_| safely. Use a WeakPtr to prevent the callback from
+ // running after |this| is destroyed (i.e. after the watch is cancelled).
+ task_runner()->PostTask(
+ FROM_HERE, Bind(&FilePathWatcherImpl::OnFilePathChangedOnOriginSequence,
+ weak_factory_.GetWeakPtr(), fired_watch, child, created,
+ deleted, is_dir));
+}
+void FilePathWatcherImpl::OnFilePathChangedOnOriginSequence(
+ InotifyReader::Watch fired_watch,
+ const FilePath::StringType& child,
+ bool created,
+ bool deleted,
+ bool is_dir) {
DCHECK(task_runner()->RunsTasksOnCurrentThread());
+ DCHECK(!watches_.empty());
DCHECK(HasValidWatchVector());
// Used below to avoid multiple recursive updates.
@@ -437,41 +445,23 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
}
void FilePathWatcherImpl::Cancel() {
- if (callback_.is_null()) {
- // Watch was never called, or the message_loop() thread is already gone.
+ if (!callback_) {
+ // Watch() was never called.
set_cancelled();
return;
}
- // Switch to the task_runner() if necessary so we can access |watches_|.
- if (!task_runner()->RunsTasksOnCurrentThread()) {
- task_runner()->PostTask(
- FROM_HERE,
- Bind(&FilePathWatcherImpl::CancelOnMessageLoopThreadOrInDestructor,
- this));
- } else {
- CancelOnMessageLoopThreadOrInDestructor();
- }
-}
-
-void FilePathWatcherImpl::CancelOnMessageLoopThreadOrInDestructor() {
- DCHECK(in_destructor_ || task_runner()->RunsTasksOnCurrentThread());
-
- if (is_cancelled())
- return;
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
+ DCHECK(!is_cancelled());
set_cancelled();
-
- if (!callback_.is_null())
- callback_.Reset();
+ callback_.Reset();
for (size_t i = 0; i < watches_.size(); ++i)
g_inotify_reader.Get().RemoveWatch(watches_[i].watch, this);
watches_.clear();
target_.clear();
-
- if (recursive_)
- RemoveRecursiveWatches();
+ RemoveRecursiveWatches();
}
void FilePathWatcherImpl::UpdateWatches() {
@@ -657,7 +647,7 @@ bool FilePathWatcherImpl::HasValidWatchVector() const {
FilePathWatcher::FilePathWatcher() {
sequence_checker_.DetachFromSequence();
- impl_ = new FilePathWatcherImpl();
+ impl_ = MakeUnique<FilePathWatcherImpl>();
}
} // namespace base
« no previous file with comments | « base/files/file_path_watcher_kqueue.cc ('k') | base/files/file_path_watcher_mac.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698