Fixed shutdown concurrency issues in FilePathWatcher.

content/common/file_path_watcher/file_path_watcher.h

Index: content/common/file_path_watcher/file_path_watcher.h
diff --git a/content/common/file_path_watcher/file_path_watcher.h b/content/common/file_path_watcher/file_path_watcher.h
index 7c784a8bc44fa9777ace5c93f5d1957069a2ee85..940e948bed67ebe7b38dd21b57f6e74c98980c22 100644
--- a/content/common/file_path_watcher/file_path_watcher.h
+++ b/content/common/file_path_watcher/file_path_watcher.h
@@ -40,22 +40,33 @@ class FilePathWatcher {
   // by the Mac implementation right now, and must be backed by a CFRunLoop
   // based MessagePump. This is usually going to be a MessageLoop of type
   // TYPE_UI.
+  // OnFilePathChanged() will be called on the same thread as Watch() is called,
+  // which should have a MessageLoop of TYPE_IO.
   bool Watch(const FilePath& path,
              Delegate* delegate,
              base::MessageLoopProxy* loop) WARN_UNUSED_RESULT;
 
   class PlatformDelegate;
 
-  // Traits for PlatformDelegate, which must delete itself on the IO message
-  // loop that Watch was called from.
-  struct DeletePlatformDelegate {
-    static void Destruct(const PlatformDelegate* delegate);
+  // A custom Task that always cleans up the PlatformDelegate, either when
+  // executed or when deleted without having been executed at all, as can
+  // happen during shutdown.
+  class CancelTask: public Task {

[Mattias Nissler (ping if slow), 2011/03/22 17:23:44]

missing space before :

[Joao da Silva, 2011/03/22 17:42:33]

Done.

+   public:
+    CancelTask(PlatformDelegate* delegate): delegate_(delegate) {}
+    virtual ~CancelTask() {
+      delegate_->CancelOnMessageLoopThread();
+    }
+
+    virtual void Run() {
+      delegate_->CancelOnMessageLoopThread();
+    }
+   private:
+    scoped_refptr<PlatformDelegate> delegate_;

[Mattias Nissler (ping if slow), 2011/03/22 17:23:44]

DISALLOW_COPY_AND_ASSIGN

[Joao da Silva, 2011/03/22 17:42:33]

Done.

   };
 
   // Used internally to encapsulate different members on different platforms.
-  class PlatformDelegate
-      : public base::RefCountedThreadSafe<PlatformDelegate,
-                                          DeletePlatformDelegate> {
+  class PlatformDelegate: public base::RefCountedThreadSafe<PlatformDelegate> {
    public:
     PlatformDelegate();
 
@@ -70,14 +81,17 @@ class FilePathWatcher {
 
     // Stop watching. This is called from FilePathWatcher's dtor in order to
     // allow to shut down properly while the object is still alive.
+    // It can be called from any thread.
     virtual void Cancel() = 0;
 
    protected:
-    friend class DeleteTask<PlatformDelegate>;
-    friend struct DeletePlatformDelegate;
-
     virtual ~PlatformDelegate();
 
+    // Stop watching. This is only called on the thread of the appropriate
+    // message loop. Since it can also be called more than once, it should
+    // check |is_cancelled()| to avoid duplicate work.
+    virtual void CancelOnMessageLoopThread() = 0;
+
     scoped_refptr<base::MessageLoopProxy> message_loop() const {
       return message_loop_;
     }
@@ -86,8 +100,21 @@ class FilePathWatcher {
       message_loop_ = loop;
     }
 
+    // Must be called before the PlatformDelegate is deleted.
+    void set_cancelled() {
+      cancelled_ = true;
+    }
+
+    bool is_cancelled() const {
+      return cancelled_;
+    }
+
    private:
+    friend class base::RefCountedThreadSafe<PlatformDelegate>;
+    friend class CancelTask;
+
     scoped_refptr<base::MessageLoopProxy> message_loop_;
+    bool cancelled_;
   };
 
  private: