Add item and document sequence numbers to FrameNavigationEntry.

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 ae5d04d2e163dd2ca50af4fab7c2ef39d08501eb..667a767d0e3432551637ae98fdfc466c8421e8f1 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -1548,6 +1548,45 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   }
 }
 
+// Verifies that item sequence numbers and document sequence numbers update
+// properly for main frames and subframes.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       FrameNavigationEntry_SequenceNumbers) {
+  const NavigationControllerImpl& controller =
+      static_cast<const NavigationControllerImpl&>(
+          shell()->web_contents()->GetController());
+
+  // 1. Navigate the main frame.
+  GURL url2(embedded_test_server()->GetURL(
+      "/navigation_controller/page_with_links.html"));
+  NavigateToURL(shell(), url2);
+  FrameTreeNode* root =
+      static_cast<WebContentsImpl*>(shell()->web_contents())->
+          GetFrameTree()->root();
+
+  FrameNavigationEntry* frame_entry =
+      controller.GetLastCommittedEntry()->root_node()->frame_entry.get();
+  int64 isn_1 = frame_entry->item_sequence_number();
+  int64 dsn_1 = frame_entry->document_sequence_number();
+  EXPECT_NE(-1, isn_1);
+  EXPECT_NE(-1, dsn_1);

[Avi (use Gerrit), 2015/06/18 00:57:00]

-1? I would have thought that would be a "not assigned" value, not one you'd see
for a committed load.

[Charlie Reis, 2015/06/18 20:30:17]

Right: EXPECT_NE.

+
+  // 2. Do an in-page fragment navigation.
+  std::string script = "document.getElementById('fraglink').click()";
+  EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+
+  frame_entry =
+      controller.GetLastCommittedEntry()->root_node()->frame_entry.get();
+  int64 isn_2 = frame_entry->item_sequence_number();
+  int64 dsn_2 = frame_entry->document_sequence_number();
+  EXPECT_NE(-1, isn_2);
+  EXPECT_NE(isn_1, isn_2);
+  EXPECT_EQ(dsn_1, dsn_2);

[Avi (use Gerrit), 2015/06/18 00:57:00]

So document sequence numbers are sometimes shared across navigation entries. Is
there a design document of some kind explaining what the larger picture here is?

[Charlie Reis, 2015/06/18 20:30:17]

It's from Blink, so I assume the answer is no.  Though actually, I'm impressed
that these values are documented in HistoryItem.h!
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

I've added similar documentation in FrameNavigationEntry (on the accessors) and
frame_navigate_params.h.

+
+  // TODO(creis): Add a subframe test.

[Charlie Reis, 2015/06/17 22:56:23]

I'll add this as well.

(I was debating checking these in the existing NavigationTypeClassification
tests since those are pretty exhaustive on the types of navigations, but it also
felt like it would pollute them with checks unrelated to the classifier.  Not
sure what the best practice here should be for features that should be tested
across all types of navigations.)

[Charlie Reis, 2015/06/18 20:30:17]

Done.

+}
+
 namespace {
 
 class HttpThrottle : public ResourceThrottle {