Remove the use of page id from building the commit params.

content/browser/frame_host/navigation_controller_impl_browsertest.cc

Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index 3eca788f95720c819ad1e27b955f30f2d4c5b7fd..72ddd70e3b36a55362a66525acbe7851b88aa391 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -4,17 +4,29 @@
 
 #include "base/bind.h"
 #include "base/strings/stringprintf.h"
+#include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
+#include "content/test/content_browser_test_utils_internal.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
 
 namespace content {
 
 class NavigationControllerBrowserTest : public ContentBrowserTest {
+ protected:
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+  }
 };
 
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, LoadDataWithBaseURL) {
@@ -76,5 +88,111 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
             controller.GetLastCommittedEntry()->GetURL());
 }
 
-}  // namespace content
+struct FrameNavigateParamsCapturer : public WebContentsObserver {
+ public:
+  explicit FrameNavigateParamsCapturer(FrameTreeNode* node)
+      : WebContentsObserver(
+            node->current_frame_host()->delegate()->GetAsWebContents()),

[Charlie Reis, 2015/02/02 17:50:32]

This feels a bit awkward to go through the RFH's delegate to get the
WebContents.  Maybe it's worth passing that in as a separate parameter?

(I agree that we shouldn't just use web_contents() here, which could be wrong in
a bunch of cases, like tests that have a separate tab, or the upcoming plans for
<webview>.)

[Avi (use Gerrit), 2015/02/02 20:59:35]

This is stolen straight from TestFrameNavigationObserver. I don't mind the
complexity; I'd rather pass in one item and be awkward getting everything we
need if it guarantees that nothing is contradictory.

+        frame_tree_node_id_(node->frame_tree_node_id()),
+        message_loop_runner_(new MessageLoopRunner) {}
+
+  void Wait() {
+    message_loop_runner_->Run();
+  }
+
+  const FrameNavigateParams& params() const {
+    return params_;
+  }
+
+  const LoadCommittedDetails& details() const {
+    return details_;
+  }
+
+ private:
+  void DidNavigateAnyFrame(RenderFrameHost* render_frame_host,
+                           const LoadCommittedDetails& details,
+                           const FrameNavigateParams& params) override {
+    RenderFrameHostImpl* rfh =
+        static_cast<RenderFrameHostImpl*>(render_frame_host);
+    if (rfh->frame_tree_node()->frame_tree_node_id() != frame_tree_node_id_)
+      return;
+
+    params_ = params;
+    details_ = details;
+    message_loop_runner_->Quit();
+  }
+
+  // The id of the FrameTreeNode in which navigations are peformed.

[Charlie Reis, 2015/02/02 17:50:32]

of the FrameTreeNode to observe.

(It's fine for navigations to occur in other frames as well; this class will
still work.)

[Avi (use Gerrit), 2015/02/02 20:59:36]

Done.

+  int frame_tree_node_id_;
+
+  // The params of the last navigation.
+  FrameNavigateParams params_;
+
+  // The details of the last navigation.
+  LoadCommittedDetails details_;
 
+  // The MessageLoopRunner used to spin the message loop.
+  scoped_refptr<MessageLoopRunner> message_loop_runner_;
+};
+
+// Verify that the distinction between manual and auto subframes is properly set
+// for subframe navigations. TODO(avi): It's rather bogus that the same info is
+// in two different enums; http://crbug.com/453555

[Charlie Reis, 2015/02/02 17:50:32]

nit: End with period.  (Codesearch gets the link right.)

[Avi (use Gerrit), 2015/02/02 20:59:36]

Done.

+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       ManualAndAutoSubframeNavigationClassification) {
+  GURL main_url(
+      embedded_test_server()->GetURL("/frame_tree/page_with_one_frame.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();
+
+  ASSERT_EQ(1U, root->child_count());
+  ASSERT_NE(nullptr, root->child_at(0));
+
+  {
+    // Navigate the iframe to a new URL; expect a manual subframe transition.
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    GURL frame_url(
+          embedded_test_server()->GetURL("/frame_tree/2-1.html"));

[Charlie Reis, 2015/02/02 17:50:32]

nit: Wrong indent.

[Avi (use Gerrit), 2015/02/02 20:59:36]

Done.

+    NavigateFrameToURL(root->child_at(0), frame_url);
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
+              capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
+  }
+
+  {
+    // History navigations should result in an auto subframe transition.
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    shell()->web_contents()->GetController().GoBack();
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.details().type);
+  }
+
+  {
+    // History navigations should result in an auto subframe transition.
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    shell()->web_contents()->GetController().GoForward();
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.details().type);
+  }
+
+  {
+    // Navigate the iframe to a new URL; expect a manual subframe transition.
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    GURL frame_url(
+          embedded_test_server()->GetURL("/frame_tree/2-3.html"));

[Charlie Reis, 2015/02/02 17:50:32]

nit: Wrong indent.

[Avi (use Gerrit), 2015/02/02 20:59:35]

Done.

+    NavigateFrameToURL(root->child_at(0), frame_url);
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
+              capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
+  }
+}
+
+}  // namespace content