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

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

Issue 2542843006: ResourceLoader: Fix a bunch of double-cancellation/double-error notification cases. (Closed)
Patch Set: Inline ReadMore 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 339 matching lines...) Expand 10 before | Expand all | Expand 10 after
350 } 350 }
351 351
352 CompleteResponseStarted(); 352 CompleteResponseStarted();
353 353
354 // If the handler deferred the request, it will resume the request later. If 354 // If the handler deferred the request, it will resume the request later. If
355 // the request was cancelled, the request will call back into |this| with a 355 // the request was cancelled, the request will call back into |this| with a
356 // bogus read completed error. 356 // bogus read completed error.
357 if (is_deferred() || !request_->status().is_success()) 357 if (is_deferred() || !request_->status().is_success())
358 return; 358 return;
359 359
360 StartReading(false); // Read the first chunk. 360 ReadMore(false); // Read the first chunk.
361 } 361 }
362 362
363 void ResourceLoader::OnReadCompleted(net::URLRequest* unused, int bytes_read) { 363 void ResourceLoader::OnReadCompleted(net::URLRequest* unused, int bytes_read) {
364 TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"), 364 TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"),
365 "ResourceLoader::OnReadCompleted"); 365 "ResourceLoader::OnReadCompleted");
366 DCHECK_EQ(request_.get(), unused); 366 DCHECK_EQ(request_.get(), unused);
367 DVLOG(1) << "OnReadCompleted: \"" << request_->url().spec() << "\"" 367 DVLOG(1) << "OnReadCompleted: \"" << request_->url().spec() << "\""
368 << " bytes_read = " << bytes_read; 368 << " bytes_read = " << bytes_read;
369 369
370 // bytes_read == -1 always implies an error. 370 // bytes_read == -1 always implies an error.
371 if (bytes_read == -1 || !request_->status().is_success()) { 371 if (bytes_read == -1 || !request_->status().is_success()) {
372 ResponseCompleted(); 372 ResponseCompleted();
373 return; 373 return;
374 } 374 }
375 375
376 CompleteRead(bytes_read); 376 CompleteRead(bytes_read);
377 377
378 // If the handler cancelled or deferred the request, do not continue 378 // If the handler cancelled or deferred the request, do not continue
379 // processing the read. If cancelled, the URLRequest has already been 379 // processing the read. If cancelled, the URLRequest has already been
380 // cancelled and either the request will call into |this| with a bogus 380 // cancelled and either the request will call into |this| with a bogus
381 // read error, or a posted task will run OnResponseCompleted, if the 381 // read error, or a posted task will run OnResponseCompleted, if the
382 // request was completed. If deferred, nothing to do until resumed. 382 // request was completed. If deferred, nothing to do until resumed.
383 if (is_deferred() || !request_->status().is_success()) 383 if (is_deferred() || !request_->status().is_success())
384 return; 384 return;
385 385
386 if (bytes_read > 0) { 386 if (bytes_read > 0) {
387 StartReading(true); // Read the next chunk. 387 ReadMore(true); // Read the next chunk.
388 } else { 388 } else {
389 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 389 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
390 tracked_objects::ScopedTracker tracking_profile( 390 tracked_objects::ScopedTracker tracking_profile(
391 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 ResponseCompleted()")); 391 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 ResponseCompleted()"));
392 392
393 // URLRequest reported an EOF. Call ResponseCompleted. 393 // URLRequest reported an EOF. Call ResponseCompleted.
394 DCHECK_EQ(0, bytes_read); 394 DCHECK_EQ(0, bytes_read);
395 ResponseCompleted(); 395 ResponseCompleted();
396 } 396 }
397 } 397 }
(...skipping 151 matching lines...) Expand 10 before | Expand all | Expand 10 after
549 549
550 bool defer = false; 550 bool defer = false;
551 if (!handler_->OnResponseStarted(response.get(), &defer)) { 551 if (!handler_->OnResponseStarted(response.get(), &defer)) {
552 Cancel(); 552 Cancel();
553 } else if (defer) { 553 } else if (defer) {
554 read_deferral_start_time_ = base::TimeTicks::Now(); 554 read_deferral_start_time_ = base::TimeTicks::Now();
555 deferred_stage_ = DEFERRED_READ; // Read first chunk when resumed. 555 deferred_stage_ = DEFERRED_READ; // Read first chunk when resumed.
556 } 556 }
557 } 557 }
558 558
559 void ResourceLoader::StartReading(bool is_continuation) { 559 void ResourceLoader::ReadMore(bool is_continuation) {
560 int bytes_read = 0;
561 ReadMore(&bytes_read);
562
563 // If IO is pending, wait for the URLRequest to call OnReadCompleted.
564 // On error or cancellation, wait for notification of failure.
565 if (request_->status().is_io_pending() || !request_->status().is_success())
566 return;
567
568 if (!is_continuation || bytes_read <= 0) {
569 OnReadCompleted(request_.get(), bytes_read);
570 } else {
571 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO
572 // thread in case the URLRequest can provide data synchronously.
573 base::ThreadTaskRunnerHandle::Get()->PostTask(
574 FROM_HERE,
575 base::Bind(&ResourceLoader::OnReadCompleted,
576 weak_ptr_factory_.GetWeakPtr(), request_.get(), bytes_read));
577 }
578 }
579
580 void ResourceLoader::ResumeReading() {
581 DCHECK(!is_deferred());
582
583 if (!read_deferral_start_time_.is_null()) {
584 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral",
585 base::TimeTicks::Now() - read_deferral_start_time_);
586 read_deferral_start_time_ = base::TimeTicks();
587 }
588 if (request_->status().is_success()) {
589 StartReading(false); // Read the next chunk (OK to complete synchronously).
590 } else {
591 ResponseCompleted();
592 }
593 }
594
595 void ResourceLoader::ReadMore(int* bytes_read) {
596 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", this, 560 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", this,
597 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 561 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
598 DCHECK(!is_deferred()); 562 DCHECK(!is_deferred());
599 563
600 // Make sure we track the buffer in at least one place. This ensures it gets 564 // Make sure we track the buffer in at least one place. This ensures it gets
601 // deleted even in the case the request has already finished its job and 565 // deleted even in the case the request has already finished its job and
602 // doesn't use the buffer. 566 // doesn't use the buffer.
603 scoped_refptr<net::IOBuffer> buf; 567 scoped_refptr<net::IOBuffer> buf;
604 int buf_size; 568 int buf_size;
605 { 569 {
606 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 570 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
607 tracked_objects::ScopedTracker tracking_profile2( 571 tracked_objects::ScopedTracker tracking_profile2(
608 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnWillRead()")); 572 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnWillRead()"));
609 573
610 if (!handler_->OnWillRead(&buf, &buf_size, -1)) { 574 if (!handler_->OnWillRead(&buf, &buf_size, -1)) {
575 // Cancel the request, which will then call back into |this| to it of
576 // a "read error".
611 Cancel(); 577 Cancel();
612 return; 578 return;
613 } 579 }
614 } 580 }
615 581
616 DCHECK(buf.get()); 582 DCHECK(buf.get());
617 DCHECK(buf_size > 0); 583 DCHECK(buf_size > 0);
618 584
619 request_->Read(buf.get(), buf_size, bytes_read); 585 int result = request_->Read(buf.get(), buf_size);
620 586
621 // No need to check the return value here as we'll detect errors by 587 if (result == net::ERR_IO_PENDING)
622 // inspecting the URLRequest's status. 588 return;
589
590 if (!is_continuation || result <= 0) {
591 OnReadCompleted(request_.get(), result);
592 } else {
593 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO
594 // thread in case the URLRequest can provide data synchronously.
595 base::ThreadTaskRunnerHandle::Get()->PostTask(
596 FROM_HERE,
597 base::Bind(&ResourceLoader::OnReadCompleted,
598 weak_ptr_factory_.GetWeakPtr(), request_.get(), result));
599 }
600 }
601
602 void ResourceLoader::ResumeReading() {
603 DCHECK(!is_deferred());
604
605 if (!read_deferral_start_time_.is_null()) {
606 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral",
607 base::TimeTicks::Now() - read_deferral_start_time_);
608 read_deferral_start_time_ = base::TimeTicks();
609 }
610 if (request_->status().is_success()) {
611 ReadMore(false); // Read the next chunk (OK to complete synchronously).
612 } else {
613 ResponseCompleted();
614 }
623 } 615 }
624 616
625 void ResourceLoader::CompleteRead(int bytes_read) { 617 void ResourceLoader::CompleteRead(int bytes_read) {
626 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this, 618 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this,
627 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 619 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
628 620
629 DCHECK(bytes_read >= 0); 621 DCHECK(bytes_read >= 0);
630 DCHECK(request_->status().is_success()); 622 DCHECK(request_->status().is_success());
631 623
632 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 624 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
(...skipping 87 matching lines...) Expand 10 before | Expand all | Expand 10 after
720 } 712 }
721 713
722 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", status, STATUS_MAX); 714 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", status, STATUS_MAX);
723 } else if (request_->response_info().unused_since_prefetch) { 715 } else if (request_->response_info().unused_since_prefetch) {
724 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time(); 716 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time();
725 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time); 717 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time);
726 } 718 }
727 } 719 }
728 720
729 } // namespace content 721 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698