Added a base::Callback interface to FilePathWatcher.

BUG=130980
TEST=browser_tests:FilePathWatcher* are green

Review URL: https://chromiumcodereview.appspot.com/10519003

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

4 files changed, 113 insertions, 2 deletions

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 0bb8ac8..71342f1 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -200,6 +200,23 @@ def _CheckNoScopedAllowIO(input_api, output_api):
       'instead.\n' + '\n'.join(problems))]
 
 
+def _CheckNoFilePathWatcherDelegate(input_api, output_api):
+  """Make sure that FilePathWatcher::Delegate is not used."""
+  problems = []
+
+  file_filter = lambda f: f.LocalPath().endswith(('.cc', '.h'))
+  for f in input_api.AffectedFiles(file_filter=file_filter):
+    for line_num, line in f.ChangedContents():
+      if 'FilePathWatcher::Delegate' in line:
+        problems.append('    %s:%d' % (f.LocalPath(), line_num))
+
+  if not problems:
+    return []
+  return [output_api.PresubmitPromptWarning('New code should not use '
+      'FilePathWatcher::Delegate. Use the callback interface instead.\n' +
+      '\n'.join(problems))]
+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
@@ -214,6 +231,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
   results.extend(_CheckNoScopedAllowIO(input_api, output_api))
+  results.extend(_CheckNoFilePathWatcherDelegate(input_api, output_api))
   return results
 
 
diff --git a/base/files/file_path_watcher.cc b/base/files/file_path_watcher.cc
index 086da1e..dd2b37f 100644
--- a/base/files/file_path_watcher.cc
+++ b/base/files/file_path_watcher.cc
@@ -13,6 +13,33 @@
 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();
 }
@@ -35,5 +62,9 @@ FilePathWatcher::PlatformDelegate::~PlatformDelegate() {
   DCHECK(is_cancelled());
 }
 
+bool FilePathWatcher::Watch(const FilePath& path, const Callback& callback) {
+  return Watch(path, new FilePathWatcherDelegate(callback));
+}
+
 }  // namespace files
 }  // namespace base
diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index 3795c9e..5919994 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -10,6 +10,7 @@
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop_proxy.h"
@@ -28,9 +29,16 @@ namespace files {
 // details.
 class BASE_EXPORT FilePathWatcher {
  public:
+  // Callback type for Watch(). |path| points to the file that was updated,
+  // and |error| is true if the platform specific code detected an error. In
+  // 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;
@@ -104,9 +112,18 @@ class BASE_EXPORT FilePathWatcher {
   // 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. The callback will
+  // be invoked on the same loop. Returns true on success.
+  bool Watch(const FilePath& path, const Callback& callback);
+
  private:
   scoped_refptr<PlatformDelegate> impl_;
 
diff --git a/base/files/file_path_watcher_browsertest.cc b/base/files/file_path_watcher_browsertest.cc
index 7886df8..5eea88c5 100644
--- a/base/files/file_path_watcher_browsertest.cc
+++ b/base/files/file_path_watcher_browsertest.cc
@@ -119,7 +119,7 @@ class TestDelegate : public FilePathWatcher::Delegate {
   DISALLOW_COPY_AND_ASSIGN(TestDelegate);
 };
 
-void SetupWatchCallback(const FilePath& target,
+void SetupWatchDelegate(const FilePath& target,
                         FilePathWatcher* watcher,
                         FilePathWatcher::Delegate* delegate,
                         bool* result,
@@ -128,6 +128,25 @@ void SetupWatchCallback(const FilePath& target,
   completion->Signal();
 }
 
+void SetupWatchCallback(const FilePath& target,
+                        FilePathWatcher* watcher,
+                        const FilePathWatcher::Callback& callback) {
+  ASSERT_TRUE(watcher->Watch(target, callback));
+}
+
+void QuitLoopWatchCallback(MessageLoop* loop,
+                           const FilePath& expected_path,
+                           bool expected_error,
+                           bool* flag,
+                           const FilePath& path,
+                           bool error) {
+  ASSERT_TRUE(flag);
+  *flag = true;
+  EXPECT_EQ(expected_path, path);
+  EXPECT_EQ(expected_error, error);
+  loop->PostTask(FROM_HERE, loop->QuitClosure());
+}
+
 class FilePathWatcherTest : public testing::Test {
  public:
   FilePathWatcherTest()
@@ -170,7 +189,7 @@ class FilePathWatcherTest : public testing::Test {
     bool result;
     file_thread_.message_loop_proxy()->PostTask(
         FROM_HERE,
-        base::Bind(SetupWatchCallback, target, watcher,
+        base::Bind(SetupWatchDelegate, target, watcher,
                    make_scoped_refptr(delegate), &result, &completion));
     completion.Wait();
     return result;
@@ -239,6 +258,32 @@ TEST_F(FilePathWatcherTest, DeletedFile) {
   ASSERT_TRUE(WaitForEvents());
 }
 
+TEST_F(FilePathWatcherTest, Callback) {
+  FilePathWatcher watcher;
+  bool called_back = false;
+
+  MessageLoop* file_loop = file_thread_.message_loop();
+  ASSERT_TRUE(file_loop);
+  // The callback makes |loop_| quit on file events, and flips |called_back|
+  // to true.
+  FilePathWatcher::Callback callback = base::Bind(
+      QuitLoopWatchCallback, &loop_, test_file(), false, &called_back);
+
+  // Start watching on the file thread, and unblock the loop once the callback
+  // has been installed.
+  file_thread_.message_loop_proxy()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(SetupWatchCallback, test_file(), &watcher, callback),
+      base::Bind(&MessageLoop::Quit, base::Unretained(&loop_)));
+  loop_.Run();
+
+  // The watch has been installed. Trigger a file event now, which will unblock
+  // the loop again.
+  ASSERT_TRUE(WriteFile(test_file(), "content"));
+  loop_.Run();
+  EXPECT_TRUE(called_back);
+}
+
 // Used by the DeleteDuringNotify test below.
 // Deletes the FilePathWatcher when it's notified.
 class Deleter : public FilePathWatcher::Delegate {