Fixed shutdown concurrency issues in FilePathWatcher.

content/common/file_path_watcher/file_path_watcher.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This module provides a way to monitor a file or directory for changes.
 
 #ifndef CONTENT_COMMON_FILE_PATH_WATCHER_FILE_PATH_WATCHER_H_
 #define CONTENT_COMMON_FILE_PATH_WATCHER_FILE_PATH_WATCHER_H_
 #pragma once
 
@@ skipping 22 matching lines @@
   };
 
   FilePathWatcher();
   ~FilePathWatcher();
 
   // Register interest in any changes on |path|. OnPathChanged will be called
   // back for each change. Returns true on success. |loop| is only used
   // 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();
 
     // Start watching for the given |path| and notify |delegate| about changes.
     // |loop| is only used 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.
     virtual bool Watch(
         const FilePath& path,
         Delegate* delegate,
         base::MessageLoopProxy* loop) 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.
+    // It can be called from any thread.
     virtual void Cancel() = 0;
 
    protected:
-    friend class DeleteTask<PlatformDelegate>;
-    friend struct DeletePlatformDelegate;
+    virtual ~PlatformDelegate();
 
-    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_;
     }
 
     void set_message_loop(base::MessageLoopProxy* loop) {
       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:
   scoped_refptr<PlatformDelegate> impl_;
 
   DISALLOW_COPY_AND_ASSIGN(FilePathWatcher);
 };
 
 #endif  // CONTENT_COMMON_FILE_PATH_WATCHER_FILE_PATH_WATCHER_H_