PlzNavigate: Add first version of NavigationURLLoader.

content/browser/loader/navigation_resource_handler.cc

Index: content/browser/loader/navigation_resource_handler.cc
diff --git a/content/browser/loader/navigation_resource_handler.cc b/content/browser/loader/navigation_resource_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e8feca31fa5b6d90834a3e9a5eefb592d7238487
--- /dev/null
+++ b/content/browser/loader/navigation_resource_handler.cc
@@ -0,0 +1,150 @@
+// Copyright 2014 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/navigation_resource_handler.h"
+
+#include "base/logging.h"
+#include "content/browser/devtools/devtools_netlog_observer.h"
+#include "content/browser/loader/navigation_url_loader_core.h"
+#include "content/browser/loader/resource_request_info_impl.h"
+#include "content/browser/resource_context_impl.h"
+#include "content/browser/streams/stream.h"
+#include "content/browser/streams/stream_context.h"
+#include "content/public/browser/resource_controller.h"
+#include "content/public/browser/stream_handle.h"
+#include "content/public/common/resource_response.h"
+#include "net/base/net_errors.h"
+#include "net/url_request/url_request.h"
+
+namespace content {
+
+NavigationResourceHandler::NavigationResourceHandler(
+    net::URLRequest* request,
+    NavigationURLLoaderCore* core)
+    : ResourceHandler(request),
+      core_(core) {
+  core_->set_resource_handler(this);
+}
+
+NavigationResourceHandler::~NavigationResourceHandler() {
+  if (core_) {
+    core_->NotifyRequestFailed(net::ERR_ABORTED);

[mmenke, 2014/10/22 15:32:58]

Can we end up here without an OnResponseCompleted first, except perhaps during
RDH teardown, at which point, core will have been deleted first, anyways?

This is one case in which the extra check may be worth keeping, but wondering if
it's actually needed.

[davidben, 2014/10/22 20:58:03]

If the ResourceLoader is uncleanly torn down, it can happen. I'm not sure if
NavigationURLLoader will always be gone by then. There's
CancelRequestsForContext, but presumably if the ResourceContext is going away,
anyone holder a Loader has already dropped it. There's CancelRequestsForRoute,
but these aren't owned by a route. There's also CancelRequest, though I dunno if
that's called outside of downloads.

I think it's better to be safe here until we finish sorting out
non-renderer-owned ResourceLoader lifetimes.

+    core_->set_resource_handler(nullptr);
+    core_ = nullptr;

[mmenke, 2014/10/22 15:32:58]

I don't think this gets us anything.

[davidben, 2014/10/22 20:58:03]

Moved into helper method.

+  }
+}
+
+void NavigationResourceHandler::Cancel() {
+  controller()->Cancel();
+  core_ = nullptr;
+}
+
+void NavigationResourceHandler::FollowRedirect() {
+  controller()->Resume();
+}
+
+void NavigationResourceHandler::SetController(ResourceController* controller) {
+  writer_.set_controller(controller);
+  ResourceHandler::SetController(controller);
+}
+
+bool NavigationResourceHandler::OnUploadProgress(uint64 position,
+                                                 uint64 size) {
+  return true;
+}
+
+bool NavigationResourceHandler::OnRequestRedirected(
+    const net::RedirectInfo& redirect_info,
+    ResourceResponse* response,
+    bool* defer) {
+  if (!core_)

[mmenke, 2014/10/22 15:32:58]

Can this happen?  We would have cancelled it by this point, which I think would
prevent us from getting the redirect message.  Same goes for all the other core
checks, except the one on destruction.

If it can't happen, suggest DCHECKs instead.

[davidben, 2014/10/22 20:58:04]

I'm never clear on what can happen with all our asynchronous cancels. But it
does call CancelWithError, so I think I agree. Anything happening after cancel
is a bug. (There's probably one of those Cancel/Resume races we still haven't
sorted out...)

+    return false;
+
+  // TODO(davidben): Perform a CSP check here, and anything else that would have
+  // been done renderer-side.
+  DevToolsNetLogObserver::PopulateResponseInfo(request(), response);
+  core_->NotifyRequestRedirected(redirect_info, response);
+  *defer = true;
+  return true;
+}
+
+bool NavigationResourceHandler::OnResponseStarted(ResourceResponse* response,
+                                                  bool* defer) {
+  if (!core_)
+    return false;
+
+  ResourceRequestInfoImpl* info = GetRequestInfo();
+
+  // If the BufferedResourceHandler intercepted this request and converted it
+  // into a download, it will still call OnResponseStarted and immediately
+  // cancel. Ignore the call; OnReadCompleted will happen shortly.
+  //
+  // TODO(davidben): Move the dispatch out of BufferedResourceHandler. Perhaps
+  // all the way to the UI thread. Downloads, user certificates, etc., should be
+  // dispatched at the navigation layer.
+  if (info->IsDownload() || info->is_stream())
+    return true;
+
+  StreamContext* stream_context =
+      GetStreamContextForResourceContext(info->GetContext());
+  writer_.InitializeStream(stream_context->registry(),
+                           request()->url().GetOrigin());
+
+  // Detach from the loader; at this point, the request is now owned by the
+  // StreamHandle.
+  DevToolsNetLogObserver::PopulateResponseInfo(request(), response);
+  core_->NotifyResponseStarted(response, writer_.stream()->CreateHandle());
+  core_->set_resource_handler(nullptr);
+  core_ = nullptr;
+  return true;
+}
+
+bool NavigationResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+  return true;
+}
+
+bool NavigationResourceHandler::OnBeforeNetworkStart(const GURL& url,
+                                                     bool* defer) {
+  return true;
+}
+
+bool NavigationResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
+                                           int* buf_size,
+                                           int min_size) {
+  writer_.OnWillRead(buf, buf_size, min_size);
+  return true;
+}
+
+bool NavigationResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+  writer_.OnReadCompleted(bytes_read, defer);
+  return true;
+}
+
+void NavigationResourceHandler::OnResponseCompleted(
+    const net::URLRequestStatus& status,
+    const std::string& security_info,
+    bool* defer) {
+  // If the request has already committed, close the stream and leave it as-is.
+  //
+  // TODO(davidben): Is the final status of a navigation request meaningful?
+  // Perhaps we should eventually extend the streams interface to push the
+  // status down. (If there was an error partway through loading the top-level.)

[mmenke, 2014/10/22 15:32:58]

Do we currently replace committed partially loaded pages with network error
pages?

[davidben, 2014/10/22 20:58:04]

No, but it apparently isn't a no-op as far as Blink is concerned either.
https://crbug.com/422246. I've filed https://crbug.com/426162 to resolve it
separately and left it as a TODO for now.

+  if (writer_.stream()) {
+    writer_.Finalize();

[mmenke, 2014/10/22 15:32:58]

Telling WebKit a blob is happily completed on error doesn't break anything,
right?  For a reload, it will re-request the resource from the browser process,
and get a new blob, rather than using the cached old one (I assume it doesn't
even cache blobs, actually).

[davidben, 2014/10/22 20:58:04]

Left a TODO and filed https://crbug.com/426162

+    return;
+  }
+
+  if (core_) {
+    DCHECK_NE(net::OK, status.error());
+    core_->NotifyRequestFailed(status.error());
+    core_->set_resource_handler(nullptr);
+    core_ = nullptr;

[mmenke, 2014/10/22 15:32:58]

Are the last two lines needed?  We end up destroying |this| anyways, don't we,
which will end up doing the same thing.

If they are needed, this block is duplicated in the destructor.  Suggest adding
a method with the
shared code.

[davidben, 2014/10/22 20:58:04]

If we don't detach at this point, then the destructor will NotifyRequestFailed a
second time.
Done.

+  }
+}
+
+void NavigationResourceHandler::OnDataDownloaded(int bytes_downloaded) {
+  NOTREACHED();
+}
+
+}  // namespace content