Add a NavigationThrottle to the public content/ interface

content/browser/frame_host/navigation_request.cc

 // 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/frame_host/navigation_request.h"
 
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/resource_request_body.h"
 #include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/common/content_client.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_request_headers.h"
 #include "net/url_request/redirect_info.h"
 
 namespace content {
 
 namespace {
 
@@ skipping 15 matching lines @@
     case FrameMsg_Navigate_Type::RESTORE_WITH_POST:
       load_flags |= net::LOAD_ONLY_FROM_CACHE;
       break;
     case FrameMsg_Navigate_Type::NORMAL:
     default:
       break;
   }
   return load_flags;
 }
 
+// Filters the URL for the navigation based.
+GURL FilterURL(const GURL& url, FrameTreeNode* frame_tree_node) {
+  RenderFrameHostImpl* render_frame_host =
+      frame_tree_node->render_manager()->speculative_frame_host()
+          ? frame_tree_node->render_manager()->speculative_frame_host()
+          : frame_tree_node->render_manager()->current_frame_host();

[carlosk, 2015/08/28 16:40:24]

This next-RFH logic should live inside RFHM. We've been in the verge of adding
it there for some time already and this usage looks like the perfect use case.

[nasko, 2015/08/31 23:25:15]

This code seems a bit fragile. What happens if there is a speculative RFH and
the url passed in doesn't match the SiteInstance for that one? The current code
doesn't handle this case, does it?

[clamy, 2015/09/03 15:30:51]

This would happen in the case of redirects. In that case we may not even have a
renderer with the right site instance. I'm not sure what to do in that case:
fall back to the current RFH? Note that the ResourceThrottles try to filter the
url based on the RFH that initiated the navigation (with the redirect it may not
be appropriate).

[nasko, 2015/09/04 23:36:49]

I'm not sure we need to call FilterURL on redirects though. We don't do it today
and it is meant to ensure the renderer doesn't request something invalid. If the
server redirects us to some bogus place, so be it. Maybe add a TODO on what the
problem is and let's leave it to be resolved later on.

+  GURL filtered_url = url;
+  render_frame_host->GetProcess()->FilterURL(false, &filtered_url);
+  return filtered_url;
+}
+
 }  // namespace
 
 // static
 scoped_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
     FrameTreeNode* frame_tree_node,
     const GURL& dest_url,
     const Referrer& dest_referrer,
     const FrameNavigationEntry& frame_entry,
     const NavigationEntryImpl& entry,
     FrameMsg_Navigate_Type::Value navigation_type,
@@ skipping 110 matching lines @@
 
 NavigationRequest::~NavigationRequest() {
 }
 
 bool NavigationRequest::BeginNavigation() {
   DCHECK(!loader_);
   DCHECK(state_ == NOT_STARTED || state_ == WAITING_FOR_RENDERER_RESPONSE);
   state_ = STARTED;
 
   if (ShouldMakeNetworkRequestForURL(common_params_.url)) {
+    // TODO(clamy): pass the real value for |is_external_protocol|.

[davidben, 2015/09/01 21:55:18]

Hrm. It seems to manage that, we'd need to either fire this until the network
request has hit the IO thread (and so it becomes UI -> IO -> UI -> IO before
starting the request), or we have a way to compute it on the UI thread, which
means the OnRequestRedirect logic changes.

I haven't looked closely, but this is only used by the Android Intents code,
right? It seems some codepaths actually compute it from the URL in Java with
UrlUtilities.isAcceptedScheme. Perhaps we should just switch it all to that and
remove it from this API?

(A little ugly to have the duplicated code like that, but maybe it's fine in
this context? That code is at least top-level. I don't know what context it's
used in.)

[clamy, 2015/09/03 15:30:51]

So it seems it is only used on Android to log to the DevTools console that a
navigation failed if it is not handled by an intent. This is some internal code
that was upstreamed recently, so I'm thinking maybe we could get rid of it
entirely. In any case, I don't want to do a UI -> IO -> UI -> IO hop unless
really forced to do so.

+    NavigationThrottle::ThrottleCheckResult result =
+        navigation_handle_->WillStartRequest(
+            begin_params_.method == "POST",
+            Referrer::SanitizeForRequest(common_params_.url,
+                                         common_params_.referrer),
+            begin_params_.has_user_gesture, common_params_.transition, false);
+
+    // Abort the request if needed. This will destroy the NavigationRequest.
+    if (result == NavigationThrottle::CANCEL_AND_IGNORE) {
+      frame_tree_node_->ResetNavigationRequest(false);
+      return false;
+    }
+
     loader_ = NavigationURLLoader::Create(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
         frame_tree_node_->frame_tree_node_id(), info_.Pass(), this);
     return true;
   }
 
   // There is no need to make a network request for this navigation, so commit
   // it immediately.
   state_ = RESPONSE_STARTED;
   frame_tree_node_->navigator()->CommitNavigation(
       frame_tree_node_, nullptr, scoped_ptr<StreamHandle>());
   return false;
 
   // TODO(davidben): Fire (and add as necessary) observer methods such as
   // DidStartProvisionalLoadForFrame for the navigation.
 }
 
 void NavigationRequest::CreateNavigationHandle(NavigatorDelegate* delegate) {
   navigation_handle_ = NavigationHandleImpl::Create(
-      common_params_.url, frame_tree_node_->IsMainFrame(), delegate);
+      common_params_.url, FilterURL(common_params_.url, frame_tree_node_),
+      frame_tree_node_->IsMainFrame(), delegate);
 }
 
 void NavigationRequest::TransferNavigationHandleOwnership(
     RenderFrameHostImpl* render_frame_host) {
   render_frame_host->SetNavigationHandle(navigation_handle_.Pass());
 }
 
 void NavigationRequest::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
-    const scoped_refptr<ResourceResponse>& response) {
-  // TODO(davidben): Track other changes from redirects. These are important
-  // for, e.g., reloads.
+    const scoped_refptr<ResourceResponse>& response,
+    bool is_external_protocol) {
   common_params_.url = redirect_info.new_url;
+  begin_params_.method = redirect_info.new_method;
+  common_params_.referrer.url = GURL(redirect_info.new_referrer);
 
-  // TODO(davidben): This where prerender and navigation_interceptor should be
-  // integrated. For now, just always follow all redirects.
+  // TODO(clamy): Have CSP + security upgrade checks here.
+  NavigationThrottle::ThrottleCheckResult result =
+      navigation_handle_->WillRedirectRequest(
+          common_params_.url, FilterURL(common_params_.url, frame_tree_node_),

[nasko, 2015/08/31 23:25:15]

FilterURL should be a called separately and possibly kill the renderer if it
fails. Even if we don't do the kill now, the code will be easier to refactor
later on.

[clamy, 2015/09/03 15:30:51]

Done. I have added a TODO for the kill (note that the current code does not kill
the renderer if it fails).

+          begin_params_.method == "POST", common_params_.referrer.url,
+          is_external_protocol);
+
+  // Abort the request if needed. This will destroy the NavigationRequest.
+  if (result == NavigationThrottle::CANCEL_AND_IGNORE) {
+    frame_tree_node_->ResetNavigationRequest(false);
+    return;
+  }
+
   loader_->FollowRedirect();
 
   navigation_handle_->DidRedirectNavigation(redirect_info.new_url);
 }
 
 void NavigationRequest::OnResponseStarted(
     const scoped_refptr<ResourceResponse>& response,
     scoped_ptr<StreamHandle> body) {
   DCHECK(state_ == STARTED);
   state_ = RESPONSE_STARTED;
   frame_tree_node_->navigator()->CommitNavigation(frame_tree_node_,
                                                   response.get(), body.Pass());
 }
 
 void NavigationRequest::OnRequestFailed(bool has_stale_copy_in_cache,
                                         int net_error) {
   DCHECK(state_ == STARTED);
   state_ = FAILED;
   navigation_handle_->set_net_error_code(static_cast<net::Error>(net_error));
   frame_tree_node_->navigator()->FailedNavigation(
       frame_tree_node_, has_stale_copy_in_cache, net_error);
 }
 
 void NavigationRequest::OnRequestStarted(base::TimeTicks timestamp) {
   frame_tree_node_->navigator()->LogResourceRequestTime(timestamp,
                                                         common_params_.url);
 }
 
 }  // namespace content