Ensuring interception of stream get determined by plugin path before checking mime type.

Earlier we are checking whether a extension with url that have mime type
should be intercepted as stream by checking plugin availablity.
Now we are passing plugin path and then checking whether their is
MimehandlerView Plugin available with that path and then we are allowing
intercepted as stream. So now interception is not depends upon mimetype.

BUG=443466

Review URL: https://codereview.chromium.org/953793003

Cr-Commit-Position: refs/heads/master@{#324187}

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

@@ -205,36 +205,6 @@ void SendExecuteMimeTypeHandlerEvent(scoped_ptr<content::StreamInfo> stream,
       extension_id, web_contents, stream.Pass(), view_id, expected_content_size,
       embedded, render_process_id, render_frame_id);
 }
-
-// TODO(raymes): This won't return the right result if plugins haven't been
-// loaded yet. Fixing this properly really requires fixing crbug.com/443466.
-bool IsPluginEnabledForExtension(const Extension* extension,
-                                 const ResourceRequestInfo* info,
-                                 const std::string& mime_type,
-                                 const GURL& url) {
-  content::PluginService* service = content::PluginService::GetInstance();
-  std::vector<content::WebPluginInfo> plugins;
-  service->GetPluginInfoArray(url, mime_type, true, &plugins, nullptr);
-  content::PluginServiceFilter* filter = service->GetFilter();
-
-  for (auto& plugin : plugins) {
-    // Check that the plugin is running the extension.
-    if (plugin.path !=
-        base::FilePath::FromUTF8Unsafe(extension->url().spec())) {
-      continue;
-    }
-    // Check that the plugin is actually enabled.
-    if (!filter || filter->IsPluginAvailable(info->GetChildID(),
-                                             info->GetRenderFrameID(),
-                                             info->GetContext(),
-                                             url,
-                                             GURL(),
-                                             &plugin)) {
-      return true;
-    }
-  }
-  return false;
-}
 #endif  // !defined(ENABLE_EXTENSIONS)
 
 #if !defined(OS_ANDROID)
@@ -601,6 +571,7 @@ bool ChromeResourceDispatcherHostDelegate::ShouldForceDownloadResource(
 
 bool ChromeResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream(
     net::URLRequest* request,
+    const base::FilePath& plugin_path,
     const std::string& mime_type,
     GURL* origin,
     std::string* payload) {
@@ -624,23 +595,27 @@ bool ChromeResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream(
          !extension_info_map->IsIncognitoEnabled(extension_id))) {
       continue;
     }
-
     MimeTypesHandler* handler = MimeTypesHandler::GetHandler(extension);
-    if (handler && handler->CanHandleMIMEType(mime_type)) {
+    // If the MimeHandlerView plugin to be loaded matches the extension,
+    // intercept the stream for that extension.
+    if (plugin_path ==
+        base::FilePath::FromUTF8Unsafe(extension->url().spec())) {
+      StreamTargetInfo target_info;
+      *origin = Extension::GetBaseURLFromExtensionId(extension_id);
+      target_info.extension_id = extension_id;
+      DCHECK(!handler->handler_url().empty());
+      target_info.view_id = base::GenerateGUID();
+      *payload = target_info.view_id;
+      stream_target_info_[request] = target_info;
+      return true;
+    } else if (plugin_path.empty() && handler &&
+               handler->CanHandleMIMEType(mime_type)) {
+      // If no plugin path is provided, then we are trying to intercept the
+      // stream for the streamsPrivate API.
       StreamTargetInfo target_info;
       *origin = Extension::GetBaseURLFromExtensionId(extension_id);
       target_info.extension_id = extension_id;
-      if (!handler->handler_url().empty()) {
-        // This is reached in the case of MimeHandlerViews. If the
-        // MimeHandlerView plugin is disabled, then we shouldn't intercept the
-        // stream.
-        if (!IsPluginEnabledForExtension(extension, info, mime_type,
-                                         request->url())) {
-          continue;
-        }
-        target_info.view_id = base::GenerateGUID();
-        *payload = target_info.view_id;
-      }
+      DCHECK(handler->handler_url().empty());
       stream_target_info_[request] = target_info;
       return true;
     }

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.h

@@ -58,6 +58,7 @@ class ChromeResourceDispatcherHostDelegate
   bool ShouldForceDownloadResource(const GURL& url,
                                    const std::string& mime_type) override;
   bool ShouldInterceptResourceAsStream(net::URLRequest* request,
+                                       const base::FilePath& plugin_path,
                                        const std::string& mime_type,
                                        GURL* origin,
                                        std::string* payload) override;

content/browser/loader/buffered_resource_handler.cc

@@ -297,6 +297,58 @@ bool BufferedResourceHandler::DetermineMimeType() {
   return made_final_decision;
 }
 
+bool BufferedResourceHandler::IsHandledByPlugin(bool* defer,
+                                                bool* request_handled) {
+#if defined(ENABLE_PLUGINS)
+  bool stale;
+  WebPluginInfo plugin;
+  bool allow_wildcard = false;
+  ResourceRequestInfoImpl* info = GetRequestInfo();
+  bool has_plugin = plugin_service_->GetPluginInfo(
+      info->GetChildID(), info->GetRenderFrameID(), info->GetContext(),
+      request()->url(), GURL(), response_->head.mime_type, allow_wildcard,
+      &stale, &plugin, NULL);
+
+  if (stale) {
+    // Refresh the plugins asynchronously.
+    plugin_service_->GetPlugins(
+        base::Bind(&BufferedResourceHandler::OnPluginsLoaded,
+                   weak_ptr_factory_.GetWeakPtr()));
+    request()->LogBlockedBy("BufferedResourceHandler");
+    *defer = true;
+    return true;
+  }
+
+  if (has_plugin) {
+    if (plugin.type == WebPluginInfo::PLUGIN_TYPE_BROWSER_PLUGIN) {
+      // If it is a MimeHandlerView plugin, intercept the stream.
+      std::string payload;
+      scoped_ptr<ResourceHandler> handler(host_->MaybeInterceptAsStream(
+          plugin.path, request(), response_.get(), &payload));
+      if (handler) {
+        *request_handled = UseAlternateNextHandler(handler.Pass(), payload);
+        return true;
+      }
+      return false;
+    } else {
+      return true;
+    }
+  }
+
+  // If execution reaches here then we should try intercepting the stream for
+  // the old streamsPrivate extensions API. This API is deprecated and should
+  // go away.
+  std::string payload;
+  scoped_ptr<ResourceHandler> handler(host_->MaybeInterceptAsStream(
+      base::FilePath(), request(), response_.get(), &payload));
+  if (handler) {
+    *request_handled = UseAlternateNextHandler(handler.Pass(), payload);
+    return true;
+  }
+#endif
+  return false;
+}
+
 bool BufferedResourceHandler::SelectNextHandler(bool* defer) {
   DCHECK(!response_->head.mime_type.empty());
 
@@ -314,13 +366,13 @@ bool BufferedResourceHandler::SelectNextHandler(bool* defer) {
   // Allow requests for object/embed tags to be intercepted as streams.
   if (info->GetResourceType() == content::RESOURCE_TYPE_OBJECT) {
     DCHECK(!info->allow_download());
-    std::string payload;
-    scoped_ptr<ResourceHandler> handler(
-        host_->MaybeInterceptAsStream(request(), response_.get(), &payload));
-    if (handler) {
-      DCHECK(!net::IsSupportedMimeType(mime_type));
-      return UseAlternateNextHandler(handler.Pass(), payload);
-    }
+
+    bool request_handled = true;
+    bool handled_by_plugin = IsHandledByPlugin(defer, &request_handled);
+    if (!request_handled)
+      return false;
+    if (handled_by_plugin)
+      return true;
   }
 
   if (!info->allow_download())
@@ -337,28 +389,12 @@ bool BufferedResourceHandler::SelectNextHandler(bool* defer) {
     if (net::IsSupportedMimeType(mime_type))
       return true;
 
-    std::string payload;
-    scoped_ptr<ResourceHandler> handler(
-        host_->MaybeInterceptAsStream(request(), response_.get(), &payload));
-    if (handler) {
-      return UseAlternateNextHandler(handler.Pass(), payload);
-    }
-
-#if defined(ENABLE_PLUGINS)
-    bool stale;
-    bool has_plugin = HasSupportingPlugin(&stale);
-    if (stale) {
-      // Refresh the plugins asynchronously.
-      plugin_service_->GetPlugins(
-          base::Bind(&BufferedResourceHandler::OnPluginsLoaded,
-                     weak_ptr_factory_.GetWeakPtr()));
-      request()->LogBlockedBy("BufferedResourceHandler");
-      *defer = true;
-      return true;
-    }
-    if (has_plugin)
+    bool request_handled = true;
+    bool handled_by_plugin = IsHandledByPlugin(defer, &request_handled);
+    if (!request_handled)
+      return false;
+    if (handled_by_plugin)
       return true;
-#endif
   }
 
   // Install download handler
@@ -475,23 +511,6 @@ bool BufferedResourceHandler::MustDownload() {
   return must_download_;
 }
 
-bool BufferedResourceHandler::HasSupportingPlugin(bool* stale) {
-#if defined(ENABLE_PLUGINS)
-  ResourceRequestInfoImpl* info = GetRequestInfo();
-
-  bool allow_wildcard = false;
-  WebPluginInfo plugin;
-  return plugin_service_->GetPluginInfo(
-      info->GetChildID(), info->GetRenderFrameID(), info->GetContext(),
-      request()->url(), GURL(), response_->head.mime_type, allow_wildcard,
-      stale, &plugin, NULL);
-#else
-  if (stale)
-    *stale = false;
-  return false;
-#endif
-}
-
 bool BufferedResourceHandler::CopyReadBufferToNextHandler() {
   if (!read_buffer_.get())
     return true;

content/browser/loader/buffered_resource_handler.h

@@ -56,6 +56,11 @@ class CONTENT_EXPORT BufferedResourceHandler
 
   bool ShouldSniffContent();
   bool DetermineMimeType();
+  // Returns true if a plugin will handle the current request. |defer| is set
+  // to true if plugin data is stale and needs to be refreshed before the
+  // request can be handled. |request_handled| is false if the request should
+  // be cancelled and true otherwise.
+  bool IsHandledByPlugin(bool* defer, bool* request_handled);
   bool SelectNextHandler(bool* defer);
   bool UseAlternateNextHandler(scoped_ptr<ResourceHandler> handler,
                                const std::string& payload_for_old_handler);
@@ -64,7 +69,6 @@ class CONTENT_EXPORT BufferedResourceHandler
   void CallReplayReadCompleted();
 
   bool MustDownload();
-  bool HasSupportingPlugin(bool* is_stale);
 
   // Copies data from |read_buffer_| to |next_handler_|.
   bool CopyReadBufferToNextHandler();

content/browser/loader/buffered_resource_handler_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/loader/buffered_resource_handler.h"
 
+#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
@@ -12,6 +13,7 @@
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/resource_response.h"
+#include "content/public/common/webplugininfo.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
 #include "content/test/fake_plugin_service.h"
@@ -84,7 +86,8 @@ class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
  public:
   explicit TestResourceDispatcherHost(bool stream_has_handler)
       : stream_has_handler_(stream_has_handler),
-        intercepted_as_stream_(false) {}
+        intercepted_as_stream_(false),
+        intercepted_as_stream_count_(0) {}
 
   bool intercepted_as_stream() const { return intercepted_as_stream_; }
 
@@ -99,9 +102,11 @@ class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
   }
 
   scoped_ptr<ResourceHandler> MaybeInterceptAsStream(
+      const base::FilePath& plugin_path,
       net::URLRequest* request,
       ResourceResponse* response,
       std::string* payload) override {
+    intercepted_as_stream_count_++;
     if (stream_has_handler_) {
       intercepted_as_stream_ = true;
       return scoped_ptr<ResourceHandler>(new TestResourceHandler).Pass();
@@ -110,12 +115,20 @@ class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
     }
   }
 
+  int intercepted_as_stream_count() const {
+    return intercepted_as_stream_count_;
+  }
+
  private:
   // Whether the URL request should be intercepted as a stream.
   bool stream_has_handler_;
 
   // Whether the URL request has been intercepted as a stream.
   bool intercepted_as_stream_;
+
+  // Count of number of times MaybeInterceptAsStream function get called in a
+  // test.
+  int intercepted_as_stream_count_;
 };
 
 class TestResourceDispatcherHostDelegate
@@ -151,14 +164,64 @@ class TestResourceController : public ResourceController {
   }
 };
 
+class TestFakePluginService : public FakePluginService {
+ public:
+  // If |is_plugin_stale| is true, GetPluginInfo will indicate the plugins are
+  // stale until GetPlugins is called.
+  TestFakePluginService(bool plugin_available, bool is_plugin_stale)
+      : plugin_available_(plugin_available),
+        is_plugin_stale_(is_plugin_stale) {}
+
+  bool GetPluginInfo(int render_process_id,
+                     int render_frame_id,
+                     ResourceContext* context,
+                     const GURL& url,
+                     const GURL& page_url,
+                     const std::string& mime_type,
+                     bool allow_wildcard,
+                     bool* is_stale,
+                     WebPluginInfo* info,
+                     std::string* actual_mime_type) override {
+    *is_stale = is_plugin_stale_;
+    if (!is_plugin_stale_ || !plugin_available_)
+      return false;
+    info->type = WebPluginInfo::PLUGIN_TYPE_BROWSER_PLUGIN;
+    info->path = base::FilePath::FromUTF8Unsafe(
+        std::string("chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/"));
+    return true;
+  }
+
+  void GetPlugins(const GetPluginsCallback& callback) override {
+    is_plugin_stale_ = false;
+    std::vector<WebPluginInfo> plugins;
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+                                           base::Bind(callback, plugins));
+  }
+
+ private:
+  const bool plugin_available_;
+  bool is_plugin_stale_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFakePluginService);
+};
+
 class BufferedResourceHandlerTest : public testing::Test {
  public:
-  BufferedResourceHandlerTest() : stream_has_handler_(false) {}
+  BufferedResourceHandlerTest()
+      : stream_has_handler_(false),
+        plugin_available_(false),
+        plugin_stale_(false) {}
 
   void set_stream_has_handler(bool stream_has_handler) {
     stream_has_handler_ = stream_has_handler;
   }
 
+  void set_plugin_available(bool plugin_available) {
+    plugin_available_ = plugin_available;
+  }
+
+  void set_plugin_stale(bool plugin_stale) { plugin_stale_ = plugin_stale; }
+
   bool TestStreamIsIntercepted(bool allow_download,
                                bool must_download,
                                ResourceType request_resource_type);
@@ -166,6 +229,8 @@ class BufferedResourceHandlerTest : public testing::Test {
  private:
   // Whether the URL request should be intercepted as a stream.
   bool stream_has_handler_;
+  bool plugin_available_;
+  bool plugin_stale_;
 
   TestBrowserThreadBundle thread_bundle_;
 };
@@ -194,7 +259,7 @@ bool BufferedResourceHandlerTest::TestStreamIsIntercepted(
   TestResourceDispatcherHostDelegate host_delegate(must_download);
   host.SetDelegate(&host_delegate);
 
-  FakePluginService plugin_service;
+  TestFakePluginService plugin_service(plugin_available_, plugin_stale_);
   scoped_ptr<ResourceHandler> buffered_handler(
       new BufferedResourceHandler(
           scoped_ptr<ResourceHandler>(new TestResourceHandler()).Pass(),
@@ -212,12 +277,13 @@ bool BufferedResourceHandlerTest::TestStreamIsIntercepted(
   buffered_handler->OnResponseStarted(response.get(), &defer);
 
   content::RunAllPendingInMessageLoop();
-
+  EXPECT_LT(host.intercepted_as_stream_count(), 2);
   return host.intercepted_as_stream();
 }
 
 // Test that stream requests are correctly intercepted under the right
-// circumstances.
+// circumstances. Test is not relevent when plugins are disabled.
+#if defined(ENABLE_PLUGINS)
 TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   bool allow_download;
   bool must_download;
@@ -226,6 +292,7 @@ TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   // Ensure the stream is handled by MaybeInterceptAsStream in the
   // ResourceDispatcherHost.
   set_stream_has_handler(true);
+  set_plugin_available(true);
 
   // Main frame request with no download allowed. Stream shouldn't be
   // intercepted.
@@ -267,7 +334,6 @@ TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   // Test the cases where the stream isn't handled by MaybeInterceptAsStream
   // in the ResourceDispatcherHost.
   set_stream_has_handler(false);
-
   allow_download = false;
   must_download = false;
   resource_type = RESOURCE_TYPE_OBJECT;
@@ -279,7 +345,29 @@ TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   resource_type = RESOURCE_TYPE_MAIN_FRAME;
   EXPECT_FALSE(
       TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Test the cases where the stream handled by MaybeInterceptAsStream
+  // with plugin not available. This is the case when intercepting streams for
+  // the streamsPrivate extensions API.
+  set_stream_has_handler(true);
+  set_plugin_available(false);
+  allow_download = false;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_OBJECT;
+  EXPECT_TRUE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Test the cases where the stream handled by MaybeInterceptAsStream
+  // with plugin not available. This is the case when intercepting streams for
+  // the streamsPrivate extensions API with stale plugin.
+  set_plugin_stale(true);
+  allow_download = false;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_OBJECT;
+  EXPECT_TRUE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
 }
+#endif
 
 }  // namespace
 

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -727,19 +727,18 @@ ResourceDispatcherHostImpl::CreateResourceHandlerForDownload(
   return handler.Pass();
 }
 
-scoped_ptr<ResourceHandler>
-ResourceDispatcherHostImpl::MaybeInterceptAsStream(net::URLRequest* request,
-                                                   ResourceResponse* response,
-                                                   std::string* payload) {
+scoped_ptr<ResourceHandler> ResourceDispatcherHostImpl::MaybeInterceptAsStream(
+    const base::FilePath& plugin_path,
+    net::URLRequest* request,
+    ResourceResponse* response,
+    std::string* payload) {
   ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest(request);
   const std::string& mime_type = response->head.mime_type;
 
   GURL origin;
   if (!delegate_ ||
-      !delegate_->ShouldInterceptResourceAsStream(request,
-                                                  mime_type,
-                                                  &origin,
-                                                  payload)) {
+      !delegate_->ShouldInterceptResourceAsStream(
+          request, plugin_path, mime_type, &origin, payload)) {
     return scoped_ptr<ResourceHandler>();
   }
 

content/browser/loader/resource_dispatcher_host_impl.h

@@ -45,6 +45,10 @@
 class ResourceHandler;
 struct ResourceHostMsg_Request;
 
+namespace base {
+class FilePath;
+}
+
 namespace net {
 class URLRequestJobFactory;
 }
@@ -242,6 +246,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // it, except on HTTP errors. This is marked virtual so it can be overriden in
   // testing.
   virtual scoped_ptr<ResourceHandler> MaybeInterceptAsStream(
+      const base::FilePath& plugin_path,
       net::URLRequest* request,
       ResourceResponse* response,
       std::string* payload);

content/public/browser/resource_dispatcher_host_delegate.cc

@@ -56,6 +56,7 @@ bool ResourceDispatcherHostDelegate::ShouldForceDownloadResource(
 
 bool ResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream(
     net::URLRequest* request,
+    const base::FilePath& plugin_path,
     const std::string& mime_type,
     GURL* origin,
     std::string* payload) {

content/public/browser/resource_dispatcher_host_delegate.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/common/content_export.h"
 #include "content/public/common/resource_type.h"
@@ -92,10 +93,12 @@ class CONTENT_EXPORT ResourceDispatcherHostDelegate {
   // If the stream will be rendered in a BrowserPlugin, |payload| will contain
   // the data that should be given to the old ResourceHandler to forward to the
   // renderer process.
-  virtual bool ShouldInterceptResourceAsStream(net::URLRequest* request,
-                                               const std::string& mime_type,
-                                               GURL* origin,
-                                               std::string* payload);
+  virtual bool ShouldInterceptResourceAsStream(
+      net::URLRequest* request,
+      const base::FilePath& plugin_path,
+      const std::string& mime_type,
+      GURL* origin,
+      std::string* payload);
 
   // Informs the delegate that a Stream was created. The Stream can be read from
   // the blob URL of the Stream, but can only be read once.