Eliminate FilePathWatcher::Delegate.

BUG=130980

Review URL: https://codereview.chromium.org/11876025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176768 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 38 insertions, 100 deletions

diff --git a/base/files/file_path_watcher.cc b/base/files/file_path_watcher.cc
index bc8db37..3802ee3 100644
--- a/base/files/file_path_watcher.cc
+++ b/base/files/file_path_watcher.cc
@@ -13,33 +13,6 @@
 namespace base {
 namespace files {
 
-namespace {
-
-// A delegate implementation for the callback interface.
-class FilePathWatcherDelegate : public base::files::FilePathWatcher::Delegate {
- public:
-  explicit FilePathWatcherDelegate(const FilePathWatcher::Callback& callback)
-      : callback_(callback) {}
-
-  // FilePathWatcher::Delegate implementation.
-  virtual void OnFilePathChanged(const FilePath& path) OVERRIDE {
-    callback_.Run(path, false);
-  }
-
-  virtual void OnFilePathError(const FilePath& path) OVERRIDE {
-    callback_.Run(path, true);
-  }
-
- private:
-  virtual ~FilePathWatcherDelegate() {}
-
-  FilePathWatcher::Callback callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(FilePathWatcherDelegate);
-};
-
-}  // namespace
-
 FilePathWatcher::~FilePathWatcher() {
   impl_->Cancel();
 }
@@ -50,11 +23,6 @@ void FilePathWatcher::CancelWatch(
   delegate->CancelOnMessageLoopThread();
 }
 
-bool FilePathWatcher::Watch(const FilePath& path, Delegate* delegate) {
-  DCHECK(path.IsAbsolute());
-  return impl_->Watch(path, false, delegate);
-}
-
 FilePathWatcher::PlatformDelegate::PlatformDelegate(): cancelled_(false) {
 }
 
@@ -66,7 +34,7 @@ bool FilePathWatcher::Watch(const FilePath& path,
                             bool recursive,
                             const Callback& callback) {
   DCHECK(path.IsAbsolute());
-  return impl_->Watch(path, recursive, new FilePathWatcherDelegate(callback));
+  return impl_->Watch(path, recursive, callback);
 }
 
 }  // namespace files
diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index 94a3f9a..57b875a 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -33,23 +33,6 @@ class BASE_EXPORT FilePathWatcher {
   // that case, the callback won't be invoked again.
   typedef base::Callback<void(const FilePath& path, bool error)> Callback;
 
-  // Declares the callback client code implements to receive notifications. Note
-  // that implementations of this interface should not keep a reference to the
-  // corresponding FileWatcher object to prevent a reference cycle.
-  //
-  // Deprecated: see comment on Watch() below.
-  class Delegate : public base::RefCountedThreadSafe<Delegate> {
-   public:
-    virtual void OnFilePathChanged(const FilePath& path) = 0;
-    // Called when platform specific code detected an error. The watcher will
-    // not call OnFilePathChanged for future changes.
-    virtual void OnFilePathError(const FilePath& path) {}
-
-   protected:
-    friend class base::RefCountedThreadSafe<Delegate>;
-    virtual ~Delegate() {}
-  };
-
   // Used internally to encapsulate different members on different platforms.
   // TODO(jhawkins): Move this into its own file. Also fix the confusing naming
   // wrt Delegate vs PlatformDelegate.
@@ -60,7 +43,7 @@ class BASE_EXPORT FilePathWatcher {
     // Start watching for the given |path| and notify |delegate| about changes.
     virtual bool Watch(const FilePath& path,
                        bool recursive,
-                       Delegate* delegate) WARN_UNUSED_RESULT = 0;
+                       const Callback& callback) WARN_UNUSED_RESULT = 0;
 
     // Stop watching. This is called from FilePathWatcher's dtor in order to
     // allow to shut down properly while the object is still alive.
@@ -108,17 +91,6 @@ class BASE_EXPORT FilePathWatcher {
   // shutdown.
   static void CancelWatch(const scoped_refptr<PlatformDelegate>& delegate);
 
-  // Register interest in any changes on |path|. OnPathChanged will be called
-  // back for each change. Returns true on success.
-  // OnFilePathChanged() will be called on the same thread as Watch() is called,
-  // which should have a MessageLoop of TYPE_IO.
-  //
-  // Deprecated: new code should use the callback interface, declared below.
-  // The FilePathWatcher::Delegate interface will be removed once all client
-  // code has been updated. http://crbug.com/130980
-  virtual bool Watch(const FilePath& path, Delegate* delegate)
-      WARN_UNUSED_RESULT;
-
   // Invokes |callback| whenever updates to |path| are detected. This should be
   // called at most once, and from a MessageLoop of TYPE_IO. Set |recursive| to
   // true, to watch |path| and its children. The callback will be invoked on
diff --git a/base/files/file_path_watcher_kqueue.cc b/base/files/file_path_watcher_kqueue.cc
index aebe15a..72c41f1 100644
--- a/base/files/file_path_watcher_kqueue.cc
+++ b/base/files/file_path_watcher_kqueue.cc
@@ -66,7 +66,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   // FilePathWatcher::PlatformDelegate overrides.
   virtual bool Watch(const FilePath& path,
                      bool recursive,
-                     FilePathWatcher::Delegate* delegate) OVERRIDE;
+                     const FilePathWatcher::Callback& callback) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
  protected:
@@ -141,7 +141,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   EventVector events_;
   scoped_refptr<base::MessageLoopProxy> io_message_loop_;
   MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_;
-  scoped_refptr<FilePathWatcher::Delegate> delegate_;
+  FilePathWatcher::Callback callback_;
   FilePath target_;
   int kqueue_;
 
@@ -364,7 +364,7 @@ void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) {
   // Error values are stored within updates, so check to make sure that no
   // errors occurred.
   if (!AreKeventValuesValid(&updates[0], count)) {
-    delegate_->OnFilePathError(target_);
+    callback_.Run(target_, true /* error */);
     Cancel();
     return;
   }
@@ -411,13 +411,13 @@ void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) {
 
   if (update_watches) {
     if (!UpdateWatches(&send_notification)) {
-      delegate_->OnFilePathError(target_);
+      callback_.Run(target_, true /* error */);
       Cancel();
     }
   }
 
   if (send_notification) {
-    delegate_->OnFilePathChanged(target_);
+    callback_.Run(target_, false);
   }
 }
 
@@ -431,10 +431,10 @@ void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() {
 
 bool FilePathWatcherImpl::Watch(const FilePath& path,
                                 bool recursive,
-                                FilePathWatcher::Delegate* delegate) {
+                                const FilePathWatcher::Callback& callback) {
   DCHECK(MessageLoopForIO::current());
   DCHECK(target_.value().empty());  // Can only watch one path.
-  DCHECK(delegate);
+  DCHECK(!callback.is_null());
   DCHECK_EQ(kqueue_, -1);
 
   if (recursive) {
@@ -443,7 +443,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
     return false;
   }
 
-  delegate_ = delegate;
+  callback_ = callback;
   target_ = path;
 
   MessageLoop::current()->AddDestructionObserver(this);
@@ -499,7 +499,7 @@ void FilePathWatcherImpl::CancelOnMessageLoopThread() {
     events_.clear();
     io_message_loop_ = NULL;
     MessageLoop::current()->RemoveDestructionObserver(this);
-    delegate_ = NULL;
+    callback_.Reset();
   }
 }
 
diff --git a/base/files/file_path_watcher_linux.cc b/base/files/file_path_watcher_linux.cc
index 9e55022..690ac6d 100644
--- a/base/files/file_path_watcher_linux.cc
+++ b/base/files/file_path_watcher_linux.cc
@@ -101,7 +101,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   // Returns true if watch for |path| has been added successfully.
   virtual bool Watch(const FilePath& path,
                      bool recursive,
-                     FilePathWatcher::Delegate* delegate) OVERRIDE;
+                     const FilePathWatcher::Callback& callback) OVERRIDE;
 
   // Cancel the watch. This unregisters the instance with InotifyReader.
   virtual void Cancel() OVERRIDE;
@@ -137,8 +137,8 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   // that exists. Updates |watched_path_|. Returns true on success.
   bool UpdateWatches() WARN_UNUSED_RESULT;
 
-  // Delegate to notify upon changes.
-  scoped_refptr<FilePathWatcher::Delegate> delegate_;
+  // Callback to notify upon changes.
+  FilePathWatcher::Callback callback_;
 
   // The file or directory we're supposed to watch.
   FilePath target_;
@@ -295,8 +295,7 @@ void InotifyReader::OnInotifyEvent(const inotify_event* event) {
   }
 }
 
-FilePathWatcherImpl::FilePathWatcherImpl()
-    : delegate_(NULL) {
+FilePathWatcherImpl::FilePathWatcherImpl() {
 }
 
 void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
@@ -339,7 +338,7 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
       // IN_ISDIR set in the event masks. As a result we may sometimes
       // call UpdateWatches() unnecessarily.
       if (change_on_target_path && !UpdateWatches()) {
-        delegate_->OnFilePathError(target_);
+        callback_.Run(target_, true /* error */);
         return;
       }
 
@@ -354,7 +353,7 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
       if (target_changed ||
           (change_on_target_path && !created) ||
           (change_on_target_path && file_util::PathExists(target_))) {
-        delegate_->OnFilePathChanged(target_);
+        callback_.Run(target_, false);
         return;
       }
     }
@@ -363,7 +362,7 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
 
 bool FilePathWatcherImpl::Watch(const FilePath& path,
                                 bool recursive,
-                                FilePathWatcher::Delegate* delegate) {
+                                const FilePathWatcher::Callback& callback) {
   DCHECK(target_.empty());
   DCHECK(MessageLoopForIO::current());
   if (recursive) {
@@ -373,7 +372,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
   }
 
   set_message_loop(base::MessageLoopProxy::current());
-  delegate_ = delegate;
+  callback_ = callback;
   target_ = path;
   MessageLoop::current()->AddDestructionObserver(this);
 
@@ -390,7 +389,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
 }
 
 void FilePathWatcherImpl::Cancel() {
-  if (!delegate_) {
+  if (callback_.is_null()) {
     // Watch was never called, or the |message_loop_| thread is already gone.
     set_cancelled();
     return;
@@ -410,9 +409,9 @@ void FilePathWatcherImpl::CancelOnMessageLoopThread() {
   if (!is_cancelled())
     set_cancelled();
 
-  if (delegate_) {
+  if (!callback_.is_null()) {
     MessageLoop::current()->RemoveDestructionObserver(this);
-    delegate_ = NULL;
+    callback_.Reset();
   }
 
   for (WatchVector::iterator watch_entry(watches_.begin());
diff --git a/base/files/file_path_watcher_stub.cc b/base/files/file_path_watcher_stub.cc
index 0c25d7f..fc3001f 100644
--- a/base/files/file_path_watcher_stub.cc
+++ b/base/files/file_path_watcher_stub.cc
@@ -16,7 +16,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
  public:
   virtual bool Watch(const FilePath& path,
                      bool recursive,
-                     FilePathWatcher::Delegate* delegate) OVERRIDE {
+                     const FilePathWatcher::Callback& callback) OVERRIDE {
     return false;
   }
 
diff --git a/base/files/file_path_watcher_win.cc b/base/files/file_path_watcher_win.cc
index c25260c..4df9d0d 100644
--- a/base/files/file_path_watcher_win.cc
+++ b/base/files/file_path_watcher_win.cc
@@ -23,14 +23,13 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                             public MessageLoop::DestructionObserver {
  public:
   FilePathWatcherImpl()
-      : delegate_(NULL),
-        handle_(INVALID_HANDLE_VALUE),
+      : handle_(INVALID_HANDLE_VALUE),
         recursive_watch_(false) {}
 
   // FilePathWatcher::PlatformDelegate overrides.
   virtual bool Watch(const FilePath& path,
                      bool recursive,
-                     FilePathWatcher::Delegate* delegate) OVERRIDE;
+                     const FilePathWatcher::Callback& callback) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
   // Deletion of the FilePathWatcher will call Cancel() to dispose of this
@@ -61,10 +60,10 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   // Cleans up and stops observing the |message_loop_| thread.
   void CancelOnMessageLoopThread() OVERRIDE;
 
-  // Delegate to notify upon changes.
-  scoped_refptr<FilePathWatcher::Delegate> delegate_;
+  // Callback to notify upon changes.
+  FilePathWatcher::Callback callback_;
 
-  // Path we're supposed to watch (passed to delegate).
+  // Path we're supposed to watch (passed to callback).
   FilePath target_;
 
   // Handle for FindFirstChangeNotification.
@@ -89,11 +88,11 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
 
 bool FilePathWatcherImpl::Watch(const FilePath& path,
                                 bool recursive,
-                                FilePathWatcher::Delegate* delegate) {
+                                const FilePathWatcher::Callback& callback) {
   DCHECK(target_.value().empty());  // Can only watch one path.
 
   set_message_loop(base::MessageLoopProxy::current());
-  delegate_ = delegate;
+  callback_ = callback;
   target_ = path;
   recursive_watch_ = recursive;
   MessageLoop::current()->AddDestructionObserver(this);
@@ -107,7 +106,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
 }
 
 void FilePathWatcherImpl::Cancel() {
-  if (!delegate_) {
+  if (callback_.is_null()) {
     // Watch was never called, or the |message_loop_| has already quit.
     set_cancelled();
     return;
@@ -129,9 +128,9 @@ void FilePathWatcherImpl::CancelOnMessageLoopThread() {
   if (handle_ != INVALID_HANDLE_VALUE)
     DestroyWatch();
 
-  if (delegate_) {
+  if (!callback_.is_null()) {
     MessageLoop::current()->RemoveDestructionObserver(this);
-    delegate_ = NULL;
+    callback_.Reset();
   }
 }
 
@@ -145,18 +144,18 @@ void FilePathWatcherImpl::OnObjectSignaled(HANDLE object) {
   scoped_refptr<FilePathWatcherImpl> keep_alive(this);
 
   if (!UpdateWatch()) {
-    delegate_->OnFilePathError(target_);
+    callback_.Run(target_, true /* error */);
     return;
   }
 
-  // Check whether the event applies to |target_| and notify the delegate.
+  // Check whether the event applies to |target_| and notify the callback.
   base::PlatformFileInfo file_info;
   bool file_exists = file_util::GetFileInfo(target_, &file_info);
   if (file_exists && (last_modified_.is_null() ||
       last_modified_ != file_info.last_modified)) {
     last_modified_ = file_info.last_modified;
     first_notification_ = base::Time::Now();
-    delegate_->OnFilePathChanged(target_);
+    callback_.Run(target_, false);
   } else if (file_exists && !first_notification_.is_null()) {
     // The target's last modification time is equal to what's on record. This
     // means that either an unrelated event occurred, or the target changed
@@ -177,10 +176,10 @@ void FilePathWatcherImpl::OnObjectSignaled(HANDLE object) {
       // Stop further notifications for this |last_modification_| time stamp.
       first_notification_ = base::Time();
     }
-    delegate_->OnFilePathChanged(target_);
+    callback_.Run(target_, false);
   } else if (!file_exists && !last_modified_.is_null()) {
     last_modified_ = base::Time();
-    delegate_->OnFilePathChanged(target_);
+    callback_.Run(target_, false);
   }
 
   // The watch may have been cancelled by the callback.