Allow URL requests for object/embed tags to be intercepted as streams.

content/browser/loader/buffered_resource_handler_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/loader/buffered_resource_handler.h"

[mmenke, 2014/11/17 18:57:08]

optional nit:  I believe the corresponding header file for code being tested
should appear first (as it already is) with a blank line after it.  Hrm...We're
not very consistent about that, even in this directory, that's done in only 2 of
4 test files.

[raymes, 2014/11/17 23:44:25]

Yes, I've seen lots of inconsistency in that. I added the space.

+#include "content/browser/loader/resource_dispatcher_host_impl.h"
+#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/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
+#include "net/url_request/url_request_context.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+class BufferedResourceHandlerTest : public testing::Test {
+ public:
+  BufferedResourceHandlerTest() : intercepted_as_stream_(false) {}

[mmenke, 2014/11/17 18:57:08]

nit:  Blank line after constructor.

[raymes, 2014/11/17 23:44:25]

Done.

+  void set_intercepted_as_stream() { intercepted_as_stream_ = true; }
+
+  void set_stream_has_handler(bool stream_has_handler) {
+    stream_has_handler_ = stream_has_handler;
+  }
+
+  bool TestStreamIsIntercepted(bool allow_download,
+                               bool must_download,
+                               ResourceType request_resource_type);
+
+ private:
+  // Whether the URL request has been intercepted as a stream.
+  bool intercepted_as_stream_;
+
+  TestBrowserThreadBundle thread_bundle_;
+};
+
+class TestResourceHandler : public ResourceHandler {
+ public:
+  TestResourceHandler() : ResourceHandler(nullptr) {}

[mmenke, 2014/11/17 18:57:08]

nit:  Blank lines between methods.

[raymes, 2014/11/17 23:44:25]

Done.

+  void SetController(ResourceController* controller) override {}
+  bool OnUploadProgress(uint64 position, uint64 size) override {
+    return false;
+  }
+  bool OnRequestRedirected(const net::RedirectInfo& redirect_info,
+                           ResourceResponse* response,
+                           bool* defer) override {
+    return false;
+  }
+  bool OnResponseStarted(ResourceResponse* response, bool* defer) override {
+    return false;
+  }
+  bool OnWillStart(const GURL& url, bool* defer) override {
+    return false;
+  }
+  bool OnBeforeNetworkStart(const GURL& url, bool* defer) override {
+    return false;
+  }
+  bool OnWillRead(scoped_refptr<net::IOBuffer>* buf,
+                  int* buf_size,
+                  int min_size) override {
+    return false;
+  }
+  bool OnReadCompleted(int bytes_read, bool* defer) override {
+    return false;
+  }
+  void OnResponseCompleted(const net::URLRequestStatus& status,
+                           const std::string& security_info,
+                           bool* defer) override {}
+  void OnDataDownloaded(int bytes_downloaded) override {}
+};
+
+class TestResourceDispatcherHost : public ResourceDispatcherHostImpl {
+ public:
+  TestResourceDispatcherHost(BufferedResourceHandlerTest* test,
+                             bool stream_has_handler)
+      : stream_has_handler_(stream_has_handler),
+        test_(test) {}
+  scoped_ptr<ResourceHandler> MaybeInterceptAsStream(

[mmenke, 2014/11/17 18:57:08]

nit:  blank line between methods.

[raymes, 2014/11/17 23:44:25]

Done.

+      net::URLRequest* request,
+      ResourceResponse* response,
+      std::string* payload) override {
+    if (stream_has_handler_) {
+      test_->set_intercepted_as_stream();
+      return scoped_ptr<ResourceHandler>(new TestResourceHandler).Pass();
+    } else {
+      return scoped_ptr<ResourceHandler>();
+    }
+  }
+
+ private:
+  BufferedResourceHandlerTest* test_;
+  bool stream_has_handler_;
+};
+
+class TestResourceDispatcherHostDelegate
+    : public ResourceDispatcherHostDelegate {
+ public:
+  TestResourceDispatcherHostDelegate(bool must_download)
+      : must_download_(must_download) {
+  }
+
+  bool ShouldForceDownloadResource(const GURL& url,
+                                   const std::string& mime_type) override {
+    return must_download_;
+  }
+
+ private:
+  bool must_download_;

[mmenke, 2014/11/17 18:57:08]

nit:  const?

[raymes, 2014/11/17 23:44:25]

Done.

+};
+
+bool BufferedResourceHandlerTest::TestStreamIsIntercepted(
+    bool allow_download,
+    bool must_download,
+    ResourceType request_resource_type) {
+  // In the implementation |allow_download| == true implies
+  // |request_resource_type| == RESOURCE_TYPE_MAIN_FRAME or
+  // |request_resource_type| == RESOURCE_TYPE_SUB_FRAME.
+  if (allow_download) {
+    CHECK(request_resource_type == RESOURCE_TYPE_MAIN_FRAME ||
+          request_resource_type == RESOURCE_TYPE_SUB_FRAME);

[mmenke, 2014/11/17 18:57:08]

Include base/logging.h, for CHECK...Actually, should these be switched to a
DCHECK, and moved into buffered_resource_handler.cc?

[raymes, 2014/11/17 23:44:25]

Done, I moved it into buffered_resource_handler.cc

+  }
+  intercepted_as_stream_ = false;
+
+  net::URLRequestContext context;
+  scoped_ptr<net::URLRequest> request(context.CreateRequest(
+      GURL("http://www.google.com"), net::DEFAULT_PRIORITY, nullptr, nullptr));

[mmenke, 2014/11/17 18:57:08]

include base/scoped_ptr.h and url/gurl.h

[raymes, 2014/11/17 23:44:25]

Done.

+  ResourceRequestInfo::AllocateForTesting(request.get(),

[mmenke, 2014/11/17 18:57:08]

request.get(), should be moved onto next line, aligned with other arguments.

[raymes, 2014/11/17 23:44:25]

Done.

+      request_resource_type, nullptr, 0, 0, 0, allow_download, true);
+
+  TestResourceDispatcherHost host(this, stream_has_handler_);
+  TestResourceDispatcherHostDelegate host_delegate(must_download);
+  host.SetDelegate(&host_delegate);
+
+  scoped_ptr<ResourceHandler> buffered_handler(
+      new BufferedResourceHandler(
+          scoped_ptr<ResourceHandler>(new TestResourceHandler()).Pass(),
+          &host, request.get()));
+
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  // The MIME type isn't important but it shouldn't be empty.
+  response->head.mime_type = "application/pdf";
+
+  bool defer = true;

[mmenke, 2014/11/17 18:57:08]

defer should always be initialized to false, to mimic behavior in production
code.

[raymes, 2014/11/17 23:44:25]

Done.

+  buffered_handler->OnResponseStarted(response.get(), &defer);
+
+  // Run the message loop so that tasks don't get run after destruction.
+  content::RunAllPendingInMessageLoop();
+
+  return intercepted_as_stream_;
+}
+
+// Test that stream requests are correctly intercepted under the right
+// circumstances.
+TEST_F(BufferedResourceHandlerTest, StreamHandling) {
+  bool allow_download;
+  bool must_download;
+  ResourceType resource_type;
+
+  // Ensure the stream is handled by MaybeInterceptAsStream in the
+  // ResourceDispatcherHost.
+  set_stream_has_handler(true);
+
+  // Main frame request with no download allowed. Stream shouldn't be
+  // intercepted.
+  allow_download = false;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_MAIN_FRAME;
+  ASSERT_FALSE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Main frame request with download allowed. Stream should be intercepted.
+  allow_download = true;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_MAIN_FRAME;
+  ASSERT_TRUE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Main frame request with download forced. Stream shouldn't be intercepted.
+  allow_download = true;
+  must_download = true;
+  resource_type = RESOURCE_TYPE_MAIN_FRAME;
+  ASSERT_FALSE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Sub-resource request with download not allowed. Stream shouldn't be
+  // intercepted.
+  allow_download = false;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_SUB_RESOURCE;
+  ASSERT_FALSE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // Object request with download not allowed. Stream should be intercepted.
+  allow_download = false;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_OBJECT;
+  ASSERT_TRUE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  // 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;
+  ASSERT_FALSE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+
+  allow_download = true;
+  must_download = false;
+  resource_type = RESOURCE_TYPE_MAIN_FRAME;
+  ASSERT_FALSE(
+      TestStreamIsIntercepted(allow_download, must_download, resource_type));
+}
+
+}  // namespace content