Clean up PluginDataRemover.

This CL fixes some threading issues in PluginDataRemover. It also adds a static method to find out if removing plugin data is supported by an installed plugin, and a method to set the completion task while it is in the process of removing data.

BUG=58235
TEST=none

Review URL: http://codereview.chromium.org/5715001

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

2 files changed, 91 insertions, 28 deletions

diff --git a/chrome/browser/plugin_data_remover.cc b/chrome/browser/plugin_data_remover.cc
index cdfd5f4..74aa07c 100644
--- a/chrome/browser/plugin_data_remover.cc
+++ b/chrome/browser/plugin_data_remover.cc
@@ -5,46 +5,55 @@
 #include "chrome/browser/plugin_data_remover.h"
 
 #include "base/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
+#include "base/version.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/plugin_service.h"
 #include "chrome/common/plugin_messages.h"
-#include "ipc/ipc_channel.h"
+#include "webkit/glue/plugins/plugin_group.h"
+#include "webkit/glue/plugins/plugin_list.h"
 
 #if defined(OS_POSIX)
 #include "ipc/ipc_channel_posix.h"
 #endif
 
 namespace {
-const std::string flash_mime_type = "application/x-shockwave-flash";
-const int64 timeout_ms = 10000;
-}
+const char* g_flash_mime_type = "application/x-shockwave-flash";
+// TODO(bauerb): Update minimum required Flash version as soon as there is one
+// implementing the API.
+const char* g_min_flash_version = "100";
+const int64 g_timeout_ms = 10000;
+}  // namespace
 
 PluginDataRemover::PluginDataRemover()
-    : channel_(NULL),
-      method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
-}
+    : is_removing_(false),
+      channel_(NULL) { }
 
 PluginDataRemover::~PluginDataRemover() {
-  DCHECK(!done_task_.get());
-  if (!BrowserThread::DeleteSoon(BrowserThread::IO, FROM_HERE, channel_))
-    delete channel_;
+  DCHECK(!is_removing_);
+  if (channel_)
+    BrowserThread::DeleteSoon(BrowserThread::IO, FROM_HERE, channel_);
 }
 
 void PluginDataRemover::StartRemoving(base::Time begin_time, Task* done_task) {
   DCHECK(!done_task_.get());
+  DCHECK(!is_removing_);
+  remove_start_time_ = base::Time::Now();
   begin_time_ = begin_time;
 
   message_loop_ = base::MessageLoopProxy::CreateForCurrentThread();
   done_task_.reset(done_task);
+  is_removing_ = true;
 
+  AddRef();
   PluginService::GetInstance()->OpenChannelToPlugin(
-      GURL(), flash_mime_type, this);
+      GURL(), g_flash_mime_type, this);
 
   BrowserThread::PostDelayedTask(
       BrowserThread::IO,
       FROM_HERE,
-      method_factory_.NewRunnableMethod(&PluginDataRemover::OnTimeout),
-      timeout_ms);
+      NewRunnableMethod(this, &PluginDataRemover::OnTimeout),
+      g_timeout_ms);
 }
 
 int PluginDataRemover::ID() {
@@ -60,18 +69,28 @@ void PluginDataRemover::SetPluginInfo(const WebPluginInfo& info) {
 }
 
 void PluginDataRemover::OnChannelOpened(const IPC::ChannelHandle& handle) {
+  ConnectToChannel(handle);
+  Release();
+}
+
+void PluginDataRemover::ConnectToChannel(const IPC::ChannelHandle& handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // If we timed out, don't bother connecting.
+  if (!is_removing_)
+    return;
+
   DCHECK(!channel_);
   channel_ = new IPC::Channel(handle, IPC::Channel::MODE_CLIENT, this);
   if (!channel_->Connect()) {
-    NOTREACHED() << "Couldn't connect to plugin";
+    LOG(DFATAL) << "Couldn't connect to plugin";
     SignalDone();
     return;
   }
 
   if (!channel_->Send(
           new PluginMsg_ClearSiteData(0, std::string(), begin_time_))) {
-    NOTREACHED() << "Couldn't send ClearSiteData message";
+    LOG(DFATAL) << "Couldn't send ClearSiteData message";
     SignalDone();
   }
 }
@@ -79,10 +98,14 @@ void PluginDataRemover::OnChannelOpened(const IPC::ChannelHandle& handle) {
 void PluginDataRemover::OnError() {
   NOTREACHED() << "Couldn't open plugin channel";
   SignalDone();
+  Release();
 }
 
 void PluginDataRemover::OnClearSiteDataResult(bool success) {
-  DCHECK(success) << "ClearSiteData returned error";
+  if (!success)
+    LOG(DFATAL) << "ClearSiteData returned error";
+  UMA_HISTOGRAM_TIMES("ClearPluginData.time",
+                      base::Time::Now() - remove_start_time_);
   SignalDone();
 }
 
@@ -100,14 +123,38 @@ void PluginDataRemover::OnMessageReceived(const IPC::Message& msg) {
 }
 
 void PluginDataRemover::OnChannelError() {
-  NOTREACHED() << "Channel error";
+  LOG(DFATAL) << "Channel error";
   SignalDone();
 }
 
 void PluginDataRemover::SignalDone() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!done_task_.get())
+  if (!is_removing_)
     return;
-  message_loop_->PostTask(FROM_HERE, done_task_.release());
-  message_loop_ = NULL;
+  is_removing_ = false;
+  if (done_task_.get()) {
+    message_loop_->PostTask(FROM_HERE, done_task_.release());
+    message_loop_ = NULL;
+  }
+}
+
+// static
+bool PluginDataRemover::IsSupported() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  bool allow_wildcard = false;
+  WebPluginInfo plugin;
+  std::string mime_type;
+  if (!NPAPI::PluginList::Singleton()->GetPluginInfo(GURL(),
+                                                     g_flash_mime_type,
+                                                     allow_wildcard,
+                                                     &plugin,
+                                                     &mime_type))
+    return false;
+  scoped_ptr<Version> version(
+      PluginGroup::CreateVersionFromString(plugin.version));
+  scoped_ptr<Version> min_version(
+      Version::GetVersionFromString(g_min_flash_version));
+  return plugin.enabled &&
+         version.get() &&
+         min_version->CompareTo(*version) == -1;
 }
diff --git a/chrome/browser/plugin_data_remover.h b/chrome/browser/plugin_data_remover.h
index cbc2556..03b7e6f 100644
--- a/chrome/browser/plugin_data_remover.h
+++ b/chrome/browser/plugin_data_remover.h
@@ -9,8 +9,7 @@
 #include "base/ref_counted.h"
 #include "base/time.h"
 #include "chrome/browser/plugin_process_host.h"
-#include "ipc/ipc_channel_proxy.h"
-#include "ipc/ipc_message.h"
+#include "ipc/ipc_channel.h"
 
 class Task;
 
@@ -18,16 +17,27 @@ namespace base {
 class MessageLoopProxy;
 }
 
-class PluginDataRemover : public PluginProcessHost::Client,
+class PluginDataRemover : public base::RefCountedThreadSafe<PluginDataRemover>,
+                          public PluginProcessHost::Client,
                           public IPC::Channel::Listener {
  public:
   PluginDataRemover();
-  ~PluginDataRemover();
 
-  // Starts removing plug-in data stored since |begin_time| and calls
-  // |done_task| on the current thread when finished.
+  // Starts removing plug-in data stored since |begin_time|. If |done_task| is
+  // not NULL, it is run on the current thread when removing has finished.
   void StartRemoving(base::Time begin_time, Task* done_task);
 
+  // Returns whether there is a plug-in installed that supports removing
+  // LSO data. Because this method possibly has to load the plug-in list, it
+  // should only be called on the FILE thread.
+  static bool IsSupported();
+
+  bool is_removing() const { return is_removing_; }
+
+  // Sets the task to run when removing has finished. Takes ownership of
+  // the passed task.
+  void set_done_task(Task* task) { done_task_.reset(task); }
+
   // PluginProcessHost::Client methods
   virtual int ID();
   virtual bool OffTheRecord();
@@ -40,18 +50,24 @@ class PluginDataRemover : public PluginProcessHost::Client,
   virtual void OnChannelError();
 
  private:
+  friend class base::RefCountedThreadSafe<PluginDataRemover>;
+  ~PluginDataRemover();
+
   void SignalDone();
-  void SignalError();
+  void ConnectToChannel(const IPC::ChannelHandle& handle);
   void OnClearSiteDataResult(bool success);
   void OnTimeout();
 
   scoped_refptr<base::MessageLoopProxy> message_loop_;
+  bool is_removing_;
   scoped_ptr<Task> done_task_;
+  // 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_;
   // We own the channel, but it's used on the IO thread, so it needs to be
   // deleted there as well.
   IPC::Channel* channel_;
-  ScopedRunnableMethodFactory<PluginDataRemover> method_factory_;
 };
 
 #endif  // CHROME_BROWSER_PLUGIN_DATA_REMOVER_H_