Use ByteStream in downloads system to decouple source and sink.

content/browser/renderer_host/resource_dispatcher_host_unittest.cc

 // Copyright (c) 2012 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/renderer_host/resource_dispatcher_host_impl.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/file_path.h"
@@ skipping 247 matching lines @@
     list_head_ = this;
   }
 
   static URLRequestTestDelayedStartJob* list_head_;
   URLRequestTestDelayedStartJob* next_;
 };
 
 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:
   explicit TestUserData(bool* was_deleted)
       : was_deleted_(was_deleted) {
   }
 
   ~TestUserData() {
     *was_deleted_ = true;
   }
@@ skipping 73 matching lines @@
   // testing::Test
   virtual void SetUp() {
     DCHECK(!test_fixture_);
     test_fixture_ = this;
     ChildProcessSecurityPolicyImpl::GetInstance()->Add(0);
     net::URLRequest::Deprecated::RegisterProtocolFactory(
         "test",
         &ResourceDispatcherHostTest::Factory);
     EnsureTestSchemeIsAllowed();
     delay_start_ = false;
+    delay_complete_ = false;
   }
 
   virtual void TearDown() {
     net::URLRequest::Deprecated::RegisterProtocolFactory("test", NULL);
     if (!scheme_.empty())
       net::URLRequest::Deprecated::RegisterProtocolFactory(
           scheme_, old_factory_);
 
     EXPECT_TRUE(URLRequestTestDelayedStartJob::DelayedStartQueueEmpty());
     URLRequestTestDelayedStartJob::ClearQueue();
@@ skipping 61 matching lines @@
     old_factory_ = net::URLRequest::Deprecated::RegisterProtocolFactory(
         scheme_, &ResourceDispatcherHostTest::Factory);
   }
 
   // Our own net::URLRequestJob factory.
   static net::URLRequestJob* Factory(net::URLRequest* request,
                                      const std::string& scheme) {
     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);
       }
     } else {
       if (delay_start_) {
         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_,
                                           test_fixture_->response_data_,
                                           false);
       }
     }
   }
 
   void SetDelayedStartJobGeneration(bool delay_job_start) {
     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_;
   BrowserThreadImpl cache_thread_;
   BrowserThreadImpl io_thread_;
   scoped_ptr<TestBrowserContext> browser_context_;
   scoped_refptr<ForwardingFilter> filter_;
   ResourceDispatcherHostImpl host_;
   ResourceIPCAccumulator accum_;
   std::string response_headers_;
   std::string response_data_;
   std::string scheme_;
   net::URLRequest::ProtocolFactory* old_factory_;
   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,
                                                  const GURL& url) {
   MakeTestRequest(filter_.get(), render_view_id, request_id, url);
 }
 
 void ResourceDispatcherHostTest::MakeTestRequest(
     ResourceMessageFilter* filter,
     int render_view_id,
@@ skipping 767 matching lines @@
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
   int request_id = 1;
 
   std::string response("HTTP\n"
                        "Content-disposition: attachment; filename=foo\n\n");
   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"));
   // Return some data so that the request is identified as a download
   // and the proper resource handlers are created.
   EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
 
   // And now simulate a cancellation coming from the renderer.
   ResourceHostMsg_CancelRequest msg(filter_->child_id(), request_id);
   bool msg_was_ok;
@@ skipping 12 matching lines @@
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
   int request_id = 1;
 
   std::string response("HTTP\n"
                        "Content-disposition: attachment; filename=foo\n\n");
   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"));
   // Return some data so that the request is identified as a download
   // and the proper resource handlers are created.
   EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
 
   // And now simulate a cancellation coming from the renderer.
   ResourceHostMsg_CancelRequest msg(filter_->child_id(), request_id);
   bool msg_was_ok;
@@ skipping 82 matching lines @@
   PickleIterator iter(msgs[0][0]);
   EXPECT_TRUE(IPC::ReadParam(&msgs[0][0], &iter, &request_id));
   EXPECT_TRUE(IPC::ReadParam(&msgs[0][0], &iter, &status));
 
   EXPECT_EQ(1, request_id);
   EXPECT_EQ(net::URLRequestStatus::FAILED, status.status());
   EXPECT_EQ(net::ERR_UNKNOWN_URL_SCHEME, status.error());
 }
 
 }  // namespace content