[Downloads] Do not store error responses during resumption.

content/public/test/test_download_request_handler.cc

 // Copyright 2015 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/public/test/test_download_request_handler.h"
 
 #include <inttypes.h>
 #include <utility>
 
 #include "base/logging.h"
@@ skipping 131 matching lines @@
   // re-entrancy.
   void NotifyHeadersCompleteAndPrepareToRead();
 
   scoped_ptr<Parameters> parameters_;
 
   base::WeakPtr<Interceptor> interceptor_;
   net::HttpResponseInfo response_info_;
   int64_t offset_of_next_read_ = -1;
   int64_t requested_range_begin_ = -1;
   int64_t requested_range_end_ = -1;
+  bool should_report_completed_request_ = false;
   base::WeakPtrFactory<PartialResponseJob> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PartialResponseJob);
 };
 
 namespace {
 
 template <class T>
 void StoreValueAndInvokeClosure(const base::Closure& closure,
                                 T* value_receiver,
@@ skipping 53 matching lines @@
     net::NetworkDelegate* network_delegate)
     : net::URLRequestJob(request, network_delegate),
       parameters_(std::move(parameters)),
       interceptor_(interceptor),
       weak_factory_(this) {
   DCHECK(parameters_.get());
   DCHECK_LT(0, parameters_->size);
   DCHECK_NE(-1, parameters_->pattern_generator_seed);
 }
 
-TestDownloadRequestHandler::PartialResponseJob::~PartialResponseJob() {}
+TestDownloadRequestHandler::PartialResponseJob::~PartialResponseJob() {
+  if (should_report_completed_request_)
+    ReportCompletedRequest(0);
+}
 
 void TestDownloadRequestHandler::PartialResponseJob::Start() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(1) << "Starting request for " << request()->url().spec();
 
   if (parameters_->on_start_handler.is_null() || !interceptor_.get()) {
     HandleOnStartDefault();
     return;
   }
 
@@ skipping 66 matching lines @@
       parameters_->pattern_generator_seed, offset_of_next_read_, bytes_to_copy,
       buf->data());
   DVLOG(1) << "Read " << bytes_to_copy << " bytes at offset "
            << offset_of_next_read_;
   offset_of_next_read_ += bytes_to_copy;
   return bytes_to_copy;
 }
 
 void TestDownloadRequestHandler::PartialResponseJob::ReportCompletedRequest(
     int64_t transferred_byte_count) {
+  should_report_completed_request_ = false;

[svaldez, 2016/01/13 17:29:18]

Seems unnecessary since we only call this on destruction?

[asanka, 2016/01/28 02:24:17]

We also call ReportCompletedRequest when the URLRequest client reads the entire
body.

However, in retrospect, should_report_completed_request_ is a bit clumsy. I
replaced it with a running count of bytes that were successfully read and then
we are calling ReportCompletedRequest() only once during destruction.

   if (interceptor_.get()) {
     TestDownloadRequestHandler::CompletedRequest completed_request;
     completed_request.transferred_byte_count = transferred_byte_count;
     completed_request.request_headers = request()->extra_request_headers();
     interceptor_->AddCompletedRequest(completed_request);
   }
 }
 
 // static
 void TestDownloadRequestHandler::PartialResponseJob::
@@ skipping 145 matching lines @@
   // prevents injecting an error at offset 0, it makes it much easier to set up
   // parameter sets for download resumption. It means that when a request is
   // interrupted at offset O, a subsequent request for the range O-<end> won't
   // immediately interrupt as well. If we don't exclude interruptions at
   // relative offset 0, then test writers would need to reset the parameter
   // prior to each resumption rather than once at the beginning of the test.
   while (!parameters_->injected_errors.empty() &&
          parameters_->injected_errors.front().offset <= requested_range_begin_)
     parameters_->injected_errors.pop();
 
+  // From this point on, the request is considered successful.
+  should_report_completed_request_ = true;
+
   base::MessageLoop::current()->PostTask(
       FROM_HERE, base::Bind(&PartialResponseJob::NotifyHeadersComplete,
                             weak_factory_.GetWeakPtr()));
 }
 
 // static
 base::WeakPtr<TestDownloadRequestHandler::Interceptor>
 TestDownloadRequestHandler::Interceptor::Register(
     const GURL& url,
     scoped_refptr<base::SequencedTaskRunner> client_task_runner) {
@@ skipping 177 matching lines @@
   base::RunLoop run_loop;
   BrowserThread::PostTaskAndReply(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&Interceptor::GetAndResetCompletedRequests, interceptor_,
                  requests),
       run_loop.QuitClosure());
   run_loop.Run();
 }
 
 }  // namespace content