Move FilePathWatcher class from browser/... to common/...

chrome/common/file_path_watcher/file_path_watcher_mac.cc

Index: chrome/common/file_path_watcher/file_path_watcher_mac.cc
diff --git a/chrome/browser/file_path_watcher/file_path_watcher_mac.cc b/chrome/common/file_path_watcher/file_path_watcher_mac.cc
similarity index 79%
rename from chrome/browser/file_path_watcher/file_path_watcher_mac.cc
rename to chrome/common/file_path_watcher/file_path_watcher_mac.cc
index e89cf872de3a4f77e3145095bd93f6f75968c8e1..1070d8d5ce7f7b5fc9e3f867336bf34b657139c9 100644
--- a/chrome/browser/file_path_watcher/file_path_watcher_mac.cc
+++ b/chrome/common/file_path_watcher/file_path_watcher_mac.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/file_path_watcher/file_path_watcher.h"
+#include "chrome/common/file_path_watcher/file_path_watcher.h"
 
 #include <CoreServices/CoreServices.h>
 #include <set>
@@ -11,9 +11,21 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
+#include "base/message_loop.h"
 #include "base/singleton.h"
 #include "base/time.h"
 
+// Note to future well meaning engineers. Unless kqueue semantics have changed
+// considerably, do NOT try to reimplement this class using kqueue. The main
+// problem is that this class requires the ability to watch a directory
+// and notice changes to any files within it. A kqueue on a directory can watch
+// for creation and deletion of files, but not for modifications to files within
+// the directory. To do this with the current kqueue semantics would require
+// kqueueing every file in the directory, and file descriptors are a limited
+// resource. If you have a good idea on how to get around this, the source for a
+// reasonable implementation of this class using kqueues is attached here:
+// http://code.google.com/p/chromium/issues/detail?id=54822#c13

[dmac, 2011/03/17 17:16:48]

out of curiosity, had you already reached this same conclusion?

[Mattias Nissler (ping if slow), 2011/03/17 18:14:27]

Yes. Sorry I didn't update the bug. IIRC, there was some email thread in which I
mentioned my findings.

+
 namespace {
 
 // The latency parameter passed to FSEventsStreamCreate().
@@ -32,8 +44,14 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
   void UpdateEventStream(FSEventStreamEventId start_event);
 
   // FilePathWatcher::PlatformDelegate overrides.
-  virtual bool Watch(const FilePath& path, FilePathWatcher::Delegate* delegate);
-  virtual void Cancel();
+  virtual bool Watch(const FilePath& path,
+                     FilePathWatcher::Delegate* delegate,
+                     scoped_refptr<base::MessageLoopProxy> loop) OVERRIDE;
+  virtual void Cancel() OVERRIDE;
+
+  scoped_refptr<base::MessageLoopProxy> run_loop_message_loop() {
+    return run_loop_message_loop_;
+  }
 
  private:
   virtual ~FilePathWatcherImpl() {}
@@ -58,6 +76,9 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
   // Backend stream we receive event callbacks from (strong reference).
   FSEventStreamRef fsevent_stream_;
 
+  // Run loop for FSEventStream to run on.
+  scoped_refptr<base::MessageLoopProxy> run_loop_message_loop_;
+
   // Used to detect early cancellation.
   bool canceled_;
 
@@ -69,10 +90,10 @@ void FSEventsCallback(ConstFSEventStreamRef stream,
                       void* event_watcher, size_t num_events,
                       void* event_paths, const FSEventStreamEventFlags flags[],
                       const FSEventStreamEventId event_ids[]) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   FilePathWatcherImpl* watcher =
       reinterpret_cast<FilePathWatcherImpl*>(event_watcher);
+  DCHECK(watcher->run_loop_message_loop()->BelongsToCurrentThread());
+
   bool root_changed = false;
   FSEventStreamEventId root_change_at = FSEventStreamGetLatestEventId(stream);
   for (size_t i = 0; i < num_events; i++) {
@@ -88,12 +109,12 @@ void FSEventsCallback(ConstFSEventStreamRef stream,
   if (root_changed) {
     // Resetting the event stream from within the callback fails (FSEvents spews
     // bad file descriptor errors), so post a task to do the reset.
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+    watcher->run_loop_message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(watcher, &FilePathWatcherImpl::UpdateEventStream,
                           root_change_at));
   }
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+  watcher->message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(watcher, &FilePathWatcherImpl::OnFilePathChanged));
 }
 
@@ -105,7 +126,7 @@ FilePathWatcherImpl::FilePathWatcherImpl()
 }
 
 void FilePathWatcherImpl::OnFilePathChanged() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(message_loop()->BelongsToCurrentThread());
   DCHECK(!target_.empty());
 
   base::PlatformFileInfo file_info;
@@ -143,9 +164,12 @@ void FilePathWatcherImpl::OnFilePathChanged() {
 }
 
 bool FilePathWatcherImpl::Watch(const FilePath& path,
-                                FilePathWatcher::Delegate* delegate) {
+                                FilePathWatcher::Delegate* delegate,
+                                scoped_refptr<base::MessageLoopProxy> loop) {
   DCHECK(target_.value().empty());
+  DCHECK(MessageLoopForIO::current());

[Mattias Nissler (ping if slow), 2011/03/17 10:37:56]

Same question as for the inotify implementation: Why can't we be consistently
using message_loop()->BelongsToCurrentThread()?

[dmac, 2011/03/17 17:16:48]

Same response ;-)

 
+  run_loop_message_loop_ = loop;
   target_ = path;
   delegate_ = delegate;
 
@@ -157,7 +181,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
     first_notification_ = base::Time::Now();
   }
 
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+  run_loop_message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(this, &FilePathWatcherImpl::UpdateEventStream,
                         start_event));
 
@@ -166,8 +190,9 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
 
 void FilePathWatcherImpl::Cancel() {
   // Switch to the UI thread if necessary, so we can tear down the event stream.
-  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+  if (run_loop_message_loop().get() &&

[Mattias Nissler (ping if slow), 2011/03/17 10:37:56]

I guess you need the NULL check since Cancel() can be called even if Watch() was
never called, correct? But shouldn't we just bail out in that case, since there
is no reason to run the cleanup? In any case, a comment explaining the situation
would probably be helpful.

[dmac, 2011/03/17 17:16:48]

Done.

+      !run_loop_message_loop()->BelongsToCurrentThread()) {
+    run_loop_message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(this, &FilePathWatcherImpl::Cancel));
     return;
   }
@@ -178,7 +203,8 @@ void FilePathWatcherImpl::Cancel() {
 }
 
 void FilePathWatcherImpl::UpdateEventStream(FSEventStreamEventId start_event) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(run_loop_message_loop()->BelongsToCurrentThread());
+  DCHECK(MessageLoopForUI::current());

[Mattias Nissler (ping if slow), 2011/03/17 10:37:56]

It seems that we can do the message loop type check at initialization time.

[dmac, 2011/03/17 17:16:48]

We can't actually (unless I'm missing something) because there doesn't appear to
be anyway of getting the type of a MessageLoopProxy.

 
   // It can happen that the watcher gets canceled while tasks that call this
   // function are still in flight, so abort if this situation is detected.
@@ -212,14 +238,14 @@ void FilePathWatcherImpl::UpdateEventStream(FSEventStreamEventId start_event) {
   FSEventStreamScheduleWithRunLoop(fsevent_stream_, CFRunLoopGetCurrent(),
                                    kCFRunLoopDefaultMode);
   if (!FSEventStreamStart(fsevent_stream_)) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+    message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(delegate_.get(),
                           &FilePathWatcher::Delegate::OnError));
   }
 }
 
 void FilePathWatcherImpl::DestroyEventStream() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(run_loop_message_loop()->BelongsToCurrentThread());
   FSEventStreamStop(fsevent_stream_);
   FSEventStreamUnscheduleFromRunLoop(fsevent_stream_, CFRunLoopGetCurrent(),
                                      kCFRunLoopDefaultMode);