Try harder to make sure that blink::FrameTree::m_uniqueName is truly unique.

third_party/WebKit/Source/core/page/FrameTree.h

 /*
  * Copyright (C) 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 19 matching lines @@
 class TreeScope;
 
 class CORE_EXPORT FrameTree final {
     WTF_MAKE_NONCOPYABLE(FrameTree);
     DISALLOW_NEW();
 public:
     explicit FrameTree(Frame* thisFrame);
     ~FrameTree();
 
     const AtomicString& name() const { return m_name; }
+    void setName(const AtomicString&);
+
+    // Unique name of a frame (unique per page).  Mainly used to identify the
+    // frame for session history purposes, but also used in expected results
+    // of layout tests.
+    //
+    // The value should be treated (at least outside of FrameTree.cpp) as an

[dcheng, 2016/05/31 19:57:47]

I would just condense this down to "The value should be treated as an
unstructured, opaque string."

[Łukasz Anforowicz, 2016/05/31 23:50:20]

Done.  Let me know if you think I should also remove pointer to the comment
describing internal implementation details (i.e. remove the last 2 lines of the
comment here).

[dcheng, 2016/06/01 20:58:11]

I think it makes sense to remove that part of the comment as well (it's
generally implied)

[Łukasz Anforowicz, 2016/06/01 22:31:28]

Done.

+    // unstructured, opaque string.  The implementation details, including
+    // the format description can be found in FrameTree.cpp, in a comment
+    // inside calculateUniqueNameForChildFrame method.
     const AtomicString& uniqueName() const { return m_uniqueName; }
-    // If |name| is not empty, |fallbackName| is ignored. Otherwise,
-    // |fallbackName| is used as a source of uniqueName.
-    void setName(const AtomicString& name, const AtomicString& fallbackName = nullAtom);
 
     // Directly assigns both the name and uniqueName.  Can be used when
     // |uniqueName| is already known (i.e. when it has been precalculated by
     // calculateUniqueNameForNewChildFrame OR when replicating the name between
     // LocalFrames and RemoteFrames for the same logical frame).
     void setPrecalculatedName(const AtomicString& name, const AtomicString& uniqueName);
 
     Frame* parent() const;
     Frame* top() const;
     Frame* previousSibling() const;
@@ skipping 16 matching lines @@
     void invalidateScopedChildCount();
 
     DECLARE_TRACE();
 
     AtomicString calculateUniqueNameForNewChildFrame(
         const AtomicString& name,
         const AtomicString& fallbackName = nullAtom) const;
 
 private:
     Frame* deepLastChild() const;
+
+    // Returns true if one of frames in the tree already has unique name equal
+    // to |uniqueNameCandidate|.
+    bool uniqueNameExists(const AtomicString& uniqueNameCandidate) const;
+
+    // Generates a hopefully-but-not-necessarily unique name based on frame's
+    // relative position in the tree and on unique names of ancestors.
+    AtomicString generateOldStyleMaybeUniqueName(bool existingChildFrame) const;

[dcheng, 2016/05/31 19:57:47]

Nit: generateUniqueNameCandidate?

[Łukasz Anforowicz, 2016/05/31 23:50:20]

Done.

+
+    // Generates a hopefully-but-not-necessarily unique suffix based on |child|
+    // absolute position in the tree.  If |child| is nullptr, calculations are
+    // made for a position that a new child of |this| would have.
+    String generateLikelyUniqueSuffix(Frame* child) const;

[dcheng, 2016/05/31 19:57:47]

Nit: generateFramePosition?

[Łukasz Anforowicz, 2016/05/31 23:50:20]

Done.

+
+    // Returns a unique name created by appending a suffix to |notYetUniqueName|.
+    // The appended suffix is based on |likelyUniqueSuffix|, but can also
+    // be extended to fulfill the uniqueness requirement.
+    AtomicString ensureUniquenessOfUniqueName(

[dcheng, 2016/05/31 19:57:47]

Nit: generateUniqueNameFromCandidate

[Łukasz Anforowicz, 2016/05/31 23:50:20]

How about
    // Concatenates |prefix|, |likelyUniqueSuffix| (and additional, internally
    // generated suffix) until the result is a unique name, that doesn't exist
    // elsewhere in the frame tree.  Returns the unique name built in this way.
    AtomicString appendUniqueSuffix(
        const AtomicString& prefix,
        const String& likelyUniqueSuffix) const;
?

+        const AtomicString& notYetUniqueName,

[dcheng, 2016/05/31 19:57:47]

Nit: candidate

[Łukasz Anforowicz, 2016/05/31 23:50:20]

How about |prefix| (we use |uniqueNameCandidate| as a variable name inside the
method's implementation, so |candidate| is a bit confusing).

+        const String& likelyUniqueSuffix) const;
+
+    // Calculates a unique name for |child| frame (which might be nullptr if the
+    // child has not yet been created - i.e. when we need unique name for a new
+    // frame).  Tries to use the |assignedName| or |fallbackName| if possible,
+    // otherwise falls back to generating a deterministic,
+    // stable-across-page-reloads string based on |child| position in the tree.
     AtomicString calculateUniqueNameForChildFrame(
-        bool existingChildFrame,
-        const AtomicString& name,
+        Frame* child,
+        const AtomicString& assignedName,
         const AtomicString& fallbackName = nullAtom) const;
-    bool uniqueNameExists(const AtomicString& name) const;
+
+    // Sets |m_uniqueName| and asserts its uniqueness.
+    void setUniqueName(const AtomicString&);
 
     Member<Frame> m_thisFrame;
 
     AtomicString m_name; // The actual frame name (may be empty).
     AtomicString m_uniqueName;
 
     mutable unsigned m_scopedChildCount;
 };
 
 } // namespace blink
 
 #ifndef NDEBUG
 // Outside the WebCore namespace for ease of invocation from gdb.
 void showFrameTree(const blink::Frame*);
 #endif
 
 #endif // FrameTree_h