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..6c6c4f1d74cb8309a87d9610b40086d41cc75b96 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"
@@ -99,6 +101,7 @@ class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
   }
 
   scoped_ptr<ResourceHandler> MaybeInterceptAsStream(
+      const base::FilePath& plugin_path,
       net::URLRequest* request,
       ResourceResponse* response,
       std::string* payload) override {

[mmenke, 2015/03/30 01:17:38]

Could you add an EXPECT that this method is only invoked once?  Just concerned
that the new test wouldn't catch the case of us incorrectly calling this method
twice.

This shouldn't be called more than once in any existing test (and if a test
every wanted to do so, intercepted_as_stream_ being a bool and not a count would
be a little weird).

[Deepak, 2015/04/07 10:21:16]

On 2015/03/30 01:17:38, mmenke wrote:

Done.

@@ -151,14 +154,63 @@ class TestResourceController : public ResourceController {
   }
 };
 
+class TestFakePluginService : public FakePluginService {
+ public:
+  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_;

[raymes, 2015/03/30 01:00:07]

nit: if it's stale, we should return false

[Deepak, 2015/04/07 10:21:16]

On 2015/03/30 01:00:07, raymes wrote:

Done.

+    if (!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());
+    std::vector<WebPluginInfo> plugins;

[raymes, 2015/03/30 01:00:07]

We should add a valid plugin here to make sure the test works.

[Deepak, 2015/04/07 10:21:16]

On 2015/03/30 01:00:07, raymes wrote:

Actually onPluginLoad() does not use Plugin vector, mmenke asked me to pass
empty vector list.

+    target_loop->PostTask(FROM_HERE, base::Bind(callback, plugins));

[raymes, 2015/03/30 01:00:07]

it should be sufficient to use base::MessageLoop::current()->PostTask

[Deepak, 2015/04/07 10:21:16]

On 2015/03/30 01:00:07, raymes wrote:

Done.

+  }
+
+ 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 +218,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 +248,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(),
@@ -217,7 +271,9 @@ bool BufferedResourceHandlerTest::TestStreamIsIntercepted(
 }
 
 // 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.
+#if !defined(OS_ANDROID)
 TEST_F(BufferedResourceHandlerTest, StreamHandling) {
   bool allow_download;
   bool must_download;
@@ -226,6 +282,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 +324,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 +335,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