Revert 137396 - Sequentialize calls to PluginList::GetPlugins from PluginService.

BUG=105987
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10381087

TBR=bauerb@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10383210

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

4 files changed, 28 insertions, 64 deletions

diff --git a/content/browser/plugin_service_impl.cc b/content/browser/plugin_service_impl.cc
index 29873b4..e7dfead 100644
--- a/content/browser/plugin_service_impl.cc
+++ b/content/browser/plugin_service_impl.cc
@@ -13,6 +13,7 @@
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
@@ -59,17 +60,13 @@ static void GetPluginsForGroupsCallback(
 
 // Callback set on the PluginList to assert that plugin loading happens on the
 // correct thread.
+void WillLoadPluginsCallback() {
 #if defined(OS_WIN)
-void WillLoadPluginsCallbackWin(
-    base::SequencedWorkerPool::SequenceToken token) {
-  CHECK(BrowserThread::GetBlockingPool()->IsRunningSequenceOnCurrentThread(
-      token));
-}
+  CHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
 #else
-void WillLoadPluginsCallbackPosix() {
   CHECK(false) << "Plugin loading should happen out-of-process.";
-}
 #endif
+}
 
 }  // namespace
 
@@ -150,14 +147,8 @@ void PluginServiceImpl::Init() {
   if (!plugin_list_)
     plugin_list_ = webkit::npapi::PluginList::Singleton();
 
-#if defined(OS_WIN)
-  plugin_list_token_ = BrowserThread::GetBlockingPool()->GetSequenceToken();
   plugin_list_->set_will_load_plugins_callback(
-      base::Bind(&WillLoadPluginsCallbackWin, plugin_list_token_));
-#else
-  plugin_list_->set_will_load_plugins_callback(
-      base::Bind(&WillLoadPluginsCallbackPosix));
-#endif
+      base::Bind(&WillLoadPluginsCallback));
 
   RegisterPepperPlugins();
 
@@ -172,7 +163,6 @@ void PluginServiceImpl::Init() {
   if (!path.empty())
     plugin_list_->AddExtraPluginDir(path);
 
-
 #if defined(OS_MACOSX)
   // We need to know when the browser comes forward so we can bring modal plugin
   // windows forward too.
@@ -202,8 +192,7 @@ void PluginServiceImpl::StartWatchingPlugins() {
       hklm_watcher_.StartWatching(hklm_event_.get(), this);
     }
   }
-#endif
-#if defined(OS_POSIX) && !defined(OS_OPENBSD)
+#elif defined(OS_POSIX) && !defined(OS_OPENBSD)
 // On ChromeOS the user can't install plugins anyway and on Windows all
 // important plugins register themselves in the registry so no need to do that.
   file_watcher_delegate_ = new PluginDirWatcherDelegate();
@@ -517,13 +506,12 @@ void PluginServiceImpl::GetPlugins(const GetPluginsCallback& callback) {
       MessageLoop::current()->message_loop_proxy());
 
 #if defined(OS_WIN)
-  BrowserThread::GetBlockingPool()->PostSequencedWorkerTaskWithShutdownBehavior(
-      plugin_list_token_,
+  BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior(
       FROM_HERE,
       base::Bind(&PluginServiceImpl::GetPluginsInternal, base::Unretained(this),
                  target_loop, callback),
       base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
-#elif defined(OS_POSIX)
+#else
   std::vector<webkit::WebPluginInfo> cached_plugins;
   if (plugin_list_->GetPluginsIfNoRefreshNeeded(&cached_plugins)) {
     // Can't assume the caller is reentrant.
@@ -538,8 +526,6 @@ void PluginServiceImpl::GetPlugins(const GetPluginsCallback& callback) {
         base::Bind(&PluginLoaderPosix::LoadPlugins, plugin_loader_,
                    target_loop, callback));
   }
-#else
-#error Not implemented
 #endif
 }
 
@@ -548,12 +534,10 @@ void PluginServiceImpl::GetPluginGroups(
   GetPlugins(base::Bind(&GetPluginsForGroupsCallback, callback));
 }
 
-#if defined(OS_WIN)
 void PluginServiceImpl::GetPluginsInternal(
      base::MessageLoopProxy* target_loop,
      const PluginService::GetPluginsCallback& callback) {
-  DCHECK(BrowserThread::GetBlockingPool()->IsRunningSequenceOnCurrentThread(
-      plugin_list_token_));
+  DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
 
   std::vector<webkit::WebPluginInfo> plugins;
   plugin_list_->GetPlugins(&plugins);
@@ -561,7 +545,6 @@ void PluginServiceImpl::GetPluginsInternal(
   target_loop->PostTask(FROM_HERE,
       base::Bind(callback, plugins));
 }
-#endif
 
 void PluginServiceImpl::OnWaitableEventSignaled(
     base::WaitableEvent* waitable_event) {
diff --git a/content/browser/plugin_service_impl.h b/content/browser/plugin_service_impl.h
index b3fcbf9..bc8f112 100644
--- a/content/browser/plugin_service_impl.h
+++ b/content/browser/plugin_service_impl.h
@@ -18,7 +18,6 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/waitable_event_watcher.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/time.h"
 #include "build/build_config.h"
 #include "content/browser/plugin_process_host.h"
@@ -178,11 +177,9 @@ class CONTENT_EXPORT PluginServiceImpl
 
   void RegisterPepperPlugins();
 
-#if defined(OS_WIN)
-  // Run on the blocking pool to load the plugins synchronously.
+  // Function that is run on the FILE thread to load the plugins synchronously.
   void GetPluginsInternal(base::MessageLoopProxy* target_loop,
                           const GetPluginsCallback& callback);
-#endif
 
   // Binding directly to GetAllowedPluginForOpenChannelToPlugin() isn't possible
   // because more arity is needed <http://crbug.com/98542>. This just forwards.
@@ -243,10 +240,6 @@ class CONTENT_EXPORT PluginServiceImpl
 
   std::set<PluginProcessHost::Client*> pending_plugin_clients_;
 
-#if defined(OS_WIN)
-  // Used to sequentialize loading plug-ins from disk.
-  base::SequencedWorkerPool::SequenceToken plugin_list_token_;
-#endif
 #if defined(OS_POSIX)
   scoped_refptr<PluginLoaderPosix> plugin_loader_;
 #endif
diff --git a/webkit/plugins/npapi/plugin_list.cc b/webkit/plugins/npapi/plugin_list.cc
index 6023fee..0b04b31 100644
--- a/webkit/plugins/npapi/plugin_list.cc
+++ b/webkit/plugins/npapi/plugin_list.cc
@@ -232,7 +232,7 @@ bool PluginList::DebugPluginLoading() {
 
 void PluginList::RefreshPlugins() {
   base::AutoLock lock(lock_);
-  loading_state_ = LOADING_STATE_NEEDS_REFRESH;
+  plugins_need_refresh_ = true;
 }
 
 void PluginList::AddExtraPluginPath(const FilePath& plugin_path) {
@@ -371,7 +371,7 @@ bool PluginList::ParseMimeTypes(
 }
 
 PluginList::PluginList()
-    : loading_state_(LOADING_STATE_NEEDS_REFRESH) {
+    : plugins_need_refresh_(true) {
   PlatformInit();
   AddHardcodedPluginGroups(kGroupDefinitions,
                            ARRAYSIZE_UNSAFE(kGroupDefinitions));
@@ -379,7 +379,7 @@ PluginList::PluginList()
 
 PluginList::PluginList(const PluginGroupDefinition* definitions,
                        size_t num_definitions)
-    : loading_state_(LOADING_STATE_NEEDS_REFRESH) {
+    : plugins_need_refresh_(true) {
   // Don't do platform-dependend initialization in unit tests.
   AddHardcodedPluginGroups(definitions, num_definitions);
 }
@@ -398,8 +398,12 @@ void PluginList::LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups) {
   base::Closure will_load_callback;
   {
     base::AutoLock lock(lock_);
+    // Clear the refresh bit now, because it might get set again before we
+    // reach the end of the method.
+    plugins_need_refresh_ = false;
     will_load_callback = will_load_plugins_callback_;
   }
+
   if (!will_load_callback.is_null())
     will_load_callback.Run();
 
@@ -417,10 +421,8 @@ void PluginList::LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups) {
 void PluginList::LoadPlugins() {
   {
     base::AutoLock lock(lock_);
-    if (loading_state_ == LOADING_STATE_UP_TO_DATE)
+    if (!plugins_need_refresh_)
       return;
-
-    loading_state_ = LOADING_STATE_REFRESHING;
   }
 
   ScopedVector<PluginGroup> new_plugin_groups;
@@ -429,10 +431,6 @@ void PluginList::LoadPlugins() {
 
   base::AutoLock lock(lock_);
   plugin_groups_.swap(new_plugin_groups);
-  // If we haven't been invalidated in the mean time, mark the plug-in list as
-  // up-to-date.
-  if (loading_state_ != LOADING_STATE_NEEDS_REFRESH)
-    loading_state_ = LOADING_STATE_UP_TO_DATE;
 }
 
 bool PluginList::LoadPlugin(const FilePath& path,
@@ -506,8 +504,7 @@ const std::vector<PluginGroup*>& PluginList::GetHardcodedPluginGroups() const {
 void PluginList::SetPlugins(const std::vector<webkit::WebPluginInfo>& plugins) {
   base::AutoLock lock(lock_);
 
-  DCHECK_NE(LOADING_STATE_REFRESHING, loading_state_);
-  loading_state_ = LOADING_STATE_UP_TO_DATE;
+  plugins_need_refresh_ = false;
 
   plugin_groups_.reset();
   for (std::vector<webkit::WebPluginInfo>::const_iterator it = plugins.begin();
@@ -535,7 +532,7 @@ void PluginList::GetPlugins(std::vector<WebPluginInfo>* plugins) {
 bool PluginList::GetPluginsIfNoRefreshNeeded(
     std::vector<webkit::WebPluginInfo>* plugins) {
   base::AutoLock lock(lock_);
-  if (loading_state_ != LOADING_STATE_UP_TO_DATE)
+  if (plugins_need_refresh_)
     return false;
 
   for (size_t i = 0; i < plugin_groups_.size(); ++i) {
@@ -560,7 +557,7 @@ void PluginList::GetPluginInfoArray(
     LoadPlugins();
   base::AutoLock lock(lock_);
   if (use_stale)
-    *use_stale = (loading_state_ != LOADING_STATE_UP_TO_DATE);
+    *use_stale = plugins_need_refresh_;
   info->clear();
   if (actual_mime_types)
     actual_mime_types->clear();
diff --git a/webkit/plugins/npapi/plugin_list.h b/webkit/plugins/npapi/plugin_list.h
index a948782..d01b38f 100644
--- a/webkit/plugins/npapi/plugin_list.h
+++ b/webkit/plugins/npapi/plugin_list.h
@@ -121,7 +121,7 @@ class WEBKIT_PLUGINS_EXPORT PluginList {
       const string16& mime_type_descriptions,
       std::vector<webkit::WebPluginMimeType>* parsed_mime_types);
 
-  // Get all the plugins synchronously, loading them if necessary.
+  // Get all the plugins synchronously.
   void GetPlugins(std::vector<webkit::WebPluginInfo>* plugins);
 
   // Returns true if the list of plugins is cached and is copied into the out
@@ -195,17 +195,6 @@ class WEBKIT_PLUGINS_EXPORT PluginList {
                                  ScopedVector<PluginGroup>* plugin_groups);
 
  private:
-  enum LoadingState {
-    LOADING_STATE_NEEDS_REFRESH,
-    LOADING_STATE_REFRESHING,
-    LOADING_STATE_UP_TO_DATE,
-  };
-
-  struct InternalPlugin {
-    webkit::WebPluginInfo info;
-    PluginEntryPoints entry_points;
-  };
-
   friend class PluginListTest;
   friend struct base::DefaultLazyInstanceTraits<PluginList>;
   FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
@@ -279,10 +268,8 @@ class WEBKIT_PLUGINS_EXPORT PluginList {
   // Internals
   //
 
-  // States whether we will load the plug-in list the next time we try to access
-  // it, whether we are currently in the process of loading it, or whether we
-  // consider it up-to-date.
-  LoadingState loading_state_;
+  // If true, we reload plugins even if they've been loaded already.
+  bool plugins_need_refresh_;
 
   // Extra plugin paths that we want to search when loading.
   std::vector<FilePath> extra_plugin_paths_;
@@ -290,6 +277,10 @@ class WEBKIT_PLUGINS_EXPORT PluginList {
   // Extra plugin directories that we want to search when loading.
   std::vector<FilePath> extra_plugin_dirs_;
 
+  struct InternalPlugin {
+    webkit::WebPluginInfo info;
+    PluginEntryPoints entry_points;
+  };
   // Holds information about internal plugins.
   std::vector<InternalPlugin> internal_plugins_;