Ensure we properly set PageTransition for iframes.

content/browser/frame_host/navigation_controller_impl.cc

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 and send the wrong transition,
and that shouldn't crash the browser process.

[Nate Chapin, 2015/03/13 21:48:35]

Oops, I wasn't careful about this while testing and forgot to double-check them
all.

Done.

+      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