Implement redirect handling on MojoAsyncResourceHandler

content/browser/loader/mojo_async_resource_handler.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/loader/mojo_async_resource_handler.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
@@ skipping 109 matching lines @@
 
 MojoAsyncResourceHandler::~MojoAsyncResourceHandler() {
   if (has_checked_for_sufficient_resources_)
     rdh_->FinishedWithResourcesForRequest(request());
 }
 
 bool MojoAsyncResourceHandler::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
     ResourceResponse* response,
     bool* defer) {
-  // Not implemented.
-  return false;
+  // Unlike OnResponseReceived, OnRequestRedirected will NOT be preceded by
+  // OnWillRead.
+  DCHECK(!shared_writer_);
+
+  *defer = true;
+  OnDefer();
+
+  NetLogObserver::PopulateResponseInfo(request(), response);
+  response->head.encoded_data_length = request()->GetTotalReceivedBytes();
+  response->head.request_start = request()->creation_time();
+  response->head.response_start = base::TimeTicks::Now();
+  // TODO(davidben): Is it necessary to pass the new first party URL for
+  // cookies? The only case where it can change is top-level navigation requests
+  // and hopefully those will eventually all be owned by the browser. It's
+  // possible this is still needed while renderer-owned ones exist.
+  url_loader_client_->OnReceiveRedirect(redirect_info, response->head);
+  return true;
 }
 
 bool MojoAsyncResourceHandler::OnResponseStarted(ResourceResponse* response,
                                                  bool* defer) {
   const ResourceRequestInfoImpl* info = GetRequestInfo();
 
   if (rdh_->delegate()) {
     rdh_->delegate()->OnResponseStarted(request(), info->GetContext(),
                                         response);
   }
@@ skipping 96 matching lines @@
   int64_t total_received_bytes = request()->GetTotalReceivedBytes();
   int64_t bytes_to_report =
       total_received_bytes - reported_total_received_bytes_;
   reported_total_received_bytes_ = total_received_bytes;
   DCHECK_LE(0, bytes_to_report);
 
   url_loader_client_->OnDataDownloaded(bytes_downloaded, bytes_to_report);
 }
 
 void MojoAsyncResourceHandler::FollowRedirect() {
-  NOTIMPLEMENTED();
+  if (!request()->status().is_success()) {
+    DVLOG(1) << "OnFollowRedirect for invalid request";
+    return;
+  }
+
+  DCHECK(did_defer_);

[dcheng, 2016/11/10 07:09:27]

Is it safe to allow this to be called multiple times? This is called from the
renderer, and an exploited renderer could just invoke FollowRedirect() multiple
times.

(Basically, I'm wondering if this should be a DCHECK or if it's something we
need to check for and handle)

[mmenke, 2016/11/10 21:42:40]

Good question...It looks like it's actually safe (We tear down the redirect and
try again), but that's just luck.  We should probably just kill the renderer if
this happens.

[yhirano, 2016/11/11 09:47:03]

Done. It's now cancelling the request, I don't know what is the preferred manner
for the malformed request with mojo.

+  did_defer_ = false;
+  request()->LogUnblocked();
+  controller()->Resume();
 }
 
-void MojoAsyncResourceHandler::ResumeForTesting() {
-  Resume();
+void MojoAsyncResourceHandler::OnWritableForTesting() {
+  OnWritable(MOJO_RESULT_OK);
 }
 
 void MojoAsyncResourceHandler::SetAllocationSizeForTesting(size_t size) {
   g_allocation_size = size;
 }
 
 MojoResult MojoAsyncResourceHandler::BeginWrite(void** data,
                                                 uint32_t* available) {
   MojoResult result = mojo::BeginWriteDataRaw(
       shared_writer_->writer(), data, available, MOJO_WRITE_DATA_FLAG_NONE);
@@ skipping 81 matching lines @@
   if (result == MOJO_RESULT_SHOULD_WAIT) {
     *defer = true;
     return true;
   }
   if (result != MOJO_RESULT_OK)
     return false;
   *buf = new WriterIOBuffer(shared_writer_, data, available);
   return true;
 }
 
-void MojoAsyncResourceHandler::Resume() {
+void MojoAsyncResourceHandler::OnDefer() {
+  request()->LogBlockedBy("MojoAsyncResourceHandler");
+  did_defer_ = true;
+}
+
+bool MojoAsyncResourceHandler::CheckForSufficientResource() {
+  if (has_checked_for_sufficient_resources_)
+    return true;
+  has_checked_for_sufficient_resources_ = true;
+
+  if (rdh_->HasSufficientResourcesForRequest(request()))
+    return true;
+
+  controller()->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
+  return false;
+}
+
+void MojoAsyncResourceHandler::OnWritable(MojoResult unused) {

[mmenke, 2016/11/10 21:42:40]

nit:  I think the style is to not name unused variables?  Also should echo the
name in the header.  "MojoResult /*result*/" or "/*=result*/", or whatnot.

[yhirano, 2016/11/11 09:47:03]

Done.

   if (!did_defer_)
     return;
   did_defer_ = false;
 
   if (is_using_io_buffer_not_from_writer_) {
     // |buffer_| is set to a net::IOBufferWithSize. Write the buffer contents
     // to the data pipe.
     DCHECK_GT(buffer_bytes_read_, 0u);
     if (!CopyReadDataToDataPipe(&did_defer_)) {
       controller()->CancelWithError(net::ERR_FAILED);
       return;
     }
   } else {
     // Allocate a buffer for the next OnWillRead call here.
     if (!AllocateWriterIOBuffer(&buffer_, &did_defer_)) {
       controller()->CancelWithError(net::ERR_FAILED);
       return;
     }
   }
 
   if (did_defer_) {
     // Continue waiting.
     return;
   }
   request()->LogUnblocked();
   controller()->Resume();
 }
 
-void MojoAsyncResourceHandler::OnDefer() {
-  request()->LogBlockedBy("MojoAsyncResourceHandler");
-  did_defer_ = true;
-}
-
-bool MojoAsyncResourceHandler::CheckForSufficientResource() {
-  if (has_checked_for_sufficient_resources_)
-    return true;
-  has_checked_for_sufficient_resources_ = true;
-
-  if (rdh_->HasSufficientResourcesForRequest(request()))
-    return true;
-
-  controller()->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
-  return false;
-}
-
-void MojoAsyncResourceHandler::OnWritable(MojoResult unused) {
-  Resume();
-}
-
 void MojoAsyncResourceHandler::Cancel() {
   const ResourceRequestInfoImpl* info = GetRequestInfo();
   ResourceDispatcherHostImpl::Get()->CancelRequestFromRenderer(
       GlobalRequestID(info->GetChildID(), info->GetRequestID()));
 }
 
 }  // namespace content