[2/3 chromium] Support redirect option of Request and "opaqueredirect" response type.

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 0339cf11c0d5ade2f2f4ebe14a64cd3da4289c3b..f92dfa52540fa9eb91289f404d86a83bc9efadec 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -280,6 +280,28 @@ void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused,
     return;
   }
 
+  if (info->fetch_redirect_mode() == FETCH_REDIRECT_MODE_ERROR) {

[mmenke, 2015/08/10 15:52:02]

We're going to need unit tests for both these paths.  I'd suggest RDH unittests,
since those are more integrationy, and cover how the ResourceLoader, RDH, and
AsyncResourceHandler all interact.

[horo, 2015/08/11 05:58:20]

Added ResourceDispatcherHostTest.FetchRedirectModeError and
ResourceDispatcherHostTest.FetchRedirectModeManual in
resource_dispatcher_host_unittest.cc

+    Cancel();
+    return;
+  } else if (info->fetch_redirect_mode() == FETCH_REDIRECT_MODE_MANUAL) {
+    // TODO(horo): If we will support upload progress events for Fetch API, we
+    // have to call ReportUploadProgress().

[mmenke, 2015/08/10 15:52:02]

This should actually probably be done unconditionally in this method - we don't
report multiple upload attempts as requests are redirected, so this method
should probably have the same behavior as OnResponseStarted.

[mmenke, 2015/08/10 15:52:02]

nit:  Move "will" should before "have".

[horo, 2015/08/11 05:58:20]

Done.

[horo, 2015/08/11 05:58:20]

FETCH_REDIRECT_MODE_MANUAL is supported only by Fetch API.
Fetch API doesn't set is_upload_progress_enabled flag.
So We don't need to call ReportUploadProgress().

Added "DCHECK(!GetRequestInfo()->is_upload_progress_enabled());"

+    CompleteResponseStarted();

[mmenke, 2015/08/10 15:52:02]

Suggest a blank line after CompleteResponseStarted().

[horo, 2015/08/11 05:58:20]

Done.

+    // We don't need to read the response body because there is no way to read
+    // the body of opaque-redirect filtered response's internal response.

[mmenke, 2015/08/10 15:52:02]

nit:  Don't use "we" in comments.

[horo, 2015/08/11 05:58:20]

Done.

+    // TODO(horo): When we will support any API which expose the internal body,

[mmenke, 2015/08/10 15:52:02]

Is this "when" or "if"?

[mmenke, 2015/08/10 15:52:02]

nit:  Move "will" should before "have".

[horo, 2015/08/11 05:58:20]

Done.

[horo, 2015/08/11 05:58:20]

Done.

+    // we have to read the body before calling ResponseCompleted().

[mmenke, 2015/08/10 15:52:02]

Add warning that Cache changes will be needed before this is possible, or such
requests will have to bypass the cache.

[horo, 2015/08/11 05:58:20]

Done.

+    if (is_deferred()) {

[mmenke, 2015/08/10 15:52:02]

I'm not following how this is possible.

[horo, 2015/08/11 05:58:20]

Removed and added "DCHECK(!is_deferred())"

+      // If |deferred_stage_| is set while calling CompleteResponseStarted(),
+      // we reset |deferred_stage_| to call ResponseCompleted when resumed.

[mmenke, 2015/08/10 15:52:02]

nit:  Don't use "we" in comments.

[horo, 2015/08/11 05:58:20]

Removed.

+      DCHECK(DEFERRED_READ == deferred_stage_);

[mmenke, 2015/08/10 15:52:02]

I'm really not following how this is possible.  We only defer reads once we
actually want to read the response body.  We only want to do that after we've
received the final set of headers, after following all redirects.  At this
point, we still have a redirect to follow.

+      deferred_stage_ = DEFERRED_RESPONSE_COMPLETE;
+      return;
+    }
+    ResponseCompleted();
+    return;
+  }
+
   delegate_->DidReceiveRedirect(this, redirect_info.new_url);
 
   if (delegate_->HandleExternalProtocol(this, redirect_info.new_url)) {