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

Side by Side Diff: content/browser/loader/resource_loader.cc

Issue 2543633004: Fix a pair of ResourceLoader cancellation/error bugs. (Closed)
Patch Set: Merge Created 4 years 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/loader/resource_loader.h" 5 #include "content/browser/loader/resource_loader.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/callback_helpers.h" 9 #include "base/callback_helpers.h"
10 #include "base/command_line.h" 10 #include "base/command_line.h"
(...skipping 332 matching lines...) Expand 10 before | Expand all | Expand 10 after
343 343
344 DVLOG(1) << "OnResponseStarted: " << request_->url().spec(); 344 DVLOG(1) << "OnResponseStarted: " << request_->url().spec();
345 345
346 if (!request_->status().is_success()) { 346 if (!request_->status().is_success()) {
347 ResponseCompleted(); 347 ResponseCompleted();
348 return; 348 return;
349 } 349 }
350 350
351 CompleteResponseStarted(); 351 CompleteResponseStarted();
352 352
353 if (is_deferred()) 353 // If the handler deferred the request, it will resume the request later. If
354 // the request was cancelled, the request will call back into |this| with a
355 // bogus read completed error.
356 if (is_deferred() || !request_->status().is_success())
354 return; 357 return;
355 358
356 if (request_->status().is_success()) 359 StartReading(false); // Read the first chunk.
357 StartReading(false); // Read the first chunk.
358 else
359 ResponseCompleted();
360 } 360 }
361 361
362 void ResourceLoader::OnReadCompleted(net::URLRequest* unused, int bytes_read) { 362 void ResourceLoader::OnReadCompleted(net::URLRequest* unused, int bytes_read) {
363 TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"), 363 TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"),
364 "ResourceLoader::OnReadCompleted"); 364 "ResourceLoader::OnReadCompleted");
365 DCHECK_EQ(request_.get(), unused); 365 DCHECK_EQ(request_.get(), unused);
366 DVLOG(1) << "OnReadCompleted: \"" << request_->url().spec() << "\"" 366 DVLOG(1) << "OnReadCompleted: \"" << request_->url().spec() << "\""
367 << " bytes_read = " << bytes_read; 367 << " bytes_read = " << bytes_read;
368 368
369 // bytes_read == -1 always implies an error. 369 // bytes_read == -1 always implies an error.
370 if (bytes_read == -1 || !request_->status().is_success()) { 370 if (bytes_read == -1 || !request_->status().is_success()) {
371 ResponseCompleted(); 371 ResponseCompleted();
372 return; 372 return;
373 } 373 }
374 374
375 CompleteRead(bytes_read); 375 CompleteRead(bytes_read);
376 376
377 // If the handler cancelled or deferred the request, do not continue 377 // If the handler cancelled or deferred the request, do not continue
378 // processing the read. If cancelled, the URLRequest has already been 378 // processing the read. If canceled, either the request will call into |this|
379 // cancelled and will schedule an erroring OnReadCompleted later. If deferred, 379 // with a bogus read error, or, if the request was completed, a task posted
380 // do nothing until resumed. 380 // from ResourceLoader::CancelREquestInternal will run OnResponseCompleted.
381 //
382 // Note: if bytes_read is 0 (EOF) and the handler defers, resumption will call
383 // ResponseCompleted().
384 if (is_deferred() || !request_->status().is_success()) 381 if (is_deferred() || !request_->status().is_success())
385 return; 382 return;
386 383
387 if (bytes_read > 0) { 384 if (bytes_read > 0) {
388 StartReading(true); // Read the next chunk. 385 StartReading(true); // Read the next chunk.
389 } else { 386 } else {
390 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 387 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
391 tracked_objects::ScopedTracker tracking_profile( 388 tracked_objects::ScopedTracker tracking_profile(
392 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 ResponseCompleted()")); 389 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 ResponseCompleted()"));
393 390
(...skipping 161 matching lines...) Expand 10 before | Expand all | Expand 10 after
555 read_deferral_start_time_ = base::TimeTicks::Now(); 552 read_deferral_start_time_ = base::TimeTicks::Now();
556 deferred_stage_ = DEFERRED_READ; // Read first chunk when resumed. 553 deferred_stage_ = DEFERRED_READ; // Read first chunk when resumed.
557 } 554 }
558 } 555 }
559 556
560 void ResourceLoader::StartReading(bool is_continuation) { 557 void ResourceLoader::StartReading(bool is_continuation) {
561 int bytes_read = 0; 558 int bytes_read = 0;
562 ReadMore(&bytes_read); 559 ReadMore(&bytes_read);
563 560
564 // If IO is pending, wait for the URLRequest to call OnReadCompleted. 561 // If IO is pending, wait for the URLRequest to call OnReadCompleted.
565 if (request_->status().is_io_pending()) 562 // On error or cancellation, wait for notification of failure.
563 if (request_->status().is_io_pending() || !request_->status().is_success())
566 return; 564 return;
567 565
568 if (!is_continuation || bytes_read <= 0) { 566 if (!is_continuation || bytes_read <= 0) {
569 OnReadCompleted(request_.get(), bytes_read); 567 OnReadCompleted(request_.get(), bytes_read);
570 } else { 568 } else {
571 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO 569 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO
572 // thread in case the URLRequest can provide data synchronously. 570 // thread in case the URLRequest can provide data synchronously.
573 base::ThreadTaskRunnerHandle::Get()->PostTask( 571 base::ThreadTaskRunnerHandle::Get()->PostTask(
574 FROM_HERE, 572 FROM_HERE,
575 base::Bind(&ResourceLoader::OnReadCompleted, 573 base::Bind(&ResourceLoader::OnReadCompleted,
(...skipping 161 matching lines...) Expand 10 before | Expand all | Expand 10 after
737 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status, 735 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status,
738 STATUS_MAX); 736 STATUS_MAX);
739 } 737 }
740 } else if (request_->response_info().unused_since_prefetch) { 738 } else if (request_->response_info().unused_since_prefetch) {
741 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time(); 739 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time();
742 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time); 740 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time);
743 } 741 }
744 } 742 }
745 743
746 } // namespace content 744 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/loader/intercepting_resource_handler_unittest.cc ('k') | content/browser/loader/resource_loader_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698