OOPIF: Fix subframe back/forward after recreating FTNs.

content/browser/frame_host/navigation_entry_impl.h

 // Copyright 2013 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_
 
 #include <stdint.h>
 
 #include "base/macros.h"
@@ skipping 19 matching lines @@
 class CONTENT_EXPORT NavigationEntryImpl
     : public NON_EXPORTED_BASE(NavigationEntry) {
  public:
   // Represents a tree of FrameNavigationEntries that make up this joint session
   // history item.  The tree currently only tracks the main frame by default,
   // and is populated with subframe nodes in --site-per-process mode.
   struct TreeNode {
     TreeNode(FrameNavigationEntry* frame_entry);
     ~TreeNode();
 
-    // Returns whether this TreeNode corresponds to |frame_tree_node|.
-    bool MatchesFrame(FrameTreeNode* frame_tree_node) const;
+    // Returns whether this TreeNode corresponds to |frame_tree_node|, based on
+    // the FrameTreeNode ID or unique name.  If the unique name matches and
+    // FrameTreeNode ID does not, then this will update the entry's
+    // FrameTreeNode ID (which may be stale if the node was recreated).
+    bool MatchesFrame(FrameTreeNode* frame_tree_node);

[alexmos, 2016/04/06 17:14:22]

This now has the side effect of sometimes updating FTN ID, but the function name
doesn't reflect that, so you don't realize this looking at the call sites.  I
couldn't come up with anything better to suggest though, so I think this is
fine. :)

[Charlie Reis, 2016/04/12 16:25:51]

Yeah, it's a bit awkward for the name to reflect that.  I view it as hopefully
temporary, since we should be able to remove FTN ID entirely from here and just
use unique name (maybe once the browser process generates unique name).  That's
what HistoryController does in the old path.

 
     // Recursively makes a deep copy of TreeNode with copies of each of the
     // FrameNavigationEntries in the subtree.  Replaces the TreeNode
     // corresponding to |frame_tree_node| (and all of its children) with a new
     // TreeNode for |frame_navigation_entry|.  Pass nullptr for both parameters
     // to make a complete clone.
     // TODO(creis): For --site-per-process, share FrameNavigationEntries between
     // NavigationEntries of the same tab.
     scoped_ptr<TreeNode> CloneAndReplace(
         FrameTreeNode* frame_tree_node,
-        FrameNavigationEntry* frame_navigation_entry) const;
+        FrameNavigationEntry* frame_navigation_entry);
 
     // Ref counted pointer that keeps the FrameNavigationEntry alive as long as
     // it is needed by this node's NavigationEntry.
     scoped_refptr<FrameNavigationEntry> frame_entry;
 
     // List of child TreeNodes, which will be deleted when this node is.
     ScopedVector<TreeNode> children;
   };
 
   static NavigationEntryImpl* FromNavigationEntry(NavigationEntry* entry);
@@ skipping 118 matching lines @@
     return frame_tree_.get();
   }
 
   // Finds the TreeNode associated with |frame_tree_node_id| to add or update
   // its FrameNavigationEntry.  A new FrameNavigationEntry is added if none
   // exists, or else the existing one (which might be shared with other
   // NavigationEntries) is updated with the given parameters.
   // 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_t item_sequence_number,
                              int64_t document_sequence_number,
                              SiteInstanceImpl* site_instance,
                              const GURL& url,
                              const Referrer& referrer,
                              const PageState& page_state);
 
   // Returns the FrameNavigationEntry corresponding to |frame_tree_node|, if
-  // there is one in this NavigationEntry.
-  FrameNavigationEntry* GetFrameEntry(FrameTreeNode* frame_tree_node) const;
-
-  // Returns the FrameNavigationEntry corresponding to the frame with the given
-  // |unique_name|, if any. This is useful when the FrameTreeNode cannot be used
-  // to find the entry, such as for a newly created subframe in a history
-  // navigation. Callers should update the FrameTreeNode ID of the entry so that
-  // it can be found with |GetFrameEntry| above.
-  // TODO(creis): Generate or verify the unique_name in the browser process.
-  FrameNavigationEntry* GetFrameEntryByUniqueName(
-      const std::string& unique_name) const;
+  // there is one in this NavigationEntry. This compares against the
+  // FrameTreeNode ID first, and if that doesn't match, then the unique_name.
+  // It will update the FrameNavigationEntry's FrameTreeNode ID if it is stale.
+  FrameNavigationEntry* GetFrameEntry(FrameTreeNode* frame_tree_node);
 
   void set_unique_id(int unique_id) {
     unique_id_ = unique_id;
   }
 
   // The SiteInstance represents which pages must share processes. This is a
   // reference counted pointer to a shared SiteInstance.
   //
   // Note that the SiteInstance should usually not be changed after it is set,
   // but this may happen if the NavigationEntry was cloned and needs to use a
@@ skipping 136 matching lines @@
     return has_user_gesture_;
   }
 
   void set_has_user_gesture (bool has_user_gesture) {
     has_user_gesture_ = has_user_gesture;
   }
 #endif
 
  private:
   // Finds the TreeNode associated with |frame_tree_node|, if any.
-  NavigationEntryImpl::TreeNode* FindFrameEntry(
-      FrameTreeNode* frame_tree_node) const;
+  NavigationEntryImpl::TreeNode* FindFrameEntry(FrameTreeNode* frame_tree_node);
 
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
   // Session/Tab restore save portions of this class so that it can be recreated
   // later. If you add a new field that needs to be persisted you'll have to
   // update SessionService/TabRestoreService and Android WebView
   // state_serializer.cc appropriately.
   // For all new fields, update |Clone| and possibly |ResetForCommit|.
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
 
   // Tree of FrameNavigationEntries, one for each frame on the page.
@@ skipping 123 matching lines @@
   // persisted, unless specific data is taken out/put back in at save/restore
   // time (see TabNavigation for an example of this).
   std::map<std::string, base::string16> extra_data_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationEntryImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_