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

Unified 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, 11 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 side-by-side diff with in-line comments
Download patch
Index: content/browser/loader/resource_loader.cc
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index da6649b8f7830e1603d0b3e44836daa4afe0bc54..dd3db560a0ddcac06cb7afaef057a0a6e7e51861 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -513,10 +513,19 @@ void ResourceLoader::Resume(bool called_from_resource_controller) {
// was called from Resume().
FollowDeferredRedirectInternal();
break;
+ case DEFERRED_ON_WILL_READ:
+ // Always post a task, as synchronous resumes don't go through this
+ // method.
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(&ResourceLoader::ReadMore,
+ weak_ptr_factory_.GetWeakPtr(), false));
+ break;
case DEFERRED_READ:
if (called_from_resource_controller) {
- // TODO(mmenke): Call ReadMore instead? Strange that this is the only
- // path which calls different methods, depending on the path.
+ // TODO(mmenke): Call PrepareToReadMore instead? Strange that this is
+ // the only case which calls different methods, depending on the path.
+ // ResumeReading does check for cancellation. Should other paths do that
+ // as well?
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(&ResourceLoader::ResumeReading,
weak_ptr_factory_.GetWeakPtr()));
@@ -524,7 +533,7 @@ void ResourceLoader::Resume(bool called_from_resource_controller) {
// If this was called as a result of a handler succeeding synchronously,
// force the result of the next read to be handled asynchronously, to
// avoid blocking the IO thread.
- ReadMore(true /* handle_result_asynchronously */);
+ PrepareToReadMore(true /* handle_result_asynchronously */);
}
break;
case DEFERRED_RESPONSE_COMPLETE:
@@ -647,33 +656,38 @@ void ResourceLoader::CompleteResponseStarted() {
base::MakeUnique<Controller>(this));
}
-void ResourceLoader::ReadMore(bool handle_result_async) {
- TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", this,
+void ResourceLoader::PrepareToReadMore(bool handle_result_async) {
+ TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::PrepareToReadMore", this,
TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
DCHECK(!is_deferred());
- // Make sure we track the buffer in at least one place. This ensures it gets
- // deleted even in the case the request has already finished its job and
- // doesn't use the buffer.
- scoped_refptr<net::IOBuffer> buf;
- int buf_size;
+ deferred_stage_ = DEFERRED_SYNC;
+
{
// TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
tracked_objects::ScopedTracker tracking_profile2(
FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnWillRead()"));
- if (!handler_->OnWillRead(&buf, &buf_size)) {
- // Cancel the request, which will then call back into |this| to inform it
- // of a "read error".
- Cancel();
- return;
- }
+ handler_->OnWillRead(&read_buffer_, &read_buffer_size_,
+ base::MakeUnique<Controller>(this));
}
- DCHECK(buf.get());
- DCHECK(buf_size > 0);
+ if (is_deferred()) {
+ deferred_stage_ = DEFERRED_ON_WILL_READ;
+ } else {
+ ReadMore(handle_result_async);
+ }
+}
+
+void ResourceLoader::ReadMore(bool handle_result_async) {
+ DCHECK(read_buffer_.get());
+ DCHECK_GT(read_buffer_size_, 0);
- int result = request_->Read(buf.get(), buf_size);
+ int result = request_->Read(read_buffer_.get(), read_buffer_size_);
+ // Have to do this after the Read call, to ensure it still has an outstanding
+ // reference.
+ read_buffer_ = nullptr;
+ read_buffer_size_ = 0;
if (result == net::ERR_IO_PENDING)
return;
@@ -699,7 +713,7 @@ void ResourceLoader::ResumeReading() {
read_deferral_start_time_ = base::TimeTicks();
}
if (request_->status().is_success()) {
- ReadMore(false /* handle_result_asynchronously */);
+ PrepareToReadMore(false /* handle_result_asynchronously */);
} else {
ResponseCompleted();
}

Powered by Google App Engine
This is Rietveld 408576698