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

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

Issue 2668603003: Make ResourceHandler::OnWillRead able to complete asynchronously. (Closed)
Patch Set: One bot doesn't like 256 day timers. :( Created 3 years, 10 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/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 495 matching lines...) Expand 10 before | Expand all | Expand 10 after
506 // won't result in synchronously calling into a ResourceHandler, if this 506 // won't result in synchronously calling into a ResourceHandler, if this
507 // was called from Resume(). 507 // was called from Resume().
508 StartRequestInternal(); 508 StartRequestInternal();
509 break; 509 break;
510 case DEFERRED_REDIRECT: 510 case DEFERRED_REDIRECT:
511 // URLRequest::Start completes asynchronously, so starting the request now 511 // URLRequest::Start completes asynchronously, so starting the request now
512 // won't result in synchronously calling into a ResourceHandler, if this 512 // won't result in synchronously calling into a ResourceHandler, if this
513 // was called from Resume(). 513 // was called from Resume().
514 FollowDeferredRedirectInternal(); 514 FollowDeferredRedirectInternal();
515 break; 515 break;
516 case DEFERRED_ON_WILL_READ:
517 // Always post a task, as synchronous resumes don't go through this
518 // method.
519 base::ThreadTaskRunnerHandle::Get()->PostTask(
520 FROM_HERE, base::Bind(&ResourceLoader::ReadMore,
521 weak_ptr_factory_.GetWeakPtr(), false));
522 break;
516 case DEFERRED_READ: 523 case DEFERRED_READ:
517 if (called_from_resource_controller) { 524 if (called_from_resource_controller) {
518 // TODO(mmenke): Call ReadMore instead? Strange that this is the only 525 // TODO(mmenke): Call PrepareToReadMore instead? Strange that this is
519 // path which calls different methods, depending on the path. 526 // the only case which calls different methods, depending on the path.
527 // ResumeReading does check for cancellation. Should other paths do that
528 // as well?
520 base::ThreadTaskRunnerHandle::Get()->PostTask( 529 base::ThreadTaskRunnerHandle::Get()->PostTask(
521 FROM_HERE, base::Bind(&ResourceLoader::ResumeReading, 530 FROM_HERE, base::Bind(&ResourceLoader::ResumeReading,
522 weak_ptr_factory_.GetWeakPtr())); 531 weak_ptr_factory_.GetWeakPtr()));
523 } else { 532 } else {
524 // If this was called as a result of a handler succeeding synchronously, 533 // If this was called as a result of a handler succeeding synchronously,
525 // force the result of the next read to be handled asynchronously, to 534 // force the result of the next read to be handled asynchronously, to
526 // avoid blocking the IO thread. 535 // avoid blocking the IO thread.
527 ReadMore(true /* handle_result_asynchronously */); 536 PrepareToReadMore(true /* handle_result_asynchronously */);
528 } 537 }
529 break; 538 break;
530 case DEFERRED_RESPONSE_COMPLETE: 539 case DEFERRED_RESPONSE_COMPLETE:
531 if (called_from_resource_controller) { 540 if (called_from_resource_controller) {
532 base::ThreadTaskRunnerHandle::Get()->PostTask( 541 base::ThreadTaskRunnerHandle::Get()->PostTask(
533 FROM_HERE, base::Bind(&ResourceLoader::ResponseCompleted, 542 FROM_HERE, base::Bind(&ResourceLoader::ResponseCompleted,
534 weak_ptr_factory_.GetWeakPtr())); 543 weak_ptr_factory_.GetWeakPtr()));
535 } else { 544 } else {
536 ResponseCompleted(); 545 ResponseCompleted();
537 } 546 }
(...skipping 102 matching lines...) Expand 10 before | Expand all | Expand 10 after
640 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 649 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
641 tracked_objects::ScopedTracker tracking_profile( 650 tracked_objects::ScopedTracker tracking_profile(
642 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnResponseStarted()")); 651 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnResponseStarted()"));
643 652
644 read_deferral_start_time_ = base::TimeTicks::Now(); 653 read_deferral_start_time_ = base::TimeTicks::Now();
645 ScopedDeferral scoped_deferral(this, DEFERRED_READ); 654 ScopedDeferral scoped_deferral(this, DEFERRED_READ);
646 handler_->OnResponseStarted(response.get(), 655 handler_->OnResponseStarted(response.get(),
647 base::MakeUnique<Controller>(this)); 656 base::MakeUnique<Controller>(this));
648 } 657 }
649 658
650 void ResourceLoader::ReadMore(bool handle_result_async) { 659 void ResourceLoader::PrepareToReadMore(bool handle_result_async) {
651 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", this, 660 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::PrepareToReadMore", this,
652 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 661 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
653 DCHECK(!is_deferred()); 662 DCHECK(!is_deferred());
654 663
655 // Make sure we track the buffer in at least one place. This ensures it gets 664 deferred_stage_ = DEFERRED_SYNC;
656 // deleted even in the case the request has already finished its job and 665
657 // doesn't use the buffer.
658 scoped_refptr<net::IOBuffer> buf;
659 int buf_size;
660 { 666 {
661 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed. 667 // TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
662 tracked_objects::ScopedTracker tracking_profile2( 668 tracked_objects::ScopedTracker tracking_profile2(
663 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnWillRead()")); 669 FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnWillRead()"));
664 670
665 if (!handler_->OnWillRead(&buf, &buf_size)) { 671 handler_->OnWillRead(&read_buffer_, &read_buffer_size_,
666 // Cancel the request, which will then call back into |this| to inform it 672 base::MakeUnique<Controller>(this));
667 // of a "read error".
668 Cancel();
669 return;
670 }
671 } 673 }
672 674
673 DCHECK(buf.get()); 675 if (is_deferred()) {
674 DCHECK(buf_size > 0); 676 deferred_stage_ = DEFERRED_ON_WILL_READ;
677 } else {
678 ReadMore(handle_result_async);
679 }
680 }
675 681
676 int result = request_->Read(buf.get(), buf_size); 682 void ResourceLoader::ReadMore(bool handle_result_async) {
683 DCHECK(read_buffer_.get());
684 DCHECK_GT(read_buffer_size_, 0);
685
686 int result = request_->Read(read_buffer_.get(), read_buffer_size_);
687 // Have to do this after the Read call, to ensure it still has an outstanding
688 // reference.
689 read_buffer_ = nullptr;
690 read_buffer_size_ = 0;
677 691
678 if (result == net::ERR_IO_PENDING) 692 if (result == net::ERR_IO_PENDING)
679 return; 693 return;
680 694
681 if (!handle_result_async || result <= 0) { 695 if (!handle_result_async || result <= 0) {
682 OnReadCompleted(request_.get(), result); 696 OnReadCompleted(request_.get(), result);
683 } else { 697 } else {
684 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO 698 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO
685 // thread in case the URLRequest can provide data synchronously. 699 // thread in case the URLRequest can provide data synchronously.
686 base::ThreadTaskRunnerHandle::Get()->PostTask( 700 base::ThreadTaskRunnerHandle::Get()->PostTask(
687 FROM_HERE, 701 FROM_HERE,
688 base::Bind(&ResourceLoader::OnReadCompleted, 702 base::Bind(&ResourceLoader::OnReadCompleted,
689 weak_ptr_factory_.GetWeakPtr(), request_.get(), result)); 703 weak_ptr_factory_.GetWeakPtr(), request_.get(), result));
690 } 704 }
691 } 705 }
692 706
693 void ResourceLoader::ResumeReading() { 707 void ResourceLoader::ResumeReading() {
694 DCHECK(!is_deferred()); 708 DCHECK(!is_deferred());
695 709
696 if (!read_deferral_start_time_.is_null()) { 710 if (!read_deferral_start_time_.is_null()) {
697 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral", 711 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral",
698 base::TimeTicks::Now() - read_deferral_start_time_); 712 base::TimeTicks::Now() - read_deferral_start_time_);
699 read_deferral_start_time_ = base::TimeTicks(); 713 read_deferral_start_time_ = base::TimeTicks();
700 } 714 }
701 if (request_->status().is_success()) { 715 if (request_->status().is_success()) {
702 ReadMore(false /* handle_result_asynchronously */); 716 PrepareToReadMore(false /* handle_result_asynchronously */);
703 } else { 717 } else {
704 ResponseCompleted(); 718 ResponseCompleted();
705 } 719 }
706 } 720 }
707 721
708 void ResourceLoader::CompleteRead(int bytes_read) { 722 void ResourceLoader::CompleteRead(int bytes_read) {
709 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this, 723 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this,
710 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 724 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
711 725
712 DCHECK(bytes_read >= 0); 726 DCHECK(bytes_read >= 0);
(...skipping 89 matching lines...) Expand 10 before | Expand all | Expand 10 after
802 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status, 816 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status,
803 STATUS_MAX); 817 STATUS_MAX);
804 } 818 }
805 } else if (request_->response_info().unused_since_prefetch) { 819 } else if (request_->response_info().unused_since_prefetch) {
806 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time(); 820 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time();
807 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time); 821 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time);
808 } 822 }
809 } 823 }
810 824
811 } // namespace content 825 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698