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

Unified Diff: content/renderer/render_frame_impl.cc

Issue 1002953004: Ensure we properly set PageTransition for iframes. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 9 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/browser/frame_host/navigation_controller_impl.cc ('k') | no next file » | 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 437e8c98be937398ffd4b3ca46cdfb4c5b964f16..2d14282527d468f41a8eab581008f09f70b5a803 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1063,6 +1063,20 @@ void RenderFrameImpl::OnNavigate(
<< commit_params.frame_to_navigate;
}
+ // If this frame isn't in the same process as its parent, it will naively
+ // assume that this is the first navigation in the iframe, but this may not
+ // actually be the case. The PageTransition differentiates between the first
+ // navigation in a subframe and subsequent navigations, so if this is a
+ // subsequent navigation, force the frame's state machine forward.
+ if (ui::PageTransitionCoreTypeIs(common_params.transition,
+ ui::PAGE_TRANSITION_MANUAL_SUBFRAME)) {
+ CHECK(frame_->parent());
+ if (frame_->parent()->isWebRemoteFrame()) {
+ CHECK_EQ(frame, frame_);
+ frame_->setCommittedFirstRealLoad();
+ }
+ }
+
if (is_reload && !render_view_->history_controller()->GetCurrentEntry()) {
// We cannot reload if we do not have any history state. This happens, for
// example, when recovering from a crash.
@@ -2343,6 +2357,14 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
// handle loading of error pages.
document_state->navigation_state()->set_transition_type(
ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+ } else if (ui::PageTransitionCoreTypeIs(
+ document_state->navigation_state()->transition_type(),
+ ui::PAGE_TRANSITION_LINK)) {
Charlie Reis 2015/03/13 23:22:14 Why is PAGE_TRANSITION_LINK the right way to detec
Nate Chapin 2015/03/13 23:28:38 This is the initial classification of a navigation
Charlie Reis 2015/03/14 00:03:49 Ah! It makes so much more sense now. :) Can you
+ // Subframe navigations that are creating a new history item should be
+ // marked MANUAL_SUBFRAME, unless it has already been marked as a
+ // FORM_SUBMIT.
+ document_state->navigation_state()->set_transition_type(
+ ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
}
FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers(),
@@ -2975,16 +2997,9 @@ void RenderFrameImpl::willSendRequest(
}
}
- WebFrame* top_frame = frame->top();
- // TODO(nasko): Hack around asking about top-frame data source. This means
- // for out-of-process iframes we are treating the current frame as the
- // top-level frame, which is wrong.
- if (!top_frame || top_frame->isWebRemoteFrame())
- top_frame = frame;
- WebDataSource* provisional_data_source = top_frame->provisionalDataSource();
- WebDataSource* top_data_source = top_frame->dataSource();
+ WebDataSource* provisional_data_source = frame->provisionalDataSource();
WebDataSource* data_source =
- provisional_data_source ? provisional_data_source : top_data_source;
+ provisional_data_source ? provisional_data_source : frame->dataSource();
DocumentState* document_state = DocumentState::FromDataSource(data_source);
DCHECK(document_state);
@@ -3121,8 +3136,14 @@ void RenderFrameImpl::willSendRequest(
extra_data->set_stream_override(stream_override.Pass());
request.setExtraData(extra_data);
+ WebFrame* top_frame = frame->top();
+ // TODO(nasko): Hack around asking about top-frame data source. This means
+ // for out-of-process iframes we are treating the current frame as the
+ // top-level frame, which is wrong.
+ if (!top_frame || top_frame->isWebRemoteFrame())
+ top_frame = frame;
DocumentState* top_document_state =
- DocumentState::FromDataSource(top_data_source);
+ DocumentState::FromDataSource(top_frame->dataSource());
if (top_document_state) {
// TODO(gavinp): separate out prefetching and prerender field trials
// if the rel=prerender rel type is sticking around.
« no previous file with comments | « content/browser/frame_host/navigation_controller_impl.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698