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

Unified Diff: content/renderer/render_frame_impl.cc

Issue 1110943003: Revert of Classify navigations without page id in parallel to the existing classifier. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 8 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
« no previous file with comments | « content/public/test/web_contents_tester.h ('k') | content/renderer/render_view_browsertest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index dfa09aaaeb606b3551bad7da2c8f2cb5c58ceca5..decaf5e6c31a819882eb1f5eb701d85caaecfbb6 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1119,10 +1119,6 @@
} else if (is_history_navigation) {
// We must know the page ID of the page we are navigating back to.
DCHECK_NE(request_params.page_id, -1);
- // We must know the nav entry ID of the page we are navigating back to,
- // which should be the case because history navigations are routed via the
- // browser.
- DCHECK_NE(0, request_params.nav_entry_id);
scoped_ptr<HistoryEntry> entry =
PageStateToHistoryEntry(request_params.page_state);
if (entry) {
@@ -2587,10 +2583,6 @@
const WebURLRequest& failed_request = ds->request();
- DocumentState* document_state = DocumentState::FromDataSource(ds);
- NavigationStateImpl* navigation_state =
- static_cast<NavigationStateImpl*>(document_state->navigation_state());
-
// Notify the browser that we failed a provisional load with an error.
//
// Note: It is important this notification occur before DidStopLoading so the
@@ -2609,6 +2601,10 @@
// Make sure we never show errors in view source mode.
frame->enableViewSourceMode(false);
+
+ DocumentState* document_state = DocumentState::FromDataSource(ds);
+ NavigationStateImpl* navigation_state =
+ static_cast<NavigationStateImpl*>(document_state->navigation_state());
// If this is a failed back/forward/reload navigation, then we need to do a
// 'replace' load. This is necessary to avoid messing up session history.
@@ -2696,7 +2692,18 @@
render_view_->history_list_offset_ + 1;
}
} else {
- if (navigation_state->request_params().page_id != -1) {
+ // Inspect the navigation_state on this frame to see if the navigation
+ // corresponds to a session history navigation... Note: |frame| may or
+ // may not be the toplevel frame, but for the case of capturing session
+ // history, the first committed frame suffices. We keep track of whether
+ // we've seen this commit before so that only capture session history once
+ // per navigation.
+ //
+ // Note that we need to check if the page ID changed. In the case of a
+ // reload, the page ID doesn't change, and UpdateSessionHistory gets the
+ // previous URL and the current page ID, which would be wrong.
+ if (navigation_state->request_params().page_id != -1 &&
+ navigation_state->request_params().page_id != render_view_->page_id_) {
// This is a successful session history navigation!
render_view_->page_id_ = navigation_state->request_params().page_id;
@@ -3854,12 +3861,8 @@
params.http_status_code = response.httpStatusCode();
params.url_is_unreachable = ds->hasUnreachableURL();
params.is_post = false;
- params.intended_as_new_entry =
- navigation_state->request_params().intended_as_new_entry;
- params.did_create_new_entry = commit_type == blink::WebStandardCommit;
params.post_id = -1;
params.page_id = render_view_->page_id_;
- params.nav_entry_id = navigation_state->request_params().nav_entry_id;
// We need to track the RenderViewHost routing_id because of downstream
// dependencies (crbug.com/392171 DownloadRequestHandle, SaveFileManager,
// ResourceDispatcherHostImpl, MediaStreamUIProxy,
« no previous file with comments | « content/public/test/web_contents_tester.h ('k') | content/renderer/render_view_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698