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

content/browser/loader/buffered_resource_handler_unittest.cc

Index: content/browser/loader/buffered_resource_handler_unittest.cc
diff --git a/content/browser/loader/buffered_resource_handler_unittest.cc b/content/browser/loader/buffered_resource_handler_unittest.cc
index e4d3668c371ff059657b48868c2a574c6f97dd13..89d707a72f27b44d444e509db8f27e7f71896909 100644
--- a/content/browser/loader/buffered_resource_handler_unittest.cc
+++ b/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,18 @@ class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
     }
   }
 
+  int GetInterceptAsStream() { return intercepted_as_stream_count; }

[mmenke, 2015/04/07 14:52:51]

GetInterceptAsStream -> intercepted_as_stream_count

[mmenke, 2015/04/07 14:52:51]

nit:  const

+
  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;

[mmenke, 2015/04/07 14:52:51]

nit:  intercepted_as_stream_count_

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

 };
 
 class TestResourceDispatcherHostDelegate
@@ -151,14 +162,64 @@ class TestResourceController : public ResourceController {
   }
 };
 
+class TestFakePluginService : public FakePluginService {
+ public:
+  TestFakePluginService(bool plugin_available, bool is_plugin_stale)

[mmenke, 2015/04/07 14:52:51]

Should comment about is_plugin_stale behavior.

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

+      : 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;
+    scoped_refptr<base::MessageLoopProxy> target_loop(
+        base::MessageLoop::current()->message_loop_proxy());

[mmenke, 2015/04/07 14:52:51]

No longer needed.

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

+    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 +227,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 +257,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 +275,14 @@ bool BufferedResourceHandlerTest::TestStreamIsIntercepted(
   buffered_handler->OnResponseStarted(response.get(), &defer);
 
   content::RunAllPendingInMessageLoop();
-
+  EXPECT_TRUE(host.GetInterceptAsStream() < 2);

[mmenke, 2015/04/07 14:52:51]

Prefer EXPECT_LT, since it prints out more useful information on failure.

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

   return host.intercepted_as_stream();
 }
 
 // Test that stream requests are correctly intercepted under the right
-// circumstances.
+// circumstances. Test is not relevent for Android OS as plugin is disabled and
+// always returns false.

[mmenke, 2015/04/07 14:52:51]

nit:  "as plugins are disabled"

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

+#if !defined(OS_ANDROID)

[mmenke, 2015/04/07 14:52:51]

Can we use "#if defined(ENABLE_PLUGINS)" instead (And update the comment?). 
Seems much better to gate on the feature than the platform.

[Deepak, 2015/04/07 15:27:44]

On 2015/04/07 14:52:51, mmenke wrote:

Done.

 TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   bool allow_download;
   bool must_download;
@@ -226,6 +291,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 +333,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 +344,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