Add item and document sequence numbers to FrameNavigationEntry.

content/browser/frame_host/navigation_controller_impl_browsertest.cc

 // Copyright 2014 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 "base/bind.h"
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
@@ skipping 1530 matching lines @@
     EXPECT_EQ(
         " Site A ------------ proxies for B\n"
         "   |--Site A ------- proxies for B\n"
         "   +--Site B ------- proxies for A\n"
         "Where A = http://127.0.0.1/\n"
         "      B = http://baz.com/",
         visualizer.DepictFrameTree(root));
   }
 }
 
+// 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 {
  public:
   // ResourceThrottle
   void WillStartRequest(bool* defer) override {
     *defer = true;
   }
 
   const char* GetNameForLogging() const override {
@@ skipping 334 matching lines @@
     // tricky.
     EXPECT_EQ(nullptr, controller.GetPendingEntry());
     shell()->web_contents()->Stop();
     watcher.Wait();
   }
 
   ResourceDispatcherHost::Get()->SetDelegate(nullptr);
 }
 
 }  // namespace content