Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(266)

Unified Diff: content/browser/frame_host/navigation_request.cc

Issue 1269813002: Add a NavigationThrottle to the public content/ interface (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@navigation-api
Patch Set: Rebase Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index a99074f2ce3aeb56440b44cc0e9530f0f0c1db39..d625058fa3b9127cc9d5a9609fb407f293361cff 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -7,6 +7,7 @@
#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_factory.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"
@@ -14,6 +15,7 @@
#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"
@@ -49,6 +51,17 @@ int LoadFlagFromNavigationType(FrameMsg_Navigate_Type::Value navigation_type) {
return load_flags;
}
+// Filters the URL for the navigation based.
Charlie Reis 2015/09/16 00:09:24 I'm not sure what "for the navigation based" means
clamy 2015/09/16 01:03:22 Method was removed.
+GURL FilterURL(const GURL& url, FrameTreeNode* frame_tree_node) {
Charlie Reis 2015/09/16 00:09:25 This feels like the wrong place to be enforcing th
clamy 2015/09/16 01:03:22 I see. In that case, since this method is PlzNavig
+ RenderFrameHostImpl* render_frame_host =
+ frame_tree_node->render_manager()->speculative_frame_host()
Charlie Reis 2015/09/16 00:09:25 Is this only used in PlzNavigate?
clamy 2015/09/16 01:03:22 Yes - but I removed this method, see above.
+ ? frame_tree_node->render_manager()->speculative_frame_host()
+ : frame_tree_node->render_manager()->current_frame_host();
+ GURL filtered_url = url;
+ render_frame_host->GetProcess()->FilterURL(false, &filtered_url);
+ return filtered_url;
+}
+
} // namespace
// static
@@ -179,6 +192,20 @@ bool NavigationRequest::BeginNavigation() {
state_ = STARTED;
if (ShouldMakeNetworkRequestForURL(common_params_.url)) {
+ // TODO(clamy): pass the real value for |is_external_protocol| if needed.
+ 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);
@@ -197,8 +224,9 @@ bool NavigationRequest::BeginNavigation() {
}
void NavigationRequest::CreateNavigationHandle(NavigatorDelegate* delegate) {
- navigation_handle_ = NavigationHandleImpl::Create(
- common_params_.url, frame_tree_node_->IsMainFrame(), delegate);
+ navigation_handle_ = NavigationHandleFactory::Create(
+ common_params_.url, FilterURL(common_params_.url, frame_tree_node_),
+ frame_tree_node_->IsMainFrame(), delegate);
}
void NavigationRequest::TransferNavigationHandleOwnership(
@@ -209,12 +237,26 @@ void NavigationRequest::TransferNavigationHandleOwnership(
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.
common_params_.url = redirect_info.new_url;
+ begin_params_.method = redirect_info.new_method;
+ common_params_.referrer.url = GURL(redirect_info.new_referrer);
+
+ // TODO(clamy): Have CSP + security upgrade checks here.
+ // TODO(clamy): Kill the renderer if FilterURL fails?
+ // TODO(clamy): pass the real value for |is_external_protocol| if needed.
+ const GURL validated_new_url =
+ FilterURL(common_params_.url, frame_tree_node_);
+ NavigationThrottle::ThrottleCheckResult result =
+ navigation_handle_->WillRedirectRequest(
+ common_params_.url, validated_new_url, begin_params_.method == "POST",
+ common_params_.referrer.url, false);
+
+ // Abort the request if needed. This will destroy the NavigationRequest.
+ if (result == NavigationThrottle::CANCEL_AND_IGNORE) {
+ frame_tree_node_->ResetNavigationRequest(false);
+ return;
+ }
- // TODO(davidben): This where prerender and navigation_interceptor should be
- // integrated. For now, just always follow all redirects.
loader_->FollowRedirect();
navigation_handle_->DidRedirectNavigation(redirect_info.new_url);

Powered by Google App Engine
This is Rietveld 408576698