Ensure we properly set PageTransition for iframes.

We currently don't set subframe navigations as manual when it isn't the first
navigation of the iframe. Also, we don't propagate the state correctly in the
case of a cross-process transition.

BUG=464014

Review URL: https://codereview.chromium.org/1002953004

Cr-Commit-Position: refs/heads/master@{#326404}

content/browser/frame_host/navigator.h

@@ -101,6 +101,7 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
                               const GURL& url,
                               SiteInstance* source_site_instance,
                               const Referrer& referrer,
+                              ui::PageTransition page_transition,
                               WindowOpenDisposition disposition,
                               bool should_replace_current_entry,
                               bool user_gesture) {}

content/browser/frame_host/navigator_impl.cc

@@ -518,6 +518,7 @@ void NavigatorImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
                                    const GURL& url,
                                    SiteInstance* source_site_instance,
                                    const Referrer& referrer,
+                                   ui::PageTransition page_transition,
                                    WindowOpenDisposition disposition,
                                    bool should_replace_current_entry,
                                    bool user_gesture) {
@@ -539,9 +540,9 @@ void NavigatorImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
   // redirects.  http://crbug.com/311721.
   std::vector<GURL> redirect_chain;
   RequestTransferURL(render_frame_host, url, source_site_instance,
-                     redirect_chain, referrer, ui::PAGE_TRANSITION_LINK,
-                     disposition, GlobalRequestID(),
-                     should_replace_current_entry, user_gesture);
+                     redirect_chain, referrer, page_transition, disposition,
+                     GlobalRequestID(), should_replace_current_entry,
+                     user_gesture);
 }
 
 void NavigatorImpl::RequestTransferURL(

content/browser/frame_host/navigator_impl.h

@@ -57,6 +57,7 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
                       const GURL& url,
                       SiteInstance* source_site_instance,
                       const Referrer& referrer,
+                      ui::PageTransition page_transition,
                       WindowOpenDisposition disposition,
                       bool should_replace_current_entry,
                       bool user_gesture) override;

content/browser/frame_host/render_frame_host_impl.cc

@@ -1672,8 +1672,14 @@ void RenderFrameHostImpl::OpenURL(const FrameHostMsg_OpenURL_Params& params,
 
   TRACE_EVENT1("navigation", "RenderFrameHostImpl::OpenURL", "url",
                validated_url.possibly_invalid_spec());
+  ui::PageTransition transition = ui::PAGE_TRANSITION_LINK;
+  if (frame_tree_node_->parent()) {
+    transition = params.should_replace_current_entry
+                     ? ui::PAGE_TRANSITION_AUTO_SUBFRAME
+                     : ui::PAGE_TRANSITION_MANUAL_SUBFRAME;
+  }
   frame_tree_node_->navigator()->RequestOpenURL(
-      this, validated_url, source_site_instance, params.referrer,
+      this, validated_url, source_site_instance, params.referrer, transition,
       params.disposition, params.should_replace_current_entry,
       params.user_gesture);
 }

content/browser/security_exploit_browsertest.cc

@@ -70,7 +70,7 @@ RenderViewHostImpl* PrepareToDuplicateHosts(Shell* shell,
   WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell->web_contents());
   wc->GetFrameTree()->root()->navigator()->RequestOpenURL(
       wc->GetFrameTree()->root()->current_frame_host(), extension_url, nullptr,
-      Referrer(), CURRENT_TAB, false, true);
+      Referrer(), ui::PAGE_TRANSITION_LINK, CURRENT_TAB, false, true);
 
   // Since the navigation above requires a cross-process swap, there will be a
   // pending RenderViewHost. Ensure it exists and is in a different process

content/browser/site_per_process_browsertest.cc

@@ -18,6 +18,7 @@
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/frame_messages.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -2044,4 +2045,33 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, RFPHDestruction) {
       DepictFrameTree(root));
 }
 
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       PageTransitionForSecondaryIframeNavigation) {
+  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
+  NavigateToURL(shell(), main_url);
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  TestNavigationObserver observer(shell()->web_contents());
+
+  // Load same-site page into iframe.
+  FrameTreeNode* child = root->child_at(0);
+  GURL http_url(embedded_test_server()->GetURL("/title1.html"));
+  NavigateFrameToURL(child, http_url);
+  EXPECT_EQ(http_url, observer.last_navigation_url());
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+
+  // Load cross-site page into iframe.
+  TestFrameNavigationObserver frame_observer(child, 1);
+  GURL url = embedded_test_server()->GetURL("foo.com", "/title2.html");
+  NavigateIframeToURL(shell()->web_contents(), "test", url);
+  frame_observer.Wait();
+
+  EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME,
+            frame_observer.load_committed_details().type);
+}
+
 }  // namespace content

content/renderer/render_frame_impl.cc

@@ -1100,6 +1100,20 @@ void RenderFrameImpl::OnNavigate(
                  << request_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.
@@ -2554,6 +2568,8 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
   document_state->set_start_load_time(Time::Now());
 
   bool is_top_most = !frame->parent();
+  NavigationStateImpl* navigation_state =
+      static_cast<NavigationStateImpl*>(document_state->navigation_state());
   if (is_top_most) {
     render_view_->set_navigation_gesture(
         WebUserGestureIndicator::isProcessingUserGesture() ?
@@ -2562,8 +2578,17 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
     // Subframe navigations that don't add session history items must be
     // marked with AUTO_SUBFRAME. See also didFailProvisionalLoad for how we
     // handle loading of error pages.
-    static_cast<NavigationStateImpl*>(document_state->navigation_state())
-        ->set_transition_type(ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+    navigation_state->set_transition_type(ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  } else if (ui::PageTransitionCoreTypeIs(navigation_state->GetTransitionType(),
+                                          ui::PAGE_TRANSITION_LINK)) {
+    // Subframe navigations that are creating a new history item should be
+    // marked MANUAL_SUBFRAME, unless it has already been marked as a
+    // FORM_SUBMIT. This state will be attached to a main resource request
+    // in the process that began the request. If the request is transferred
+    // to a different process, this state will be used in
+    // RenderFrameImpl::OnNavigate() in the new process (as well as in the
+    // browser process).
+    navigation_state->set_transition_type(ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
   }
 
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers(),
@@ -3147,16 +3172,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);
@@ -3295,8 +3313,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.

content/test/test_frame_navigation_observer.cc

@@ -67,6 +67,7 @@ void TestFrameNavigationObserver::DidNavigateAnyFrame(
 
   ++navigations_completed_;
   if (navigations_completed_ == number_of_navigations_) {
+    load_committed_details_ = details;
     navigation_started_ = false;
     message_loop_runner_->Quit();
   }

content/test/test_frame_navigation_observer.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/test_utils.h"
 
@@ -19,7 +20,6 @@ namespace content {
 class FrameTreeNode;
 class RenderFrameHostImpl;
 class WebContents;
-struct LoadCommittedDetails;
 
 // For content_browsertests, which run on the UI thread, run a second
 // MessageLoop and quit when the navigation in a specific frame completes
@@ -38,6 +38,11 @@ class TestFrameNavigationObserver : public WebContentsObserver {
   // navigations are complete.
   void Wait();
 
+  // Returns the LoadCommittedDetails for the last navigation to commit.
+  const LoadCommittedDetails& load_committed_details() const {
+    return load_committed_details_;
+  }
+
  private:
   // WebContentsObserver
   void DidStartProvisionalLoadForFrame(RenderFrameHost* render_frame_host,
@@ -60,6 +65,8 @@ class TestFrameNavigationObserver : public WebContentsObserver {
   // The number of navigations to wait for.
   int number_of_navigations_;
 
+  LoadCommittedDetails load_committed_details_;
+
   // The MessageLoopRunner used to spin the message loop.
   scoped_refptr<MessageLoopRunner> message_loop_runner_;