Use ByteStream in downloads system to decouple source and sink.

content/browser/renderer_host/resource_dispatcher_host_unittest.cc

Index: content/browser/renderer_host/resource_dispatcher_host_unittest.cc
diff --git a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
index 344bc6b2c4e59019ab06ff75cc699d12f9f02dad..eeda586337b6a6a63258faaf6b32a7205aeb8d71 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc
@@ -265,6 +265,27 @@ class URLRequestTestDelayedStartJob : public net::URLRequestTestJob {
 URLRequestTestDelayedStartJob*
 URLRequestTestDelayedStartJob::list_head_ = NULL;
 
+// This class is a variation on URLRequestTestJob in that it only

[rvargas (doing something else), 2012/05/29 18:13:36]

nit: remove "only" ?

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Done.

+// returns IO_pending errors before every read, not just the first one.
+class URLRequestTestDelayedCompleteJob : public net::URLRequestTestJob {

[rvargas (doing something else), 2012/05/29 18:13:36]

...DelayedCompletion... ? DelayCompletion ?

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Went with URLRequestTestDelayedCompletionJob; let me know if that's not ok.

+ public:
+  URLRequestTestDelayedCompleteJob(net::URLRequest* request)

[rvargas (doing something else), 2012/05/29 18:13:36]

explicit

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Whoops; thank you.

+      : net::URLRequestTestJob(request) { }

[rvargas (doing something else), 2012/05/29 18:13:36]

nit: no space between {} (same thing below)

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Done.

+  URLRequestTestDelayedCompleteJob(net::URLRequest* request, bool auto_advance)
+      : net::URLRequestTestJob(request, auto_advance) { }
+  URLRequestTestDelayedCompleteJob(net::URLRequest* request,
+                                const std::string& response_headers,

[rvargas (doing something else), 2012/05/29 18:13:36]

nit: indent three more spaces

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Done; sorry.

+                                const std::string& response_data,
+                                bool auto_advance)
+      : net::URLRequestTestJob(
+          request, response_headers, response_data, auto_advance) { }

[rvargas (doing something else), 2012/05/29 18:13:36]

nit: arguments start on the previous line

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Done.

+
+ private:
+  bool FinalRoundIOPending() { return true; }

[rvargas (doing something else), 2012/05/29 18:32:13]

I forgot about this one:
nit: virtual.

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Also added OVERRIDE, now that you got me thinking in that direction.  Done.

+};
+
+
+
 // Associated with an URLRequest to determine if the URLRequest gets deleted.
 class TestUserData : public base::SupportsUserData::Data {
  public:
@@ -358,6 +379,7 @@ class ResourceDispatcherHostTest : public testing::Test,
         &ResourceDispatcherHostTest::Factory);
     EnsureTestSchemeIsAllowed();
     delay_start_ = false;
+    delay_complete_ = false;
   }
 
   virtual void TearDown() {
@@ -439,6 +461,8 @@ class ResourceDispatcherHostTest : public testing::Test,
     if (test_fixture_->response_headers_.empty()) {
       if (delay_start_) {
         return new URLRequestTestDelayedStartJob(request);
+      } else if (delay_complete_) {
+        return new URLRequestTestDelayedCompleteJob(request);
       } else {
         return new net::URLRequestTestJob(request);
       }
@@ -447,6 +471,10 @@ class ResourceDispatcherHostTest : public testing::Test,
         return new URLRequestTestDelayedStartJob(
             request, test_fixture_->response_headers_,
             test_fixture_->response_data_, false);
+      } else if (delay_complete_) {
+        return new URLRequestTestDelayedCompleteJob(
+            request, test_fixture_->response_headers_,
+            test_fixture_->response_data_, false);
       } else {
         return new net::URLRequestTestJob(request,
                                           test_fixture_->response_headers_,
@@ -460,6 +488,10 @@ class ResourceDispatcherHostTest : public testing::Test,
     delay_start_ = delay_job_start;
   }
 
+  void SetDelayedCompleteJobGeneration(bool delay_job_complete) {
+    delay_complete_ = delay_job_complete;
+  }
+
   MessageLoopForIO message_loop_;
   BrowserThreadImpl ui_thread_;
   BrowserThreadImpl file_thread_;
@@ -476,10 +508,12 @@ class ResourceDispatcherHostTest : public testing::Test,
   ResourceType::Type resource_type_;
   static ResourceDispatcherHostTest* test_fixture_;
   static bool delay_start_;
+  static bool delay_complete_;
 };
 // Static.
 ResourceDispatcherHostTest* ResourceDispatcherHostTest::test_fixture_ = NULL;
 bool ResourceDispatcherHostTest::delay_start_ = false;
+bool ResourceDispatcherHostTest::delay_complete_ = false;
 
 void ResourceDispatcherHostTest::MakeTestRequest(int render_view_id,
                                                  int request_id,
@@ -1267,9 +1301,12 @@ TEST_F(ResourceDispatcherHostTest, IgnoreCancelForDownloads) {
   std::string raw_headers(net::HttpUtil::AssembleRawHeaders(response.data(),
                                                             response.size()));
   std::string response_data("01234567890123456789\x01foobar");
+  // Get past sniffing metrics.
+  response_data.resize(1025, ' ');

[rvargas (doing something else), 2012/05/29 18:13:36]

It seems quite fragile. Hopefully the failure will be clear when someone is
refactoring that logic.

[Randy Smith (Not in Mondays), 2012/05/30 00:22:32]

Completely fair, but I'm not coming up with a good way around it.  I guess the
failure mode would be that the DownloadResourceHandler is never invoked, and we
could put some tracking in to make sure that actually happens.  If it does
happen in the current situation it won't be a download request, so the ignoring
of the cancel test will fail.  I'm not sure how obvious it'll be why, though
:-{.  

Having written the above, I think I'll amplify this comment.  I can't think of
anything better to do, though.

 
   SetResponse(raw_headers, response_data);
   SetResourceType(ResourceType::MAIN_FRAME);
+  SetDelayedCompleteJobGeneration(true);
   HandleScheme("http");
 
   MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));
@@ -1302,9 +1339,12 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContext) {
   std::string raw_headers(net::HttpUtil::AssembleRawHeaders(response.data(),
                                                             response.size()));
   std::string response_data("01234567890123456789\x01foobar");
+  // Get past sniffing metrics.
+  response_data.resize(1025, ' ');
 
   SetResponse(raw_headers, response_data);
   SetResourceType(ResourceType::MAIN_FRAME);
+  SetDelayedCompleteJobGeneration(true);
   HandleScheme("http");
 
   MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah"));