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

Unified Diff: content/renderer/render_frame_impl.cc

Issue 743803002: Avoid stale navigation requests without excessive page id knowledge in the renderer process. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fixes, test Created 6 years 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/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 99b9b512ea97eb7a82ce37aa9c54e911cfd94fc3..dda00c30077931f459886cfe3c72d5345a2423a1 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -947,35 +947,33 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) {
TRACE_EVENT2("navigation", "RenderFrameImpl::OnNavigate",
"id", routing_id_,
"url", params.common_params.url.possibly_invalid_spec());
+
+ // If this is a stale back/forward (due to a recent navigation the browser
+ // didn't know about), ignore it. Only check if swapped in because if the
+ // frame is swapped out, it won't commit before asking the browser.
+ bool is_history_navigation = params.commit_params.page_state.IsValid();
+ if (!render_view_->is_swapped_out() && is_history_navigation &&
+ render_view_->history_list_offset_ !=
+ params.current_history_list_offset) {
+ return;
+ }
+
bool is_reload =
RenderViewImpl::IsReload(params.common_params.navigation_type);
WebURLRequest::CachePolicy cache_policy =
WebURLRequest::UseProtocolCachePolicy;
if (!RenderFrameImpl::PrepareRenderViewForNavigation(
params.common_params.url, params.common_params.navigation_type,
- params.commit_params.page_state, true, params.pending_history_list_offset,
- params.page_id, &is_reload, &cache_policy)) {
+ params.commit_params.page_state, params.page_id, &is_reload,
+ &cache_policy)) {
return;
}
- int pending_history_list_offset = params.pending_history_list_offset;
- int current_history_list_offset = params.current_history_list_offset;
- int current_history_list_length = params.current_history_list_length;
+ render_view_->history_list_offset_ = params.current_history_list_offset;
+ render_view_->history_list_length_ = params.current_history_list_length;
if (params.should_clear_history_list) {
- CHECK_EQ(pending_history_list_offset, -1);
- CHECK_EQ(current_history_list_offset, -1);
- CHECK_EQ(current_history_list_length, 0);
- }
- render_view_->history_list_offset_ = current_history_list_offset;
- render_view_->history_list_length_ = current_history_list_length;
- if (render_view_->history_list_length_ >= 0) {
- render_view_->history_page_ids_.resize(
- render_view_->history_list_length_, -1);
- }
- if (pending_history_list_offset >= 0 &&
- pending_history_list_offset < render_view_->history_list_length_) {
- render_view_->history_page_ids_[pending_history_list_offset] =
- params.page_id;
+ CHECK_EQ(-1, render_view_->history_list_offset_);
+ CHECK_EQ(0, render_view_->history_list_length_);
}
GetContentClient()->SetActiveURL(params.common_params.url);
@@ -1013,7 +1011,7 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) {
frame->reloadWithOverrideURL(params.common_params.url, true);
else
frame->reload(ignore_cache);
- } else if (params.commit_params.page_state.IsValid()) {
+ } else if (is_history_navigation) {
// We must know the page ID of the page we are navigating back to.
DCHECK_NE(params.page_id, -1);
scoped_ptr<HistoryEntry> entry =
@@ -2352,7 +2350,7 @@ void RenderFrameImpl::didCommitProvisionalLoad(
// We bump our Page ID to correspond with the new session history entry.
render_view_->page_id_ = render_view_->next_page_id_++;
- // Don't update history_page_ids_ (etc) for kSwappedOutURL, since
+ // Don't update history list values for kSwappedOutURL, since
// we don't want to forget the entry that was there, and since we will
// never come back to kSwappedOutURL. Note that we have to call
// UpdateSessionHistory and update page_id_ even in this case, so that
@@ -2366,10 +2364,6 @@ void RenderFrameImpl::didCommitProvisionalLoad(
render_view_->history_list_offset_ = kMaxSessionHistoryEntries - 1;
render_view_->history_list_length_ =
render_view_->history_list_offset_ + 1;
- render_view_->history_page_ids_.resize(
- render_view_->history_list_length_, -1);
- render_view_->history_page_ids_[render_view_->history_list_offset_] =
- render_view_->page_id_;
}
} else {
// Inspect the navigation_state on this frame to see if the navigation
@@ -2390,14 +2384,6 @@ void RenderFrameImpl::didCommitProvisionalLoad(
render_view_->history_list_offset_ =
navigation_state->pending_history_list_offset();
-
- // If the history list is valid, our list of page IDs should be correct.
- DCHECK(render_view_->history_list_length_ <= 0 ||
- render_view_->history_list_offset_ < 0 ||
- render_view_->history_list_offset_ >=
- render_view_->history_list_length_ ||
- render_view_->history_page_ids_[render_view_->history_list_offset_]
- == render_view_->page_id_);
}
}
@@ -3688,7 +3674,7 @@ void RenderFrameImpl::OnCommitNavigation(
WebURLRequest::UseProtocolCachePolicy;
if (!RenderFrameImpl::PrepareRenderViewForNavigation(
common_params.url, common_params.navigation_type,
- commit_params.page_state, false, -1, -1, &is_reload, &cache_policy)) {
+ commit_params.page_state, -1, &is_reload, &cache_policy)) {
return;
}
@@ -4110,8 +4096,6 @@ bool RenderFrameImpl::PrepareRenderViewForNavigation(
const GURL& url,
FrameMsg_Navigate_Type::Value navigate_type,
const PageState& state,
- bool check_history,
- int pending_history_list_offset,
int32 page_id,
bool* is_reload,
WebURLRequest::CachePolicy* cache_policy) {
@@ -4122,12 +4106,6 @@ bool RenderFrameImpl::PrepareRenderViewForNavigation(
FOR_EACH_OBSERVER(
RenderViewObserver, render_view_->observers_, Navigate(url));
- // If this is a stale back/forward (due to a recent navigation the browser
- // didn't know about), ignore it.
- if (check_history && render_view_->IsBackForwardToStaleEntry(
- state, pending_history_list_offset, page_id, *is_reload))
- return false;
-
if (!is_swapped_out_ || frame_->parent())
return true;

Powered by Google App Engine
This is Rietveld 408576698