Fix race in PluginDataRemoverImpl going away while it's still being used on the IO thread...

content/browser/plugin_data_remover_impl.h

Index: content/browser/plugin_data_remover_impl.h
===================================================================
--- content/browser/plugin_data_remover_impl.h	(revision 110846)
+++ content/browser/plugin_data_remover_impl.h	(working copy)
@@ -9,14 +9,10 @@
 #include <string>
 
 #include "base/compiler_specific.h"
-#include "base/memory/weak_ptr.h"
 #include "content/browser/plugin_process_host.h"
 #include "content/public/browser/plugin_data_remover.h"
 
-class CONTENT_EXPORT PluginDataRemoverImpl
-    : public content::PluginDataRemover,
-      public NON_EXPORTED_BASE(PluginProcessHost::Client),
-      public IPC::Channel::Listener {
+class CONTENT_EXPORT PluginDataRemoverImpl : public content::PluginDataRemover {
  public:
   explicit PluginDataRemoverImpl(
       const content::ResourceContext& resource_context);
@@ -30,48 +26,71 @@
   // different plug-in (for example in tests).
   void set_mime_type(const std::string& mime_type) { mime_type_ = mime_type; }
 
-  // PluginProcessHost::Client methods.
-  virtual int ID() OVERRIDE;
-  virtual bool OffTheRecord() OVERRIDE;
-  virtual const content::ResourceContext& GetResourceContext() OVERRIDE;
-  virtual void SetPluginInfo(const webkit::WebPluginInfo& info) OVERRIDE;
-  virtual void OnFoundPluginProcessHost(PluginProcessHost* host) OVERRIDE;
-  virtual void OnSentPluginChannelRequest() OVERRIDE;
-  virtual void OnChannelOpened(const IPC::ChannelHandle& handle) OVERRIDE;
-  virtual void OnError() OVERRIDE;
+ private:
+  class Context : public PluginProcessHost::Client,

[Bernhard Bauer, 2011/11/21 14:33:59]

Is there any advantage to using a inner Context class instead of just making
PluginDataRemover refcounted again?

[jam, 2011/11/21 17:29:18]

Yes, keeping this simpler for embedders. Now they can destroy
content::PluginDataRemove on the UI thread and not have to worry that it's being
used elsewhere. It's also better to hide the implementation details (i.e. that
it's refcounted) from the embedder.

[Bernhard Bauer, 2011/11/21 18:07:09]

OK. The destruction thing could be achieved with DeleteOnIOThread though.

+                  public IPC::Channel::Listener,
+                  public base::RefCountedThreadSafe<Context> {
+   public:
+    Context(const std::string& mime_type,
+            base::Time begin_time,
+            const content::ResourceContext& resource_context);
+    virtual ~Context();
 
-  // IPC::Channel::Listener methods.
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-  virtual void OnChannelError() OVERRIDE;
+    // PluginProcessHost::Client methods.
+    virtual int ID() OVERRIDE;
+    virtual bool OffTheRecord() OVERRIDE;
+    virtual const content::ResourceContext& GetResourceContext() OVERRIDE;
+    virtual void SetPluginInfo(const webkit::WebPluginInfo& info) OVERRIDE;
+    virtual void OnFoundPluginProcessHost(PluginProcessHost* host) OVERRIDE;
+    virtual void OnSentPluginChannelRequest() OVERRIDE;
+    virtual void OnChannelOpened(const IPC::ChannelHandle& handle) OVERRIDE;
+    virtual void OnError() OVERRIDE;
 
- private:
-  // Signals that we are finished with removing data (successful or not). This
-  // method is safe to call multiple times.
-  void SignalDone();
-  // Connects the client side of a newly opened plug-in channel.
-  void ConnectToChannel(const IPC::ChannelHandle& handle);
-  // Handles the PluginHostMsg_ClearSiteDataResult message.
-  void OnClearSiteDataResult(bool success);
-  // Called when a timeout happens in order not to block the client
-  // indefinitely.
-  void OnTimeout();
+    // IPC::Channel::Listener methods.
+    virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+    virtual void OnChannelError() OVERRIDE;
 
+    base::WaitableEvent* event() { return event_.get(); }
+
+   private:
+    // Initialize on the IO thread.
+    void Init(const std::string& mime_type);
+
+    // Connects the client side of a newly opened plug-in channel.
+    void ConnectToChannel(const IPC::ChannelHandle& handle);
+    // Handles the PluginHostMsg_ClearSiteDataResult message.
+    void OnClearSiteDataResult(bool success);
+    // Called when a timeout happens in order not to block the client
+    // indefinitely.
+    void OnTimeout();
+
+    // Signals that we are finished with removing data (successful or not). This
+    // method is safe to call multiple times.
+    void SignalDone();
+
+    scoped_ptr<base::WaitableEvent> event_;
+    // The point in time when we start removing data.
+    base::Time remove_start_time_;
+    // The point in time from which on we remove data.
+    base::Time begin_time_;
+    bool is_removing_;
+
+    // The resource context for the profile.
+    const content::ResourceContext& resource_context_;
+
+    // We own the channel, but it's used on the IO thread, so it needs to be
+    // deleted there. It's NULL until we have opened a connection to the plug-in
+    // process.
+    IPC::Channel* channel_;
+  };
+
   std::string mime_type_;
-  bool is_starting_process_;
-  bool is_removing_;
-  // The point in time when we start removing data.
-  base::Time remove_start_time_;
-  // The point in time from which on we remove data.
-  base::Time begin_time_;
   // The resource context for the profile.
-  const content::ResourceContext& context_;
-  scoped_ptr<base::WaitableEvent> event_;
-  // We own the channel, but it's used on the IO thread, so it needs to be
-  // deleted there. It's NULL until we have opened a connection to the plug-in
-  // process.
-  IPC::Channel* channel_;
+  const content::ResourceContext& resource_context_;
 
-  base::WeakPtrFactory<PluginDataRemoverImpl> weak_factory_;
+  // This allows this object to be deleted on the UI thread while it's still
+  // being used on the IO thread.
+  scoped_refptr<Context> context_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginDataRemoverImpl);
 };