Restart plugin loading only if the plugin list has actually become stale.

content/browser/plugin_loader_posix.cc

Index: content/browser/plugin_loader_posix.cc
diff --git a/content/browser/plugin_loader_posix.cc b/content/browser/plugin_loader_posix.cc
index 611b1472179f951524751e95ad1bee4f797a0088..cbf6c2a88cb03d3c07da7a4f85210ce4deed0c48 100644
--- a/content/browser/plugin_loader_posix.cc
+++ b/content/browser/plugin_loader_posix.cc
@@ -22,16 +22,35 @@ PluginLoaderPosix::PluginLoaderPosix()
     : next_load_index_(0) {
 }
 
-void PluginLoaderPosix::LoadPlugins(
-    scoped_refptr<base::MessageLoopProxy> target_loop,
+void PluginLoaderPosix::GetPlugins(
     const PluginService::GetPluginsCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  callbacks_.push_back(PendingCallback(target_loop, callback));
+  std::vector<WebPluginInfo> cached_plugins;
+  if (PluginList::Singleton()->GetPluginsNoRefresh(&cached_plugins)) {
+    // Can't assume the caller is reentrant.
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(callback, cached_plugins));
+    return;
+  }
+
+  if (callbacks_.empty()) {
+    callbacks_.push_back(callback);
+
+    PluginList::Singleton()->PrepareForPluginLoading();
 
-  if (callbacks_.size() == 1) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&PluginLoaderPosix::GetPluginsToLoad, this));
+    BrowserThread::PostTask(BrowserThread::FILE,
+                            FROM_HERE,
+                            base::Bind(&PluginLoaderPosix::GetPluginsToLoad,
+                                       make_scoped_refptr(this)));
+  } else {
+    // If we are currently loading plugins, the plugin list might have been
+    // invalidated in the mean time, or might get invalidated before we finish.
+    // We'll wait until we have finished the current run, then try to get them
+    // again from the plugin list. If it has indeed been invalidated, it will
+    // restart plugin loading, otherwise it will immediately run the callback.
+    callbacks_.push_back(base::Bind(&PluginLoaderPosix::GetPluginsWrapper,
+                                    make_scoped_refptr(this), callback));
   }
 }
 
@@ -122,6 +141,16 @@ void PluginLoaderPosix::LoadPluginsInternal() {
   process_host_->Send(new UtilityMsg_LoadPlugins(canonical_list_));
 }
 
+void PluginLoaderPosix::GetPluginsWrapper(
+    const PluginService::GetPluginsCallback& callback,
+    const std::vector<WebPluginInfo>& /* plugins */) {

[Robert Sesek, 2014/01/21 20:58:18]

nit: I don't think we typically omit parameters

[Bernhard Bauer, 2014/01/21 23:02:11]

Hmm... the style guide does say that unused parameters should be commented out
(http://goo.gl/LN96S2)/. In this case in particular, I want to make it clear
that the value passed to us shouldn't be used. Rename it to |plugins_unused|?

[Bernhard Bauer, 2014/01/22 14:10:20]

Renamed.

+  // We are being called after plugin loading has finished, but we don't know
+  // whether the plugin list has been invalidated in the mean time
+  // (and therefore |plugins| might already be stale). So we simply ignore it
+  // and call regular GetPlugins() instead.
+  GetPlugins(callback);
+}
+
 void PluginLoaderPosix::OnPluginLoaded(uint32 index,
                                        const WebPluginInfo& plugin) {
   if (index != next_load_index_) {
@@ -172,37 +201,20 @@ bool PluginLoaderPosix::MaybeRunPendingCallbacks() {
 
   PluginList::Singleton()->SetPlugins(loaded_plugins_);
 
-  // Only call the first callback with loaded plugins because there may be
-  // some extra plugin paths added since the first callback is added.
-  if (!callbacks_.empty()) {
-    PendingCallback callback = callbacks_.front();
-    callbacks_.pop_front();
-    callback.target_loop->PostTask(
-        FROM_HERE,
-        base::Bind(callback.callback, loaded_plugins_));
+  for (std::vector<PluginService::GetPluginsCallback>::iterator it =
+           callbacks_.begin();
+       it != callbacks_.end(); ++it) {
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+                                           base::Bind(*it, loaded_plugins_));
   }
+  callbacks_.clear();
 
   HISTOGRAM_TIMES("PluginLoaderPosix.LoadDone",
                   (base::TimeTicks::Now() - load_start_time_)
                       * base::Time::kMicrosecondsPerMillisecond);
   load_start_time_ = base::TimeTicks();
 
-  if (!callbacks_.empty()) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&PluginLoaderPosix::GetPluginsToLoad, this));
-    return false;
-  }
   return true;
 }
 
-PluginLoaderPosix::PendingCallback::PendingCallback(
-    scoped_refptr<base::MessageLoopProxy> loop,
-    const PluginService::GetPluginsCallback& cb)
-    : target_loop(loop),
-      callback(cb) {
-}
-
-PluginLoaderPosix::PendingCallback::~PendingCallback() {
-}
-
 }  // namespace content