Refactor ResourceHandler API.

content/browser/loader/resource_loader.cc

Index: content/browser/loader/resource_loader.cc
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index f2f670a566d55ed03d811df7269c241b66a3e1bf..452d97fbf03a1f648ab333b01c72e4b73438f71f 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -163,16 +163,13 @@ void ResourceLoader::StartRequest() {
     return;
   }
 
-  // Give the handler a chance to delay the URLRequest from being started.
-  bool defer_start = false;
-  if (!handler_->OnWillStart(request_->url(), &defer_start)) {
-    Cancel();
-    return;
-  }
-
   TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::StartRequest", this,
                          TRACE_EVENT_FLAG_FLOW_OUT);
-  if (defer_start) {
+
+  // Give the handler a chance to delay the URLRequest from being started.
+  bool defer_or_cancel = false;
+  handler_->OnWillStart(request_->url(), &defer_or_cancel);
+  if (defer_or_cancel) {
     deferred_stage_ = DEFERRED_START;
   } else {
     StartRequestInternal();
@@ -182,19 +179,7 @@ void ResourceLoader::StartRequest() {
 void ResourceLoader::CancelRequest(bool from_renderer) {
   TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CancelRequest", this,
                          TRACE_EVENT_FLAG_FLOW_IN);
-  CancelRequestInternal(net::ERR_ABORTED, from_renderer);
-}
-
-void ResourceLoader::CancelAndIgnore() {
-  ResourceRequestInfoImpl* info = GetRequestInfo();
-  info->set_was_ignored_by_handler(true);
-  CancelRequest(false);
-}
-
-void ResourceLoader::CancelWithError(int error_code) {
-  TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CancelWithError", this,
-                         TRACE_EVENT_FLAG_FLOW_IN);
-  CancelRequestInternal(error_code, false);
+  CancelRequestInternal(net::ERR_ABORTED, from_renderer, false);
 }
 
 void ResourceLoader::MarkAsTransferring(
@@ -283,11 +268,11 @@ void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused,
   scoped_refptr<ResourceResponse> response = new ResourceResponse();
   PopulateResourceResponse(info, request_.get(), response.get());
   delegate_->DidReceiveRedirect(this, redirect_info.new_url, response.get());
-  if (!handler_->OnRequestRedirected(redirect_info, response.get(), defer)) {
-    Cancel();
-  } else if (*defer) {
+
+  // Cancellations are treated much like defers, so ignore the difference.
+  handler_->OnRequestRedirected(redirect_info, response.get(), defer);
+  if (*defer)
     deferred_stage_ = DEFERRED_REDIRECT;  // Follow redirect when resumed.
-  }
 }
 
 void ResourceLoader::OnAuthRequired(net::URLRequest* unused,
@@ -476,7 +461,17 @@ void ResourceLoader::Resume() {
 }
 
 void ResourceLoader::Cancel() {
-  CancelRequest(false);
+  CancelWithError(net::ERR_ABORTED);
+}
+
+void ResourceLoader::CancelAndIgnore() {
+  CancelRequestInternal(net::ERR_ABORTED, false, true);
+}
+
+void ResourceLoader::CancelWithError(int error_code) {
+  TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CancelWithError", this,
+                         TRACE_EVENT_FLAG_FLOW_IN);
+  CancelRequestInternal(error_code, false, false);
 }
 
 void ResourceLoader::StartRequestInternal() {
@@ -492,10 +487,14 @@ void ResourceLoader::StartRequestInternal() {
   delegate_->DidStartRequest(this);
 }
 
-void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {
+void ResourceLoader::CancelRequestInternal(int error,
+                                           bool from_renderer,
+                                           bool ignore) {
   DVLOG(1) << "CancelRequestInternal: " << request_->url().spec();
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
+  if (ignore)
+    info->set_was_ignored_by_handler(true);
 
   // WebKit will send us a cancel for downloads since it no longer handles
   // them.  In this case, ignore the cancel since we handle downloads in the
@@ -549,18 +548,42 @@ void ResourceLoader::CompleteResponseStarted() {
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnResponseStarted()"));
 
-  bool defer = false;
-  if (!handler_->OnResponseStarted(response.get(), &defer)) {
-    Cancel();
-  } else if (defer) {
+  bool defer_or_cancel = false;
+  handler_->OnResponseStarted(response.get(), &defer_or_cancel);
+  if (defer_or_cancel) {
     read_deferral_start_time_ = base::TimeTicks::Now();
     deferred_stage_ = DEFERRED_READ;  // Read first chunk when resumed.
   }
 }
 
 void ResourceLoader::StartReading(bool is_continuation) {
+  TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::StartReading", this,
+                         TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
+  DCHECK(!is_deferred());
   int bytes_read = 0;
-  ReadMore(&bytes_read);
+
+  // 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;
+  {
+    // 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, -1)) {
+      Cancel();
+      return;
+    }
+  }
+
+  DCHECK(buf.get());
+  DCHECK(buf_size > 0);
+
+  // No need to check the return value here.  Instead, detect errors below by
+  // inspecting the URLRequest's status.
+  request_->Read(buf.get(), buf_size, &bytes_read);
 
   // If IO is pending, wait for the URLRequest to call OnReadCompleted.
   if (request_->status().is_io_pending())
@@ -593,36 +616,6 @@ void ResourceLoader::ResumeReading() {
   }
 }
 
-void ResourceLoader::ReadMore(int* bytes_read) {
-  TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::ReadMore", 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;
-  {
-    // 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, -1)) {
-      Cancel();
-      return;
-    }
-  }
-
-  DCHECK(buf.get());
-  DCHECK(buf_size > 0);
-
-  request_->Read(buf.get(), buf_size, bytes_read);
-
-  // No need to check the return value here as we'll detect errors by
-  // inspecting the URLRequest's status.
-}
-
 void ResourceLoader::CompleteRead(int bytes_read) {
   TRACE_EVENT_WITH_FLOW0("loading", "ResourceLoader::CompleteRead", this,
                          TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
@@ -634,10 +627,9 @@ void ResourceLoader::CompleteRead(int bytes_read) {
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION("475761 OnReadCompleted()"));
 
-  bool defer = false;
-  if (!handler_->OnReadCompleted(bytes_read, &defer)) {
-    Cancel();
-  } else if (defer) {
+  bool defer_or_cancel = false;
+  handler_->OnReadCompleted(bytes_read, &defer_or_cancel);
+  if (defer_or_cancel) {
     deferred_stage_ =
         bytes_read > 0 ? DEFERRED_READ : DEFERRED_RESPONSE_COMPLETE;
   }
@@ -664,11 +656,11 @@ void ResourceLoader::ResponseCompleted() {
     handler_->OnResponseCompleted(request_->status(), &defer);
   }
   if (defer) {
-    // The handler is not ready to die yet.  We will call DidFinishLoading when
-    // we resume.
+    // The handler is not ready to die yet.  DidFinishLoading will be called on
+    // resume.
     deferred_stage_ = DEFERRED_FINISH;
   } else {
-    // This will result in our destruction.
+    // This will result in destroying |this|.
     CallDidFinishLoading();
   }
 }