Use mojo for Chrome Loading, Part 1

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"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/time/time.h"
+#include "content/browser/loader/netlog_observer.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/browser/loader/resource_message_filter.h"
+#include "content/browser/loader/resource_request_info_impl.h"
+#include "content/common/resource_request_completion_status.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/common/resource_response.h"
+#include "net/base/io_buffer.h"
+#include "net/base/load_flags.h"
+#include "net/log/net_log.h"
+#include "net/url_request/redirect_info.h"
+
+namespace content {
+namespace {
+
+int g_max_allocation_size = 1024 * 32;
+
+void GetNumericArg(const std::string& name, int* result) {
+  const std::string& value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(name);
+  if (!value.empty())
+    base::StringToInt(value, result);
+}
+
+void InitializeResourceBufferConstants() {
+  static bool did_init = false;
+  if (did_init)
+    return;
+  did_init = true;
+
+  GetNumericArg("resource-buffer-max-allocation-size", &g_max_allocation_size);
+}
+
+}  // namespace
+
+MojoAsyncResourceHandler::MojoAsyncResourceHandler(
+    net::URLRequest* request,
+    ResourceDispatcherHostImpl* rdh,
+    std::unique_ptr<mojom::URLLoader> url_loader,
+    mojom::URLLoaderClientPtr url_loader_client)
+    : ResourceHandler(request),
+      rdh_(rdh),
+      url_loader_(std::move(url_loader)),
+      url_loader_client_(std::move(url_loader_client)) {
+  DCHECK(url_loader_);
+  DCHECK(url_loader_client_);
+  InitializeResourceBufferConstants();
+}
+
+MojoAsyncResourceHandler::~MojoAsyncResourceHandler() {
+  if (has_checked_for_sufficient_resources_)
+    rdh_->FinishedWithResourcesForRequest(request());
+}
+
+void MojoAsyncResourceHandler::OnFollowRedirect(int request_id) {
+  NOTREACHED();
+}
+
+bool MojoAsyncResourceHandler::OnRequestRedirected(
+    const net::RedirectInfo& redirect_info,
+    ResourceResponse* response,
+    bool* defer) {
+  // Not implemented.
+  return false;
+}
+
+bool MojoAsyncResourceHandler::OnResponseStarted(ResourceResponse* response,
+                                                 bool* defer) {
+  const ResourceRequestInfoImpl* info = GetRequestInfo();
+  if (!info->filter())
+    return false;
+
+  if (rdh_->delegate()) {
+    rdh_->delegate()->OnResponseStarted(request(), info->GetContext(), response,
+                                        info->filter());
+  }
+
+  NetLogObserver::PopulateResponseInfo(request(), response);
+
+  // If the parent handler downloaded the resource to a file, grant the child
+  // read permissions on it.
+  if (!response->head.download_file_path.empty()) {
+    rdh_->RegisterDownloadedTempFile(info->GetChildID(), info->GetRequestID(),
+                                     response->head.download_file_path);
+  }

[mmenke, 2016/05/26 21:46:09]

Seems like we shouldn't include this code until OnDataDownloaded is hooked up -
half-hooking something up is worth than not hooking something up.  Can just
DCHECK it's empty, for now.

[yhirano, 2016/05/27 11:30:34]

Done.

+
+  response->head.request_start = request()->creation_time();
+  response->head.response_start = base::TimeTicks::Now();
+  sent_received_response_message_ = true;
+  url_loader_client_->OnReceiveResponse(response->head);
+  return true;
+}
+
+bool MojoAsyncResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+  return true;
+}
+
+bool MojoAsyncResourceHandler::OnBeforeNetworkStart(const GURL& url,
+                                                    bool* defer) {
+  return true;
+}
+
+bool MojoAsyncResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
+                                          int* buf_size,
+                                          int min_size) {
+  DCHECK_EQ(-1, min_size);
+
+  if (!CheckForSufficientResource())
+    return false;
+
+  void* buffer = nullptr;
+  uint32_t available = 0;
+  if (!writer_.is_valid()) {
+    MojoCreateDataPipeOptions options;
+    options.struct_size = sizeof(MojoCreateDataPipeOptions);
+    options.flags = MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_NONE;
+    options.element_num_bytes = 1;
+    options.capacity_num_bytes = g_max_allocation_size;
+    mojo::DataPipe data_pipe(options);
+
+    writer_ = std::move(data_pipe.producer_handle);
+    ResourceMessageFilter* filter = GetRequestInfo()->filter();
+    if (filter) {
+      url_loader_client_->OnStartLoadingResponseBody(
+          std::move(data_pipe.consumer_handle));
+    }
+  }
+  if (!writer_.is_valid()) {
+    controller()->CancelWithError(net::ERR_FAILED);
+    return false;

[mmenke, 2016/05/26 21:46:09]

Returning false results in a cancel, as does calling CancelWithError.  While
URLRequestJob does prevent Bad Things from double cancel, it looks like the only
code that does has a TODO about removing it, and ResourceLoader doesn't really
expect a double cancel message, so let's return return false, without calling
CancelWithError (That will, unfortunately, cancel the request with ERR_ABORTED
instead of ERR_FAILED, and ERR_ABORTED has magic behavior, but fixing that's
beyond the scope of this CL, and mimics the old class's behavior.

[yhirano, 2016/05/27 11:30:34]

Done.

Does that mean CancelWithError in checkForSufficientResource (copied from
AsyncResourceHandler) is bad, too?

[mmenke, 2016/05/27 15:39:26]

Yes.  It may make sense to return a network error code, instead of
true/false...but well beyond the scope of this CL.

+  }
+
+  MojoResult result = mojo::BeginWriteDataRaw(
+      writer_.get(), &buffer, &available, MOJO_WRITE_DATA_FLAG_NONE);
+  // SHOULD_WAIT should be handled in OnReadCompleted.

[mmenke, 2016/05/26 21:46:09]

I'm not following this comment.  Docs seem to indicate this can return
MOJO_RESULT_SHOULD_WAIT, and we should handle it here.

[yhirano, 2016/05/27 11:30:34]

Generally we don't treat MOJO_RESULT_SHOULD_WAIT as a fatal error, but we do
here, because that SHOULD_WAIT should have been handled in
OnReadCompleted(between L173-L181 in this CL) and OnDefer should have been
called (i.e., this function should not be called while we are waiting the reader
for reading).

[mmenke, 2016/05/27 15:39:26]

I'm not following.  The docs say nothing about when BeginWriteDataRaw returns
MOJO_RESULT_SHOULD_WAIT.  If this is the first mojo::BeginWriteDataRaw, we may
not have even called OnReadCompleted yet.

I also don't see any guarantee that if one call to BeginWriteDataRaw succeeds,
and then we call EndWriteDataRaw(..., 0), that the next call to
BeginWriteDataRaw is guaranteed not to return MOJO_RESULT_SHOULD_WAIT.

[yhirano, 2016/05/30 12:41:45]

I expected so:
 - After creating a data pipe, the first BeginWriteData will
   not return SHOULD_WAIT and |*buffer_num_bytes| will be set
   to the capacity if it returns OK.
 - When we call EndWriteData with |num_bytes_written| just after
   calling successful BeginWriteData (i.e., it returned OK), the
   next BeginWriteData will not return SHOULD_WAIT.
unless there is an error. Of course, if there is an error, we can
return with that error as the current code does.

But strictly speaking you're right, the comments say nothing about it.
yzshen1@: can you tell me if it's guaranteed or not?

[yzshen1, 2016/05/31 21:07:38]

Generally speaking, I think we should stick to what the API documentation says,
instead of the unspecified behavior of the implementation.
I think this is true. (But please see my comment above)
Not necessarily, it is possible that the other side hasn't consumed data to
release the buffer.

[yhirano, 2016/06/01 14:40:51]

Thank you. Let me address this issue in a separate CL because I need to update
ResourceHandler's design so that we can set *buf_size to 0.

+  if (result == MOJO_RESULT_OK) {
+    *buf = new net::WrappedIOBuffer(static_cast<const char*>(buffer));
+    *buf_size = available;
+    return true;
+  }
+  controller()->CancelWithError(net::ERR_FAILED);

[mmenke, 2016/05/26 21:46:09]

remove this.

[yhirano, 2016/05/27 11:30:34]

Done.

+  return false;

[mmenke, 2016/05/26 21:46:09]

Suggest the failure case first.

[yhirano, 2016/05/27 11:30:33]

Done.

+}
+
+bool MojoAsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+  DCHECK_GE(bytes_read, 0);
+
+  if (!bytes_read)
+    return true;
+
+  MojoResult result = mojo::EndWriteDataRaw(writer_.get(), bytes_read);
+  if (result != MOJO_RESULT_OK)
+    return false;
+  void* buffer = nullptr;
+  uint32_t available = 0;
+  // To see if the handle is still writable.
+  result = mojo::BeginWriteDataRaw(writer_.get(), &buffer, &available,
+                                   MOJO_WRITE_DATA_FLAG_NONE);
+  if (result == MOJO_RESULT_SHOULD_WAIT ||
+      (result == MOJO_RESULT_OK && available == 0)) {

[mmenke, 2016/05/26 21:46:09]

Can the second condition occur?  Should it be able to occur?  If it should be
treated just like MOJO_RESULT_SHOULD_WAIT, seems like a mojo bug.

[yhirano, 2016/05/27 11:30:34]

The comment was not clear to me, but the actual implementation doesn't return OK
with available = 0.

+    *defer = did_defer_ = true;
+    OnDefer();
+    handle_watcher_.Start(writer_.get(), MOJO_HANDLE_SIGNAL_WRITABLE,
+                          MOJO_DEADLINE_INDEFINITE,
+                          base::Bind(&MojoAsyncResourceHandler::OnWritable,
+                                     base::Unretained(this)));
+  }
+  if (result == MOJO_RESULT_OK)
+    mojo::EndWriteDataRaw(writer_.get(), 0);
+  return true;
+}
+
+void MojoAsyncResourceHandler::OnDataDownloaded(int bytes_downloaded) {
+  // Not implemented.
+}
+
+void MojoAsyncResourceHandler::OnResponseCompleted(
+    const net::URLRequestStatus& status,
+    const std::string& security_info,
+    bool* defer) {
+  const ResourceRequestInfoImpl* info = GetRequestInfo();
+  if (!info->filter())
+    return;
+
+  int error_code = status.error();
+  bool was_ignored_by_handler = info->WasIgnoredByHandler();
+
+  DCHECK(status.status() != net::URLRequestStatus::IO_PENDING);

[mmenke, 2016/05/26 21:46:09]

Please keep the (status.status() != net::URLRequestStatus::SUCCESS ||
sent_received_response_msg_) check - it's helped us uncover real bugs.  Fine
with it being switched to a DCHECK.

[yhirano, 2016/05/27 11:30:33]

Done.

+  // If this check fails, then we're in an inconsistent state because all
+  // requests ignored by the handler should be canceled (which should result in
+  // the ERR_ABORTED error code).
+  DCHECK(!was_ignored_by_handler || error_code == net::ERR_ABORTED);
+
+  // TODO(mkosiba): Fix up cases where we create a URLRequestStatus
+  // with a status() != SUCCESS and an error_code() == net::OK.
+  if (status.status() == net::URLRequestStatus::CANCELED &&
+      error_code == net::OK) {
+    error_code = net::ERR_ABORTED;
+  } else if (status.status() == net::URLRequestStatus::FAILED &&
+             error_code == net::OK) {
+    error_code = net::ERR_FAILED;
+  }
+
+  ResourceRequestCompletionStatus request_complete_data;
+  request_complete_data.error_code = error_code;
+  request_complete_data.was_ignored_by_handler = was_ignored_by_handler;
+  request_complete_data.exists_in_cache = request()->response_info().was_cached;
+  request_complete_data.security_info = security_info;
+  request_complete_data.completion_time = base::TimeTicks::Now();
+  request_complete_data.encoded_data_length =
+      request()->GetTotalReceivedBytes();
+
+  url_loader_client_->OnComplete(request_complete_data);
+}
+
+void MojoAsyncResourceHandler::ResumeIfDeferred() {
+  if (did_defer_) {
+    did_defer_ = false;
+    request()->LogUnblocked();
+    controller()->Resume();
+  }
+}
+
+void MojoAsyncResourceHandler::OnDefer() {
+  request()->LogBlockedBy("MojoAsyncResourceHandler");
+}
+
+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) {
+  ResumeIfDeferred();
+}
+
+}  // namespace content