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 895d3891a6ca6662039bd1f7a5bf949729ed204b..81eb5c65569ddd7fd0af8bcf4c89dc52472b99a6 100644 |
--- a/content/browser/frame_host/navigation_request.cc |
+++ b/content/browser/frame_host/navigation_request.cc |
@@ -220,6 +220,19 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated( |
// the renderer in the first place as part of OpenURL. |
bool browser_initiated = !entry.is_renderer_initiated(); |
+ bool is_history_navigation = frame_entry.page_state().IsValid(); |
+ |
+ // A same-document fragment-navigation happens when the only part of the url |
+ // that is modified is after the '#' character. |
+ // Be careful not to consider history navigations. For instance, if the |
+ // history is: 'A#bar' -> 'B' -> 'A#foo'. Then an history navigation from |
+ // 'A#foo' to 'A#bar' is not a same-document navigation, but a |
+ // different-document one! The two FrameNavigationEntry doesn't share the same |
+ // document_sequence_number. |
+ bool is_same_document_fragment_change = |
+ net::IsFragmentAddedOrUpdated(frame_tree_node->current_url(), dest_url) && |
+ !is_history_navigation; |
nasko
2017/01/13 19:36:57
Thanks for the comment! I was discussing this logi
arthursonzogni
2017/01/17 15:24:47
Thanks for shouldPerformFragmentNavigation. I will
|
+ |
std::unique_ptr<NavigationRequest> navigation_request(new NavigationRequest( |
frame_tree_node, entry.ConstructCommonNavigationParams( |
frame_entry, request_body, dest_url, dest_referrer, |
@@ -231,8 +244,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated( |
blink::WebMixedContentContextType::Blockable, |
initiator), |
entry.ConstructRequestNavigationParams( |
- frame_entry, is_same_document_history_load, |
- is_history_navigation_in_new_child, |
+ frame_entry, is_same_document_fragment_change, |
+ is_same_document_history_load, is_history_navigation_in_new_child, |
entry.GetSubframeUniqueNames(frame_tree_node), |
frame_tree_node->has_committed_real_load(), |
controller->GetPendingEntryIndex() == -1, |
@@ -252,34 +265,42 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated( |
const BeginNavigationParams& begin_params, |
int current_history_list_offset, |
int current_history_list_length) { |
- // TODO(clamy): Check if some PageState should be provided here. |
+ |
+ // No renderer-initiated same-document navigation are using this method. The |
+ // navigation takes place in the renderer without asking the browser to |
+ // navigate. |
+ DCHECK(net::IsFragmentAddedOrUpdated(frame_tree_node->current_url(), |
+ common_params.url)); |
+ |
+ // Please note that no history-navigation uses this method as well. |
nasko
2017/01/13 19:36:57
nit: Put the comments together before the DCHECK.
|
+ |
// TODO(clamy): See how we should handle override of the user agent when the |
// navigation may start in a renderer and commit in another one. |
// TODO(clamy): See if the navigation start time should be measured in the |
// renderer and sent to the browser instead of being measured here. |
// TODO(clamy): The pending history list offset should be properly set. |
RequestNavigationParams request_params( |
- false, // is_overriding_user_agent |
- std::vector<GURL>(), // redirects |
- false, // can_load_local_resources |
- PageState(), // page_state |
- 0, // nav_entry_id |
- false, // is_same_document_history_load |
- false, // is_history_navigation_in_new_child |
- std::map<std::string, bool>(), // subframe_unique_names |
+ false, // is_overriding_user_agent |
+ std::vector<GURL>(), // redirects |
+ false, // can_load_local_resources |
+ PageState(), // page_state |
+ 0, // nav_entry_id |
+ false, // is_same_document_fragment_change |
+ false, // is_same_document_history_load |
+ false, // is_history_navigation_in_new_child |
+ std::map<std::string, bool>(), // subframe_unique_names |
frame_tree_node->has_committed_real_load(), |
- false, // intended_as_new_entry |
- -1, // pending_history_list_offset |
+ false, // intended_as_new_entry |
+ -1, // pending_history_list_offset |
current_history_list_offset, current_history_list_length, |
- false, // is_view_source |
- false, // should_clear_history_list |
+ false, // is_view_source |
+ false, // should_clear_history_list |
begin_params.has_user_gesture); |
- std::unique_ptr<NavigationRequest> navigation_request( |
- new NavigationRequest(frame_tree_node, common_params, begin_params, |
- request_params, |
- false, // browser_initiated |
- false, // may_transfer |
- nullptr, nullptr)); |
+ std::unique_ptr<NavigationRequest> navigation_request(new NavigationRequest( |
+ frame_tree_node, common_params, begin_params, request_params, |
+ false, // browser_initiated |
+ false, // may_transfer |
+ nullptr, nullptr)); |
return navigation_request; |
} |
@@ -346,7 +367,8 @@ void NavigationRequest::BeginNavigation() { |
state_ = STARTED; |
RenderFrameDevToolsAgentHost::OnBeforeNavigation(navigation_handle_.get()); |
- if (ShouldMakeNetworkRequestForURL(common_params_.url)) { |
+ if (ShouldMakeNetworkRequestForURL(common_params_.url) && |
+ !navigation_handle_->IsSamePage()) { |
// It's safe to use base::Unretained because this NavigationRequest owns |
// the NavigationHandle where the callback will be stored. |
// TODO(clamy): pass the real value for |is_external_protocol| if needed. |
@@ -381,11 +403,11 @@ void NavigationRequest::BeginNavigation() { |
} |
void NavigationRequest::CreateNavigationHandle(int pending_nav_entry_id) { |
- // TODO(nasko): Update the NavigationHandle creation to ensure that the |
- // proper values are specified for is_same_page. |
+ bool is_same_page = request_params_.is_same_document_fragment_change || |
+ request_params_.is_same_document_history_load; |
+ |
navigation_handle_ = NavigationHandleImpl::Create( |
- common_params_.url, frame_tree_node_, !browser_initiated_, |
- false, // is_same_page |
+ common_params_.url, frame_tree_node_, !browser_initiated_, is_same_page, |
common_params_.navigation_start, pending_nav_entry_id, |
false); // started_in_context_menu |
@@ -697,7 +719,8 @@ void NavigationRequest::OnWillProcessResponseChecksComplete( |
} |
void NavigationRequest::CommitNavigation() { |
- DCHECK(response_ || !ShouldMakeNetworkRequestForURL(common_params_.url)); |
+ DCHECK(response_ || !ShouldMakeNetworkRequestForURL(common_params_.url) || |
+ navigation_handle_->IsSamePage()); |
DCHECK(!common_params_.url.SchemeIs(url::kJavaScriptScheme)); |
// Retrieve the RenderFrameHost that needs to commit the navigation. |