Ensure we properly set PageTransition for iframes.

content/browser/site_per_process_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/site_per_process_browsertest.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #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"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
@@ skipping 1922 matching lines @@
   EXPECT_EQ("http://bar.com/", DumpProxyHostSiteInstances(root));
 
   // Navigate back to the parent's origin.
   url = embedded_test_server()->GetURL("/title1.html");
   NavigateFrameToURL(child, url);
   EXPECT_EQ(url, observer.last_navigation_url());
   EXPECT_TRUE(observer.last_navigation_succeeded());
   EXPECT_EQ("", DumpProxyHostSiteInstances(root));
 }
 
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,

[Charlie Reis, 2015/04/15 22:46:11]

I'm torn between adding this test here (where it will run on all the bots in
--site-per-process mode) and NavigationControllerBrowserTest, where it would fit
in a little better but would only run in --site-per-process mode on the Site
Isolation FYI bots.  (It's pretty close to
NavigationTypeClassification_NewAndAutoSubframe, for example.)

I think I like having it run on more bots, so let's leave it here for now.

+                       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,

[Charlie Reis, 2015/04/15 22:46:11]

Please add a similar test for an initially cross-site page in an iframe, where
we will expect NAVIGATION_TYPE_AUTO_SUBFRAME.

See the end of
NavigationControllerBrowserTest.NavigationTypeClassification_NewAndAutoSubframe
for an example of how to watch for this when creating a new frame.

[Nate Chapin, 2015/04/17 21:48:18]

That case reliably returns NAVIGATION_TYPE_NEW_SUBFRAME, because of the same
class of bugging that causes this test to pass with and without this change. Do
you want me to add a case that asserts the wrong behavior?

[Charlie Reis, 2015/04/22 20:44:56]

Sure, with a TODO to make it AUTO.  I should be able to fix it once your CL and
Avi's CL land.

+            frame_observer.load_committed_details().type);
+}
+
 }  // namespace content