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

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

Issue 2668603003: Make ResourceHandler::OnWillRead able to complete asynchronously. (Closed)
Patch Set: Response to comments Created 3 years, 9 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 494 matching lines...) Expand 10 before | Expand all | Expand 10 after
505 // won't result in synchronously calling into a ResourceHandler, if this 505 // won't result in synchronously calling into a ResourceHandler, if this
506 // was called from Resume(). 506 // was called from Resume().
507 StartRequestInternal(); 507 StartRequestInternal();
508 break; 508 break;
509 case DEFERRED_REDIRECT: 509 case DEFERRED_REDIRECT:
510 // URLRequest::Start completes asynchronously, so starting the request now 510 // URLRequest::Start completes asynchronously, so starting the request now
511 // won't result in synchronously calling into a ResourceHandler, if this 511 // won't result in synchronously calling into a ResourceHandler, if this
512 // was called from Resume(). 512 // was called from Resume().
513 FollowDeferredRedirectInternal(); 513 FollowDeferredRedirectInternal();
514 break; 514 break;
515 case DEFERRED_ON_WILL_READ:
516 // Always post a task, as synchronous resumes don't go through this
517 // method.
518 base::ThreadTaskRunnerHandle::Get()->PostTask(
519 FROM_HERE,
520 base::Bind(&ResourceLoader::ReadMore, weak_ptr_factory_.GetWeakPtr(),
521 false /* handle_result_asynchronously */));
522 break;
515 case DEFERRED_READ: 523 case DEFERRED_READ:
516 if (called_from_resource_controller) { 524 if (called_from_resource_controller) {
517 // TODO(mmenke): Call ReadMore instead? Strange that this is the only 525 // TODO(mmenke): Call PrepareToReadMore instead? Strange that this is
518 // 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?
519 base::ThreadTaskRunnerHandle::Get()->PostTask( 529 base::ThreadTaskRunnerHandle::Get()->PostTask(
520 FROM_HERE, base::Bind(&ResourceLoader::ResumeReading, 530 FROM_HERE, base::Bind(&ResourceLoader::ResumeReading,
521 weak_ptr_factory_.GetWeakPtr())); 531 weak_ptr_factory_.GetWeakPtr()));
522 } else { 532 } else {
523 // 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,
524 // 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
525 // avoid blocking the IO thread. 535 // avoid blocking the IO thread.
526 ReadMore(true /* handle_result_asynchronously */); 536 PrepareToReadMore(true /* handle_result_asynchronously */);
527 } 537 }
528 break; 538 break;
529 case DEFERRED_RESPONSE_COMPLETE: 539 case DEFERRED_RESPONSE_COMPLETE:
530 if (called_from_resource_controller) { 540 if (called_from_resource_controller) {
531 base::ThreadTaskRunnerHandle::Get()->PostTask( 541 base::ThreadTaskRunnerHandle::Get()->PostTask(
532 FROM_HERE, base::Bind(&ResourceLoader::ResponseCompleted, 542 FROM_HERE, base::Bind(&ResourceLoader::ResponseCompleted,
533 weak_ptr_factory_.GetWeakPtr())); 543 weak_ptr_factory_.GetWeakPtr()));
534 } else { 544 } else {
535 ResponseCompleted(); 545 ResponseCompleted();
536 } 546 }
(...skipping 94 matching lines...) Expand 10 before | Expand all | Expand 10 after
631 641
632 void ResourceLoader::CompleteResponseStarted() { 642 void ResourceLoader::CompleteResponseStarted() {
633 ResourceRequestInfoImpl* info = GetRequestInfo(); 643 ResourceRequestInfoImpl* info = GetRequestInfo();
634 scoped_refptr<ResourceResponse> response = new ResourceResponse(); 644 scoped_refptr<ResourceResponse> response = new ResourceResponse();
635 PopulateResourceResponse(info, request_.get(), response.get()); 645 PopulateResourceResponse(info, request_.get(), response.get());
636 646
637 delegate_->DidReceiveResponse(this); 647 delegate_->DidReceiveResponse(this);
638 648
639 read_deferral_start_time_ = base::TimeTicks::Now(); 649 read_deferral_start_time_ = base::TimeTicks::Now();
640 // Using a ScopedDeferral here would result in calling ReadMore(true) on sync 650 // Using a ScopedDeferral here would result in calling ReadMore(true) on sync
641 // success. Calling ReadMore(false) here instead allows small responses to be 651 // success. Calling PrepareToReadMore(false) here instead allows small
642 // handled completely synchronously, if no ResourceHandler defers handling of 652 // responses to be handled completely synchronously, if no ResourceHandler
643 // the response. 653 // defers handling of the response.
644 deferred_stage_ = DEFERRED_SYNC; 654 deferred_stage_ = DEFERRED_SYNC;
645 handler_->OnResponseStarted(response.get(), 655 handler_->OnResponseStarted(response.get(),
646 base::MakeUnique<Controller>(this)); 656 base::MakeUnique<Controller>(this));
647 if (is_deferred()) { 657 if (is_deferred()) {
648 deferred_stage_ = DEFERRED_READ; 658 deferred_stage_ = DEFERRED_READ;
649 } else { 659 } else {
650 ReadMore(false); 660 PrepareToReadMore(false);
661 }
662 }
663
664 void ResourceLoader::PrepareToReadMore(bool handle_result_async) {
665 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::PrepareToReadMore", this,
666 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
667 DCHECK(!is_deferred());
668
669 deferred_stage_ = DEFERRED_SYNC;
670
671 handler_->OnWillRead(&read_buffer_, &read_buffer_size_,
672 base::MakeUnique<Controller>(this));
673
674 if (is_deferred()) {
675 deferred_stage_ = DEFERRED_ON_WILL_READ;
676 } else {
677 ReadMore(handle_result_async);
651 } 678 }
652 } 679 }
653 680
654 void ResourceLoader::ReadMore(bool handle_result_async) { 681 void ResourceLoader::ReadMore(bool handle_result_async) {
655 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", this, 682 DCHECK(read_buffer_.get());
656 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 683 DCHECK_GT(read_buffer_size_, 0);
657 DCHECK(!is_deferred());
658 684
659 // Make sure we track the buffer in at least one place. This ensures it gets 685 int result = request_->Read(read_buffer_.get(), read_buffer_size_);
660 // deleted even in the case the request has already finished its job and 686 // Have to do this after the Read call, to ensure it still has an outstanding
661 // doesn't use the buffer. 687 // reference.
662 scoped_refptr<net::IOBuffer> buf; 688 read_buffer_ = nullptr;
663 int buf_size; 689 read_buffer_size_ = 0;
664 if (!handler_->OnWillRead(&buf, &buf_size)) {
665 // Cancel the request, which will then call back into |this| to inform it of
666 // a "read error".
667 Cancel();
668 return;
669 }
670
671 DCHECK(buf.get());
672 DCHECK(buf_size > 0);
673
674 int result = request_->Read(buf.get(), buf_size);
675 690
676 if (result == net::ERR_IO_PENDING) 691 if (result == net::ERR_IO_PENDING)
677 return; 692 return;
678 693
679 if (!handle_result_async || result <= 0) { 694 if (!handle_result_async || result <= 0) {
680 OnReadCompleted(request_.get(), result); 695 OnReadCompleted(request_.get(), result);
681 } else { 696 } else {
682 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO 697 // Else, trigger OnReadCompleted asynchronously to avoid starving the IO
683 // thread in case the URLRequest can provide data synchronously. 698 // thread in case the URLRequest can provide data synchronously.
684 base::ThreadTaskRunnerHandle::Get()->PostTask( 699 base::ThreadTaskRunnerHandle::Get()->PostTask(
685 FROM_HERE, 700 FROM_HERE,
686 base::Bind(&ResourceLoader::OnReadCompleted, 701 base::Bind(&ResourceLoader::OnReadCompleted,
687 weak_ptr_factory_.GetWeakPtr(), request_.get(), result)); 702 weak_ptr_factory_.GetWeakPtr(), request_.get(), result));
688 } 703 }
689 } 704 }
690 705
691 void ResourceLoader::ResumeReading() { 706 void ResourceLoader::ResumeReading() {
692 DCHECK(!is_deferred()); 707 DCHECK(!is_deferred());
693 708
694 if (!read_deferral_start_time_.is_null()) { 709 if (!read_deferral_start_time_.is_null()) {
695 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral", 710 UMA_HISTOGRAM_TIMES("Net.ResourceLoader.ReadDeferral",
696 base::TimeTicks::Now() - read_deferral_start_time_); 711 base::TimeTicks::Now() - read_deferral_start_time_);
697 read_deferral_start_time_ = base::TimeTicks(); 712 read_deferral_start_time_ = base::TimeTicks();
698 } 713 }
699 if (request_->status().is_success()) { 714 if (request_->status().is_success()) {
700 ReadMore(false /* handle_result_asynchronously */); 715 PrepareToReadMore(false /* handle_result_asynchronously */);
701 } else { 716 } else {
702 ResponseCompleted(); 717 ResponseCompleted();
703 } 718 }
704 } 719 }
705 720
706 void ResourceLoader::CompleteRead(int bytes_read) { 721 void ResourceLoader::CompleteRead(int bytes_read) {
707 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this, 722 TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this,
708 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); 723 TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
709 724
710 DCHECK(bytes_read >= 0); 725 DCHECK(bytes_read >= 0);
(...skipping 81 matching lines...) Expand 10 before | Expand all | Expand 10 after
792 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status, 807 UMA_HISTOGRAM_ENUMERATION("Net.Prefetch.Pattern", prefetch_status,
793 STATUS_MAX); 808 STATUS_MAX);
794 } 809 }
795 } else if (request_->response_info().unused_since_prefetch) { 810 } else if (request_->response_info().unused_since_prefetch) {
796 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time(); 811 TimeDelta total_time = base::TimeTicks::Now() - request_->creation_time();
797 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time); 812 UMA_HISTOGRAM_TIMES("Net.Prefetch.TimeSpentOnPrefetchHit", total_time);
798 } 813 }
799 } 814 }
800 815
801 } // namespace content 816 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/loader/resource_loader.h ('k') | content/browser/loader/resource_loader_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698