Index: content/browser/frame_host/navigation_controller_impl.cc |
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc |
index 0b0f411a1a041f52893e84357af935f7fe81c0df..3cefb2d94804692a5e671c70b279559d11649987 100644 |
--- a/content/browser/frame_host/navigation_controller_impl.cc |
+++ b/content/browser/frame_host/navigation_controller_impl.cc |
@@ -900,14 +900,15 @@ NavigationType NavigationControllerImpl::ClassifyNavigation( |
RenderFrameHostImpl* rfh, |
const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { |
if (params.page_id == -1) { |
- // TODO(nasko, creis): An out-of-process child frame has no way of |
- // knowing the page_id of its parent, so it is passing back -1. The |
- // semantics here should be re-evaluated during session history refactor |
- // (see http://crbug.com/236848). For now, we assume this means the |
- // child frame loaded and proceed. Note that this may do the wrong thing |
- // for cross-process AUTO_SUBFRAME navigations. |
- if (rfh->IsCrossProcessSubframe()) |
- return NAVIGATION_TYPE_NEW_SUBFRAME; |
+ if (rfh->IsCrossProcessSubframe()) { |
+ CHECK(!ui::PageTransitionIsMainFrame(params.transition)); |
Charlie Reis
2015/03/12 22:54:17
DCHECK. The renderer could be exploited or buggy
Nate Chapin
2015/03/13 21:48:35
Oops, I wasn't careful about this while testing an
|
+ if (ui::PageTransitionCoreTypeIs(params.transition, |
+ ui::PAGE_TRANSITION_MANUAL_SUBFRAME)) { |
+ return NAVIGATION_TYPE_NEW_SUBFRAME; |
+ } else { |
+ return NAVIGATION_TYPE_AUTO_SUBFRAME; |
+ } |
+ } |
// The renderer generates the page IDs, and so if it gives us the invalid |
// page ID (-1) we know it didn't actually navigate. This happens in a few |
@@ -1232,27 +1233,8 @@ void NavigationControllerImpl::RendererDidNavigateInPage( |
void NavigationControllerImpl::RendererDidNavigateNewSubframe( |
RenderFrameHostImpl* rfh, |
const FrameHostMsg_DidCommitProvisionalLoad_Params& params) { |
- if (!ui::PageTransitionCoreTypeIs(params.transition, |
- ui::PAGE_TRANSITION_MANUAL_SUBFRAME)) { |
- // There was a comment here that said, "This is not user-initiated. Ignore." |
- // But this makes no sense; non-user-initiated navigations should be |
- // determined to be of type NAVIGATION_TYPE_AUTO_SUBFRAME and sent to |
- // RendererDidNavigateAutoSubframe below. |
- // |
- // This if clause dates back to https://codereview.chromium.org/115919 and |
- // the handling of immediate redirects. TODO(avi): Is this still valid? I'm |
- // pretty sure that's there's nothing left of that code and that we should |
- // take this out. |
- // |
- // Except for cross-process iframes; this doesn't work yet for them. |
- if (!base::CommandLine::ForCurrentProcess()->HasSwitch( |
- switches::kSitePerProcess)) { |
- NOTREACHED(); |
- } |
- |
- DiscardNonCommittedEntriesInternal(); |
- return; |
- } |
+ DCHECK(ui::PageTransitionCoreTypeIs(params.transition, |
+ ui::PAGE_TRANSITION_MANUAL_SUBFRAME)); |
// Manual subframe navigations just get the current entry cloned so the user |
// can go back or forward to it. The actual subframe information will be |