OOPIF: Add frame_unique_name to FrameNavigationEntry.

This holds the unique name of the frame in which the navigation took
place.  This is stable after back/forward and restore, whereas the
FrameTreeNode ID will change over time.

BUG=236848, 502317
TEST=No behavior change yet.

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

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

9 files changed, 149 insertions, 32 deletions

diff --git a/content/browser/frame_host/frame_navigation_entry.cc b/content/browser/frame_host/frame_navigation_entry.cc
index bb96050..74d9a39 100644
--- a/content/browser/frame_host/frame_navigation_entry.cc
+++ b/content/browser/frame_host/frame_navigation_entry.cc
@@ -13,12 +13,14 @@ FrameNavigationEntry::FrameNavigationEntry(int frame_tree_node_id)
 }
 
 FrameNavigationEntry::FrameNavigationEntry(int frame_tree_node_id,
+                                           const std::string& frame_unique_name,
                                            int64 item_sequence_number,
                                            int64 document_sequence_number,
                                            SiteInstanceImpl* site_instance,
                                            const GURL& url,
                                            const Referrer& referrer)
     : frame_tree_node_id_(frame_tree_node_id),
+      frame_unique_name_(frame_unique_name),
       item_sequence_number_(item_sequence_number),
       document_sequence_number_(document_sequence_number),
       site_instance_(site_instance),
@@ -31,17 +33,20 @@ FrameNavigationEntry::~FrameNavigationEntry() {
 
 FrameNavigationEntry* FrameNavigationEntry::Clone() const {
   FrameNavigationEntry* copy = new FrameNavigationEntry(frame_tree_node_id_);
-  copy->UpdateEntry(item_sequence_number_, document_sequence_number_,
-                    site_instance_.get(), url_, referrer_, page_state_);
+  copy->UpdateEntry(frame_unique_name_, item_sequence_number_,
+                    document_sequence_number_, site_instance_.get(), url_,
+                    referrer_, page_state_);
   return copy;
 }
 
-void FrameNavigationEntry::UpdateEntry(int64 item_sequence_number,
+void FrameNavigationEntry::UpdateEntry(const std::string& frame_unique_name,
+                                       int64 item_sequence_number,
                                        int64 document_sequence_number,
                                        SiteInstanceImpl* site_instance,
                                        const GURL& url,
                                        const Referrer& referrer,
                                        const PageState& page_state) {
+  frame_unique_name_ = frame_unique_name;
   item_sequence_number_ = item_sequence_number;
   document_sequence_number_ = document_sequence_number;
   site_instance_ = site_instance;
diff --git a/content/browser/frame_host/frame_navigation_entry.h b/content/browser/frame_host/frame_navigation_entry.h
index d92a924..fcbdb04 100644
--- a/content/browser/frame_host/frame_navigation_entry.h
+++ b/content/browser/frame_host/frame_navigation_entry.h
@@ -19,17 +19,16 @@ namespace content {
 // For now, it is owned by a single NavigationEntry and only tracks the main
 // frame.
 //
-// TODO(creis): In --site-per-process, fill in a tree of FrameNavigationEntries
-// in each NavigationEntry, one per frame.  FrameNavigationEntries may be shared
-// across NavigationEntries if the frame hasn't changed.
+// If SiteIsolationPolicy::UseSubframeNavigationEntries is true, there will be a
+// tree of FrameNavigationEntries in each NavigationEntry, one per frame.
+// TODO(creis): Share these FrameNavigationEntries across NavigationEntries if
+// the frame hasn't changed.
 class CONTENT_EXPORT FrameNavigationEntry
     : public base::RefCounted<FrameNavigationEntry> {
  public:
-  // TODO(creis): We should not use FTN IDs here, since they will change if you
-  // leave a page and come back later.  We should evaluate whether Blink's
-  // unique names would work instead, similar to HistoryNode.
   explicit FrameNavigationEntry(int frame_tree_node_id);
   FrameNavigationEntry(int frame_tree_node_id,
+                       const std::string& frame_unique_name,
                        int64 item_sequence_number,
                        int64 document_sequence_number,
                        SiteInstanceImpl* site_instance,
@@ -41,7 +40,8 @@ class CONTENT_EXPORT FrameNavigationEntry
   FrameNavigationEntry* Clone() const;
 
   // Updates all the members of this entry.
-  void UpdateEntry(int64 item_sequence_number,
+  void UpdateEntry(const std::string& frame_unique_name,
+                   int64 item_sequence_number,
                    int64 document_sequence_number,
                    SiteInstanceImpl* site_instance,
                    const GURL& url,
@@ -51,9 +51,23 @@ class CONTENT_EXPORT FrameNavigationEntry
   // The ID of the FrameTreeNode this entry is for.  -1 for the main frame,
   // since we don't always know the FrameTreeNode ID when creating the overall
   // NavigationEntry.
-  // TODO(creis): Replace with frame sequence number or unique name.
+  // TODO(creis): Consider removing |frame_tree_node_id| in favor of
+  // |frame_unique_name|, if we can move unique name computation to the browser
+  // process.
   int frame_tree_node_id() const { return frame_tree_node_id_; }
 
+  // The unique name of the frame this entry is for.  This is a stable name for
+  // the frame based on its position in the tree and relation to other named
+  // frames, which does not change after cross-process navigations or restores.
+  // Only the main frame can have an empty name.
+  //
+  // This is unique relative to other frames in the same page, but not among
+  // other pages (i.e., not globally unique).
+  const std::string& frame_unique_name() const { return frame_unique_name_; }
+  void set_frame_unique_name(const std::string& frame_unique_name) {
+    frame_unique_name_ = frame_unique_name;
+  }
+
   // Keeps track of where this entry belongs in the frame's session history.
   // The item sequence number identifies each stop in the back/forward history
   // and is globally unique.  The document sequence number increments for each
@@ -91,13 +105,14 @@ class CONTENT_EXPORT FrameNavigationEntry
   virtual ~FrameNavigationEntry();
 
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
-  // For all new fields, update |Clone|.
+  // Add all new fields to |UpdateEntry|.
   // TODO(creis): These fields have implications for session restore.  This is
   // currently managed by NavigationEntry, but the logic will move here.
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
 
   // See the accessors above for descriptions.
   int frame_tree_node_id_;
+  std::string frame_unique_name_;
   int64 item_sequence_number_;
   int64 document_sequence_number_;
   scoped_refptr<SiteInstanceImpl> site_instance_;
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 6ceff9a..87abff0 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -748,7 +748,7 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
       if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
         entry = GetLastCommittedEntry()->Clone();
         entry->SetPageID(-1);
-        entry->AddOrUpdateFrameEntry(node, -1, -1, nullptr, params.url,
+        entry->AddOrUpdateFrameEntry(node, "", -1, -1, nullptr, params.url,
                                      params.referrer, PageState());
       }
     }
@@ -1124,6 +1124,7 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
   // Update the FrameNavigationEntry for new main frame commits.
   FrameNavigationEntry* frame_entry =
       new_entry->GetFrameEntry(rfh->frame_tree_node());
+  frame_entry->set_frame_unique_name(params.frame_unique_name);
   frame_entry->set_item_sequence_number(params.item_sequence_number);
   frame_entry->set_document_sequence_number(params.document_sequence_number);
 
@@ -1259,7 +1260,7 @@ void NavigationControllerImpl::RendererDidNavigateNewSubframe(
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
     // Make sure new_entry takes ownership of frame_entry in a scoped_refptr.
     FrameNavigationEntry* frame_entry = new FrameNavigationEntry(
-        rfh->frame_tree_node()->frame_tree_node_id(),
+        rfh->frame_tree_node()->frame_tree_node_id(), params.frame_unique_name,
         params.item_sequence_number, params.document_sequence_number,
         rfh->GetSiteInstance(), params.url, params.referrer);
     new_entry = GetLastCommittedEntry()->CloneAndReplace(rfh->frame_tree_node(),
@@ -1320,9 +1321,9 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe(
     // it may be a "history auto" case where we update an existing one.
     NavigationEntryImpl* last_committed = GetLastCommittedEntry();
     last_committed->AddOrUpdateFrameEntry(
-        rfh->frame_tree_node(), params.item_sequence_number,
-        params.document_sequence_number, rfh->GetSiteInstance(), params.url,
-        params.referrer, params.page_state);
+        rfh->frame_tree_node(), params.frame_unique_name,
+        params.item_sequence_number, params.document_sequence_number,
+        rfh->GetSiteInstance(), params.url, params.referrer, params.page_state);
 
     // Cross-process subframe navigations may leave a pending entry around.
     // Clear it if it's actually for the subframe.
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index fe2616c..373f010 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -1904,6 +1904,94 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   }
 }
 
+// Verifies that the |frame_unique_name| is set to the correct frame, so that we
+// can match subframe FrameNavigationEntries to newly created frames after
+// back/forward and restore.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       FrameNavigationEntry_FrameUniqueName) {
+  const NavigationControllerImpl& controller =
+      static_cast<const NavigationControllerImpl&>(
+          shell()->web_contents()->GetController());
+
+  // 1. Navigate the main frame.
+  GURL url(embedded_test_server()->GetURL(
+      "/navigation_controller/page_with_links.html"));
+  NavigateToURL(shell(), url);
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  SiteInstance* main_site_instance =
+      root->current_frame_host()->GetSiteInstance();
+
+  // The main frame defaults to an empty name.
+  FrameNavigationEntry* frame_entry =
+      controller.GetLastCommittedEntry()->GetFrameEntry(root);
+  EXPECT_EQ("", frame_entry->frame_unique_name());
+
+  // Test subframe unique names only if enabled, e.g. in --site-per-process.
+  if (!SiteIsolationPolicy::UseSubframeNavigationEntries())
+    return;
+
+  // 2. Add an unnamed subframe, which does an AUTO_SUBFRAME navigation.
+  {
+    LoadCommittedCapturer capturer(shell()->web_contents());
+    std::string script = "var iframe = document.createElement('iframe');"
+                         "iframe.src = '" + url.spec() + "';"
+                         "document.body.appendChild(iframe);";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+  }
+
+  // The root FrameNavigationEntry hasn't changed.
+  EXPECT_EQ(frame_entry,
+            controller.GetLastCommittedEntry()->GetFrameEntry(root));
+
+  // The subframe should have a generated name.
+  FrameTreeNode* subframe = root->child_at(0);
+  EXPECT_EQ(main_site_instance,
+            subframe->current_frame_host()->GetSiteInstance());
+  FrameNavigationEntry* subframe_entry =
+      controller.GetLastCommittedEntry()->GetFrameEntry(subframe);
+  std::string unnamed_subframe_name = "<!--framePath //<!--frame0-->-->";
+  EXPECT_EQ(unnamed_subframe_name, subframe_entry->frame_unique_name());
+
+  // 3. Add a named subframe.
+  {
+    LoadCommittedCapturer capturer(shell()->web_contents());
+    std::string script = "var iframe = document.createElement('iframe');"
+                         "iframe.src = '" + url.spec() + "';"
+                         "iframe.name = 'foo';"
+                         "document.body.appendChild(iframe);";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+  }
+
+  // The new subframe should have the specified name.
+  EXPECT_EQ(frame_entry,
+            controller.GetLastCommittedEntry()->GetFrameEntry(root));
+  FrameTreeNode* foo_subframe = root->child_at(1);
+  EXPECT_EQ(main_site_instance,
+            foo_subframe->current_frame_host()->GetSiteInstance());
+  FrameNavigationEntry* foo_subframe_entry =
+      controller.GetLastCommittedEntry()->GetFrameEntry(foo_subframe);
+  std::string named_subframe_name = "foo";
+  EXPECT_EQ(named_subframe_name, foo_subframe_entry->frame_unique_name());
+
+  // 4. Navigating in the subframes cross-process shouldn't change their names.
+  // TODO(creis): Fix the unnamed case in https://crbug.com/502317.
+  GURL bar_url(embedded_test_server()->GetURL(
+      "bar.com", "/navigation_controller/simple_page_1.html"));
+  NavigateFrameToURL(foo_subframe, bar_url);
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_NE(main_site_instance,
+            foo_subframe->current_frame_host()->GetSiteInstance());
+  foo_subframe_entry =
+      controller.GetLastCommittedEntry()->GetFrameEntry(foo_subframe);
+  EXPECT_EQ(named_subframe_name, foo_subframe_entry->frame_unique_name());
+}
+
 // Verifies that item sequence numbers and document sequence numbers update
 // properly for main frames and subframes.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
@@ -1946,10 +2034,10 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   // 3. Add a subframe, which does an AUTO_SUBFRAME navigation.
   {
     LoadCommittedCapturer capturer(shell()->web_contents());
-    std::string script = "var iframe = document.createElement('iframe');"
-                         "iframe.src = '" + url.spec() + "';"
-                         "document.body.appendChild(iframe);";
-    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    std::string add_script = "var iframe = document.createElement('iframe');"
+                             "iframe.src = '" + url.spec() + "';"
+                             "document.body.appendChild(iframe);";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), add_script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
   }
diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc
index a1f47a9..8102a47 100644
--- a/content/browser/frame_host/navigation_entry_impl.cc
+++ b/content/browser/frame_host/navigation_entry_impl.cc
@@ -102,7 +102,7 @@ NavigationEntryImpl::NavigationEntryImpl(SiteInstanceImpl* instance,
                                          ui::PageTransition transition_type,
                                          bool is_renderer_initiated)
     : frame_tree_(new TreeNode(
-          new FrameNavigationEntry(-1, -1, -1, instance, url, referrer))),
+          new FrameNavigationEntry(-1, "", -1, -1, instance, url, referrer))),
       unique_id_(GetUniqueIDInConstructor()),
       bindings_(kInvalidBindings),
       page_type_(PAGE_TYPE_NORMAL),
@@ -547,13 +547,15 @@ void NavigationEntryImpl::ResetForCommit() {
 #endif
 }
 
-void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
-                                                int64 item_sequence_number,
-                                                int64 document_sequence_number,
-                                                SiteInstanceImpl* site_instance,
-                                                const GURL& url,
-                                                const Referrer& referrer,
-                                                const PageState& page_state) {
+void NavigationEntryImpl::AddOrUpdateFrameEntry(
+    FrameTreeNode* frame_tree_node,
+    const std::string& frame_unique_name,
+    int64 item_sequence_number,
+    int64 document_sequence_number,
+    SiteInstanceImpl* site_instance,
+    const GURL& url,
+    const Referrer& referrer,
+    const PageState& page_state) {
   // We should already have a TreeNode for the parent node by the time this node
   // commits.  Find it first.
   DCHECK(frame_tree_node->parent());
@@ -570,7 +572,7 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   for (TreeNode* child : parent_node->children) {
     if (child->frame_entry->frame_tree_node_id() == frame_tree_node_id) {
       // Update the existing FrameNavigationEntry (e.g., for replaceState).
-      child->frame_entry->UpdateEntry(item_sequence_number,
+      child->frame_entry->UpdateEntry(frame_unique_name, item_sequence_number,
                                       document_sequence_number, site_instance,
                                       url, referrer, page_state);
       return;
@@ -581,8 +583,8 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   // Unordered list, since we expect to look up entries by frame sequence number
   // or unique name.
   FrameNavigationEntry* frame_entry = new FrameNavigationEntry(
-      frame_tree_node_id, item_sequence_number, document_sequence_number,
-      site_instance, url, referrer);
+      frame_tree_node_id, frame_unique_name, item_sequence_number,
+      document_sequence_number, site_instance, url, referrer);
   frame_entry->set_page_state(page_state);
   parent_node->children.push_back(
       new NavigationEntryImpl::TreeNode(frame_entry));
diff --git a/content/browser/frame_host/navigation_entry_impl.h b/content/browser/frame_host/navigation_entry_impl.h
index 7eb9a4c..051a3ba 100644
--- a/content/browser/frame_host/navigation_entry_impl.h
+++ b/content/browser/frame_host/navigation_entry_impl.h
@@ -180,6 +180,7 @@ class CONTENT_EXPORT NavigationEntryImpl
   // Does nothing if there is no entry already and |url| is about:blank, since
   // that does not count as a real commit.
   void AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
+                             const std::string& frame_unique_name,
                              int64 item_sequence_number,
                              int64 document_sequence_number,
                              SiteInstanceImpl* site_instance,
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h
index 0f8efbf..945d358 100644
--- a/content/common/frame_messages.h
+++ b/content/common/frame_messages.h
@@ -154,6 +154,7 @@ IPC_STRUCT_END()
 IPC_STRUCT_TRAITS_BEGIN(content::FrameNavigateParams)
   IPC_STRUCT_TRAITS_MEMBER(page_id)
   IPC_STRUCT_TRAITS_MEMBER(nav_entry_id)
+  IPC_STRUCT_TRAITS_MEMBER(frame_unique_name)
   IPC_STRUCT_TRAITS_MEMBER(item_sequence_number)
   IPC_STRUCT_TRAITS_MEMBER(document_sequence_number)
   IPC_STRUCT_TRAITS_MEMBER(url)
diff --git a/content/public/common/frame_navigate_params.h b/content/public/common/frame_navigate_params.h
index e3b9334..9e006c0 100644
--- a/content/public/common/frame_navigate_params.h
+++ b/content/public/common/frame_navigate_params.h
@@ -33,6 +33,9 @@ struct CONTENT_EXPORT FrameNavigateParams {
   // means. If the navigation was renderer-initiated, this value is 0.
   int nav_entry_id;
 
+  // The unique name of the frame in which this navigation takes place.
+  std::string frame_unique_name;
+
   // The item sequence number identifies each stop in the session history.  It
   // is unique within the renderer process and makes a best effort to be unique
   // across browser sessions (using a renderer process timestamp).
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index b80ddeb..0d2acf7 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -4167,6 +4167,7 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
     params.page_state = SingleHistoryItemToPageState(item);
     post_id = ExtractPostId(item);
   }
+  params.frame_unique_name = item.target().utf8();
   params.item_sequence_number = item.itemSequenceNumber();
   params.document_sequence_number = item.documentSequenceNumber();