Revert 23064 - Ensure we don't load plugins on the IO thread

BUG=17938
TEST=added asserts which crash if plugins loaded on IO thread, current UI tests exercise them
Review URL: http://codereview.chromium.org/164305

TBR=jam@chromium.org
Review URL: http://codereview.chromium.org/165321

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

9 files changed, 78 insertions, 176 deletions

diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc
index 1360e6f..d9d3e65 100644
--- a/chrome/browser/plugin_service.cc
+++ b/chrome/browser/plugin_service.cc
@@ -75,14 +75,8 @@ PluginService::~PluginService() {
 #endif
 }
 
-bool PluginService::PluginsLoaded() {
-  AutoLock lock(lock_);
-  return NPAPI::PluginList::PluginsLoaded();
-}
-
 void PluginService::GetPlugins(bool refresh,
                                std::vector<WebPluginInfo>* plugins) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   AutoLock lock(lock_);
   NPAPI::PluginList::Singleton()->GetPlugins(refresh, plugins);
 }
@@ -186,8 +180,6 @@ FilePath PluginService::GetPluginPath(const GURL& url,
                                       const std::string& clsid,
                                       std::string* actual_mime_type) {
   AutoLock lock(lock_);
-  DCHECK(NPAPI::PluginList::PluginsLoaded() ||
-         ChromeThread::CurrentlyOn(ChromeThread::FILE));
   bool allow_wildcard = true;
   WebPluginInfo info;
   if (NPAPI::PluginList::Singleton()->GetPluginInfo(url, mime_type, clsid,
@@ -203,16 +195,12 @@ FilePath PluginService::GetPluginPath(const GURL& url,
 bool PluginService::GetPluginInfoByPath(const FilePath& plugin_path,
                                         WebPluginInfo* info) {
   AutoLock lock(lock_);
-  DCHECK(NPAPI::PluginList::PluginsLoaded() ||
-         ChromeThread::CurrentlyOn(ChromeThread::FILE));
   return NPAPI::PluginList::Singleton()->GetPluginInfoByPath(plugin_path, info);
 }
 
 bool PluginService::HavePluginFor(const std::string& mime_type,
                                   bool allow_wildcard) {
   AutoLock lock(lock_);
-  DCHECK(NPAPI::PluginList::PluginsLoaded() ||
-         ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
   GURL url;
   WebPluginInfo info;
diff --git a/chrome/browser/plugin_service.h b/chrome/browser/plugin_service.h
index c8be00d..8bcbf7c 100644
--- a/chrome/browser/plugin_service.h
+++ b/chrome/browser/plugin_service.h
@@ -45,11 +45,7 @@ class PluginService
   // Returns the PluginService singleton.
   static PluginService* GetInstance();
 
-  // Returns true iff the plugin list has been loaded.
-  bool PluginsLoaded();
-
-  // Loads the plugin list.  Must be called on the file thread only since it may
-  // have to crawl the disk.
+  // Gets the list of available plugins.
   void GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins);
 
   // Load all the plugins that should be loaded for the lifetime of the browser
diff --git a/chrome/browser/renderer_host/buffered_resource_handler.cc b/chrome/browser/renderer_host/buffered_resource_handler.cc
index 60212e0..b1007fe 100644
--- a/chrome/browser/renderer_host/buffered_resource_handler.cc
+++ b/chrome/browser/renderer_host/buffered_resource_handler.cc
@@ -9,13 +9,10 @@
 #include "base/string_util.h"
 #include "net/base/mime_sniffer.h"
 #include "net/base/net_errors.h"
-#include "chrome/browser/chrome_thread.h"
-#include "chrome/browser/plugin_service.h"
 #include "chrome/browser/renderer_host/download_throttling_resource_handler.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/common/url_constants.h"
 #include "net/base/mime_sniffer.h"
-#include "net/base/mime_util.h"
 #include "net/base/io_buffer.h"
 #include "net/http/http_response_headers.h"
 
@@ -52,7 +49,6 @@ BufferedResourceHandler::BufferedResourceHandler(ResourceHandler* handler,
       bytes_read_(0),
       sniff_content_(false),
       should_buffer_(false),
-      wait_for_plugins_(false),
       buffering_(false),
       finished_(false) {
 }
@@ -122,13 +118,12 @@ bool BufferedResourceHandler::OnReadCompleted(int request_id, int* bytes_read) {
       return true;
 
     LOG(INFO) << "Finished buffering " << request_->url().spec();
+    sniff_content_ = should_buffer_ = false;
     *bytes_read = bytes_read_;
 
     // Done buffering, send the pending ResponseStarted event.
     if (!CompleteResponseStarted(request_id, true))
       return false;
-  } else if (wait_for_plugins_) {
-    return true;
   }
 
   // Release the reference that we acquired at OnWillRead.
@@ -176,12 +171,6 @@ bool BufferedResourceHandler::DelayResponse() {
     LOG(INFO) << "To buffer: " << request_->url().spec();
     return true;
   }
-
-  if (ShouldWaitForPlugins()) {
-    wait_for_plugins_ = true;
-    return true;
-  }
-
   return false;
 }
 
@@ -200,12 +189,6 @@ bool BufferedResourceHandler::ShouldBuffer(const GURL& url,
   return mime_type == "text/html";
 }
 
-bool BufferedResourceHandler::DidBufferEnough(int bytes_read) {
-  const int kRequiredLength = 256;
-
-  return bytes_read >= kRequiredLength;
-}
-
 bool BufferedResourceHandler::KeepBuffering(int bytes_read) {
   DCHECK(read_buffer_);
   if (my_buffer_) {
@@ -236,34 +219,17 @@ bool BufferedResourceHandler::KeepBuffering(int bytes_read) {
     response_->response_head.mime_type.assign(new_type);
 
     // We just sniffed the mime type, maybe there is a doctype to process.
-    if (ShouldBuffer(request_->url(), new_type)) {
+    if (ShouldBuffer(request_->url(), new_type))
       should_buffer_ = true;
-    } else if (ShouldWaitForPlugins()) {
-      wait_for_plugins_ = true;
-    }
   }
 
-  if (should_buffer_) {
-    if (!finished_ && !DidBufferEnough(bytes_read_)) {
+  if (!finished_ && should_buffer_) {
+    if (!DidBufferEnough(bytes_read_)) {
       buffering_ = true;
       return true;
     }
-
-    should_buffer_ = false;
-    if (ShouldWaitForPlugins())
-      wait_for_plugins_ = true;
   }
-
   buffering_ = false;
-
-  if (wait_for_plugins_) {
-    // We don't want to keep buffering as our buffer will fill up.
-    ResourceDispatcherHost::ExtraRequestInfo* info =
-        ResourceDispatcherHost::ExtraInfoForRequest(request_);
-    host_->PauseRequest(info->process_id, info->request_id, true);
-    return true;
-  }
-
   return false;
 }
 
@@ -272,10 +238,16 @@ bool BufferedResourceHandler::CompleteResponseStarted(int request_id,
   // Check to see if we should forward the data from this request to the
   // download thread.
   // TODO(paulg): Only download if the context from the renderer allows it.
+  std::string content_disposition;
+  request_->GetResponseHeaderByName("content-disposition",
+                                    &content_disposition);
+
   ResourceDispatcherHost::ExtraRequestInfo* info =
       ResourceDispatcherHost::ExtraInfoForRequest(request_);
 
-  if (info->allow_download && ShouldDownload(NULL)) {
+  if (info->allow_download &&
+      host_->ShouldDownload(response_->response_head.mime_type,
+                            content_disposition)) {
     if (response_->response_head.headers &&  // Can be NULL if FTP.
         response_->response_head.headers->response_code() / 100 != 2) {
       // The response code indicates that this is an error page, but we don't
@@ -321,87 +293,8 @@ bool BufferedResourceHandler::CompleteResponseStarted(int request_id,
   return real_handler_->OnResponseStarted(request_id, response_);
 }
 
-bool BufferedResourceHandler::ShouldWaitForPlugins() {
-  bool need_plugin_list;
-  if (!ShouldDownload(&need_plugin_list) || !need_plugin_list)
-    return false;
-
-  // Schedule plugin loading on the file thread.
-  ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &BufferedResourceHandler::LoadPlugins));
-  return true;
-}
-
-// This test mirrors the decision that WebKit makes in
-// WebFrameLoaderClient::dispatchDecidePolicyForMIMEType.
-bool BufferedResourceHandler::ShouldDownload(bool* need_plugin_list) {
-  if (need_plugin_list)
-    *need_plugin_list = false;
-  std::string type = StringToLowerASCII(response_->response_head.mime_type);
-  std::string disposition;
-  request_->GetResponseHeaderByName("content-disposition", &disposition);
-  disposition = StringToLowerASCII(disposition);
-
-  // First, examine content-disposition.
-  if (!disposition.empty()) {
-    bool should_download = true;
-
-    // Some broken sites just send ...
-    //    Content-Disposition: ; filename="file"
-    // ... screen those out here.
-    if (disposition[0] == ';')
-      should_download = false;
-
-    if (disposition.compare(0, 6, "inline") == 0)
-      should_download = false;
-
-    // Some broken sites just send ...
-    //    Content-Disposition: filename="file"
-    // ... without a disposition token... Screen those out.
-    if (disposition.compare(0, 8, "filename") == 0)
-      should_download = false;
-
-    // Also in use is Content-Disposition: name="file"
-    if (disposition.compare(0, 4, "name") == 0)
-      should_download = false;
-
-    // We have a content-disposition of "attachment" or unknown.
-    // RFC 2183, section 2.8 says that an unknown disposition
-    // value should be treated as "attachment".
-    if (should_download)
-      return true;
-  }
-
-  // MIME type checking.
-  if (net::IsSupportedMimeType(type))
-    return false;
-
-  if (need_plugin_list) {
-    if (!PluginService::GetInstance()->PluginsLoaded()) {
-      *need_plugin_list = true;
-      return true;
-    }
-  } else {
-    DCHECK(PluginService::GetInstance()->PluginsLoaded());
-  }
-
-  // Finally, check the plugin service.
-  bool allow_wildcard = false;
-  return !PluginService::GetInstance()->HavePluginFor(type, allow_wildcard);
-}
-
-void BufferedResourceHandler::LoadPlugins() {
-  std::vector<WebPluginInfo> plugins;
-  PluginService::GetInstance()->GetPlugins(false, &plugins);
-  ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &BufferedResourceHandler::OnPluginsLoaded));
-}
+bool BufferedResourceHandler::DidBufferEnough(int bytes_read) {
+  const int kRequiredLength = 256;
 
-void BufferedResourceHandler::OnPluginsLoaded() {
-  wait_for_plugins_ = false;
-  ResourceDispatcherHost::ExtraRequestInfo* info =
-      ResourceDispatcherHost::ExtraInfoForRequest(request_);
-  host_->PauseRequest(info->process_id, info->request_id, false);
-  if (!CompleteResponseStarted(info->request_id, false))
-    host_->CancelRequest(info->process_id, info->request_id, false);
+  return bytes_read >= kRequiredLength;
 }
diff --git a/chrome/browser/renderer_host/buffered_resource_handler.h b/chrome/browser/renderer_host/buffered_resource_handler.h
index b36ac9b..3799b99 100644
--- a/chrome/browser/renderer_host/buffered_resource_handler.h
+++ b/chrome/browser/renderer_host/buffered_resource_handler.h
@@ -49,22 +49,6 @@ class BufferedResourceHandler : public ResourceHandler {
   // this is invoked from |OnResponseCompleted|.
   bool CompleteResponseStarted(int request_id, bool in_complete);
 
-  // Returns true if we have to wait until the plugin list is generated.
-  bool ShouldWaitForPlugins();
-
-  // A test to determining whether the request should be forwarded to the
-  // download thread.  If need_plugin_list was passed in and was set to true,
-  // that means that the check couldn't be fully done because the plugins aren't
-  // loaded.  The function should be called again after the plugin list is
-  // loaded.
-  bool ShouldDownload(bool* need_plugin_list);
-
-  // Called on the file thread to load the list of plugins.
-  void LoadPlugins();
-
-  // Called on the IO thread once the list of plugins has been loaded.
-  void OnPluginsLoaded();
-
   scoped_refptr<ResourceHandler> real_handler_;
   scoped_refptr<ResourceResponse> response_;
   ResourceDispatcherHost* host_;
@@ -75,7 +59,6 @@ class BufferedResourceHandler : public ResourceHandler {
   int bytes_read_;
   bool sniff_content_;
   bool should_buffer_;
-  bool wait_for_plugins_;
   bool buffering_;
   bool finished_;
 
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc
index 590a31d..05d116d 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -256,6 +256,7 @@ ResourceDispatcherHost::ResourceDispatcherHost(MessageLoop* io_loop)
       safe_browsing_(new SafeBrowsingService),
       webkit_thread_(new WebKitThread),
       request_id_(-1),
+      plugin_service_(PluginService::GetInstance()),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_runner_(this)),
       is_shutdown_(false),
       max_outstanding_requests_cost_per_process_(
@@ -467,7 +468,7 @@ void ResourceDispatcherHost::BeginRequest(
   // requests.  Does nothing if they are already loaded.
   // TODO(mpcomplete): This takes 200 ms!  Investigate parallelizing this by
   // starting the load earlier in a BG thread.
-  PluginService::GetInstance()->LoadChromePlugins(this);
+  plugin_service_->LoadChromePlugins(this);
 
   // Construct the event handler.
   scoped_refptr<ResourceHandler> handler;
@@ -667,7 +668,7 @@ void ResourceDispatcherHost::BeginDownload(const GURL& url,
 
   // Ensure the Chrome plugins are loaded, as they may intercept network
   // requests.  Does nothing if they are already loaded.
-  PluginService::GetInstance()->LoadChromePlugins(this);
+  plugin_service_->LoadChromePlugins(this);
   URLRequest* request = new URLRequest(url, this);
 
   request_id_--;
@@ -733,7 +734,7 @@ void ResourceDispatcherHost::BeginSaveFile(const GURL& url,
 
   // Ensure the Chrome plugins are loaded, as they may intercept network
   // requests.  Does nothing if they are already loaded.
-  PluginService::GetInstance()->LoadChromePlugins(this);
+  plugin_service_->LoadChromePlugins(this);
 
   scoped_refptr<ResourceHandler> handler =
       new SaveFileResourceHandler(process_id,
@@ -1259,6 +1260,53 @@ void ResourceDispatcherHost::BeginRequestInternal(URLRequest* request) {
   }
 }
 
+// This test mirrors the decision that WebKit makes in
+// WebFrameLoaderClient::dispatchDecidePolicyForMIMEType.
+// static.
+bool ResourceDispatcherHost::ShouldDownload(
+    const std::string& mime_type, const std::string& content_disposition) {
+  std::string type = StringToLowerASCII(mime_type);
+  std::string disposition = StringToLowerASCII(content_disposition);
+
+  // First, examine content-disposition.
+  if (!disposition.empty()) {
+    bool should_download = true;
+
+    // Some broken sites just send ...
+    //    Content-Disposition: ; filename="file"
+    // ... screen those out here.
+    if (disposition[0] == ';')
+      should_download = false;
+
+    if (disposition.compare(0, 6, "inline") == 0)
+      should_download = false;
+
+    // Some broken sites just send ...
+    //    Content-Disposition: filename="file"
+    // ... without a disposition token... Screen those out.
+    if (disposition.compare(0, 8, "filename") == 0)
+      should_download = false;
+
+    // Also in use is Content-Disposition: name="file"
+    if (disposition.compare(0, 4, "name") == 0)
+      should_download = false;
+
+    // We have a content-disposition of "attachment" or unknown.
+    // RFC 2183, section 2.8 says that an unknown disposition
+    // value should be treated as "attachment".
+    if (should_download)
+      return true;
+  }
+
+  // MIME type checking.
+  if (net::IsSupportedMimeType(type))
+    return false;
+
+  // Finally, check the plugin service.
+  bool allow_wildcard = false;
+  return !plugin_service_->HavePluginFor(type, allow_wildcard);
+}
+
 bool ResourceDispatcherHost::PauseRequestIfNeeded(ExtraRequestInfo* info) {
   if (info->pause_count > 0)
     info->is_paused = true;
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.h b/chrome/browser/renderer_host/resource_dispatcher_host.h
index bcac386..5353562 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.h
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.h
@@ -352,6 +352,11 @@ class ResourceDispatcherHost : public URLRequest::Delegate {
   // Retrieves a URLRequest.  Must be called from the IO thread.
   URLRequest* GetURLRequest(GlobalRequestID request_id) const;
 
+  // A test to determining whether a given request should be forwarded to the
+  // download thread.
+  bool ShouldDownload(const std::string& mime_type,
+                      const std::string& content_disposition);
+
   // Notifies our observers that a request has been cancelled.
   void NotifyResponseCompleted(URLRequest* request, int process_id);
 
@@ -546,6 +551,8 @@ class ResourceDispatcherHost : public URLRequest::Delegate {
   // List of objects observing resource dispatching.
   ObserverList<Observer> observer_list_;
 
+  PluginService* plugin_service_;
+
   // For running tasks.
   ScopedRunnableMethodFactory<ResourceDispatcherHost> method_runner_;
 
diff --git a/chrome/browser/renderer_host/resource_handler.h b/chrome/browser/renderer_host/resource_handler.h
index fec7db0..76851b6 100644
--- a/chrome/browser/renderer_host/resource_handler.h
+++ b/chrome/browser/renderer_host/resource_handler.h
@@ -52,7 +52,7 @@ struct ResourceResponse : public base::RefCounted<ResourceResponse> {
 // The resource dispatcher host uses this interface to push load events to the
 // renderer, allowing for differences in the types of IPC messages generated.
 // See the implementations of this interface defined below.
-class ResourceHandler : public base::RefCountedThreadSafe<ResourceHandler> {
+class ResourceHandler : public base::RefCounted<ResourceHandler> {
  public:
   virtual ~ResourceHandler() {}
 
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 77e45ce..d817aa7 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -288,7 +288,7 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& msg) {
 #if defined(OS_WIN)  // This hack is Windows-specific.
       IPC_MESSAGE_HANDLER(ViewHostMsg_LoadFont, OnLoadFont)
 #endif
-      IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetPlugins, OnGetPlugins)
+      IPC_MESSAGE_HANDLER(ViewHostMsg_GetPlugins, OnGetPlugins)
       IPC_MESSAGE_HANDLER(ViewHostMsg_GetPluginPath, OnGetPluginPath)
       IPC_MESSAGE_HANDLER(ViewHostMsg_DownloadUrl, OnDownloadUrl)
       IPC_MESSAGE_HANDLER_GENERIC(ViewHostMsg_ContextMenu,
@@ -545,20 +545,8 @@ void ResourceMessageFilter::OnLoadFont(LOGFONT font) {
 #endif
 
 void ResourceMessageFilter::OnGetPlugins(bool refresh,
-                                         IPC::Message* reply_msg) {
-  ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &ResourceMessageFilter::OnGetPluginsOnFileThread,
-          refresh, reply_msg));
-}
-
-void ResourceMessageFilter::OnGetPluginsOnFileThread(bool refresh,
-                                                     IPC::Message* reply_msg) {
-  std::vector<WebPluginInfo> plugins;
-  PluginService::GetInstance()->GetPlugins(refresh, &plugins);
-
-  ViewHostMsg_GetPlugins::WriteReplyParams(reply_msg, plugins);
-  ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &ResourceMessageFilter::Send, reply_msg));
+                                         std::vector<WebPluginInfo>* plugins) {
+  plugin_service_->GetPlugins(refresh, plugins);
 }
 
 void ResourceMessageFilter::OnGetPluginPath(const GURL& url,
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index 8cdd660..e435bac 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -141,8 +141,7 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   // Not handled in the IO thread on Mac.
   void OnGetScreenInfo(gfx::NativeViewId window, IPC::Message* reply);
 #endif
-  void OnGetPlugins(bool refresh, IPC::Message* reply_msg);
-  void OnGetPluginsOnFileThread(bool refresh, IPC::Message* reply_msg);
+  void OnGetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins);
   void OnGetPluginPath(const GURL& url,
                        const GURL& policy_url,
                        const std::string& mime_type,