Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(200)

Side by Side Diff: content/browser/download/download_request_core.cc

Issue 2832223004: interrupt and resume download with CONTENT_LENGTH_MISMATCH errors (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/browser/download/download_request_core.h" 5 #include "content/browser/download/download_request_core.h"
6 6
7 #include <string> 7 #include <string>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/callback_helpers.h" 10 #include "base/callback_helpers.h"
(...skipping 380 matching lines...) Expand 10 before | Expand all | Expand 10 after
391 391
392 void DownloadRequestCore::OnResponseCompleted( 392 void DownloadRequestCore::OnResponseCompleted(
393 const net::URLRequestStatus& status) { 393 const net::URLRequestStatus& status) {
394 DCHECK_CURRENTLY_ON(BrowserThread::IO); 394 DCHECK_CURRENTLY_ON(BrowserThread::IO);
395 int response_code = status.is_success() ? request()->GetResponseCode() : 0; 395 int response_code = status.is_success() ? request()->GetResponseCode() : 0;
396 DVLOG(20) << __func__ << "() " << DebugString() 396 DVLOG(20) << __func__ << "() " << DebugString()
397 << " status.status() = " << status.status() 397 << " status.status() = " << status.status()
398 << " status.error() = " << status.error() 398 << " status.error() = " << status.error()
399 << " response_code = " << response_code; 399 << " response_code = " << response_code;
400 400
401 DownloadInterruptReason reason = HandleRequestStatus(status); 401 DownloadInterruptReason reason = HandleRequestStatus(
402 status, request()->response_headers()->HasStrongValidators());
402 403
403 if (status.error() == net::ERR_ABORTED) { 404 if (status.error() == net::ERR_ABORTED) {
404 // ERR_ABORTED == something outside of the network 405 // ERR_ABORTED == something outside of the network
405 // stack cancelled the request. There aren't that many things that 406 // stack cancelled the request. There aren't that many things that
406 // could do this to a download request (whose lifetime is separated from 407 // could do this to a download request (whose lifetime is separated from
407 // the tab from which it came). We map this to USER_CANCELLED as the 408 // the tab from which it came). We map this to USER_CANCELLED as the
408 // case we know about (system suspend because of laptop close) corresponds 409 // case we know about (system suspend because of laptop close) corresponds
409 // to a user action. 410 // to a user action.
410 // TODO(asanka): A lid close or other power event should result in an 411 // TODO(asanka): A lid close or other power event should result in an
411 // interruption that doesn't discard the partial state, unlike 412 // interruption that doesn't discard the partial state, unlike
(...skipping 81 matching lines...) Expand 10 before | Expand all | Expand 10 after
493 " this=%p " 494 " this=%p "
494 " url_ = " 495 " url_ = "
495 "\"%s\"" 496 "\"%s\""
496 " }", 497 " }",
497 reinterpret_cast<const void*>(this), 498 reinterpret_cast<const void*>(this),
498 request() ? request()->url().spec().c_str() : "<NULL request>"); 499 request() ? request()->url().spec().c_str() : "<NULL request>");
499 } 500 }
500 501
501 // static 502 // static
502 DownloadInterruptReason DownloadRequestCore::HandleRequestStatus( 503 DownloadInterruptReason DownloadRequestCore::HandleRequestStatus(
503 const net::URLRequestStatus& status) { 504 const net::URLRequestStatus& status, bool has_strong_validators) {
504 net::Error error_code = net::OK; 505 net::Error error_code = net::OK;
505 if (!status.is_success()) { 506 if (!status.is_success()) {
506 error_code = static_cast<net::Error>(status.error()); // Normal case. 507 error_code = static_cast<net::Error>(status.error()); // Normal case.
507 // Make sure that at least the fact of failure comes through. 508 // Make sure that at least the fact of failure comes through.
508 if (error_code == net::OK) 509 if (error_code == net::OK)
509 error_code = net::ERR_FAILED; 510 error_code = net::ERR_FAILED;
510 } 511 }
511 512
512 // ERR_CONTENT_LENGTH_MISMATCH is allowed since a number of servers in the 513 // ERR_CONTENT_LENGTH_MISMATCH can be caused by 1 of the following reasons:
513 // wild close the connection too early by mistake. Other browsers - IE9, 514 // 1. server close the connection too early.
514 // Firefox 11.0, and Safari 5.1.4 - treat downloads as complete in both cases, 515 // 2. The content-length header is wrong.
515 // so we follow their lead. 516 // If the download has strong validators, we can interrupt the download
516 if (error_code == net::ERR_CONTENT_LENGTH_MISMATCH) 517 // and let it resume automatically. Otherwise, resuming the download will
518 // cause it to restart and the download may never complete if the error was
519 // caused by reason 2. As a result, downloads without strong validators are
520 // treated as completed here.
521 // TODO(qinmin): check the metrics from downloads with strong validators,
522 // and decide whether we should interrupt downloads without strong validators
523 // rather than complete them.
524 if (error_code == net::ERR_CONTENT_LENGTH_MISMATCH &&
525 !has_strong_validators) {
517 error_code = net::OK; 526 error_code = net::OK;
527 RecordDownloadCount(COMPLETED_WITH_CONTENT_LENGTH_MISMATCH_COUNT);
528 }
529
518 DownloadInterruptReason reason = ConvertNetErrorToInterruptReason( 530 DownloadInterruptReason reason = ConvertNetErrorToInterruptReason(
519 error_code, DOWNLOAD_INTERRUPT_FROM_NETWORK); 531 error_code, DOWNLOAD_INTERRUPT_FROM_NETWORK);
520 532
521 return reason; 533 return reason;
522 } 534 }
523 535
524 // static 536 // static
525 DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse( 537 DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
526 const net::HttpResponseHeaders& http_headers, 538 const net::HttpResponseHeaders& http_headers,
527 DownloadSaveInfo* save_info) { 539 DownloadSaveInfo* save_info) {
(...skipping 148 matching lines...) Expand 10 before | Expand all | Expand 10 after
676 // old servers that didn't implement "If-Match" and must be ignored when 688 // old servers that didn't implement "If-Match" and must be ignored when
677 // "If-Match" presents. 689 // "If-Match" presents.
678 if (has_last_modified) { 690 if (has_last_modified) {
679 request->SetExtraRequestHeaderByName( 691 request->SetExtraRequestHeaderByName(
680 net::HttpRequestHeaders::kIfUnmodifiedSince, params->last_modified(), 692 net::HttpRequestHeaders::kIfUnmodifiedSince, params->last_modified(),
681 true); 693 true);
682 } 694 }
683 } 695 }
684 696
685 } // namespace content 697 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698