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

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

 /*
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Library General Public License for more details.
  *
  * You should have received a copy of the GNU Library General Public License
  * along with this library; see the file COPYING.LIB.  If not, write to
  * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  * Boston, MA 02110-1301, USA.
  */
 
 #include "core/page/FrameTree.h"
 
 #include "core/dom/Document.h"
 #include "core/frame/FrameClient.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/RemoteFrame.h"
 #include "core/frame/RemoteFrameView.h"
 #include "core/page/Page.h"
+#include "wtf/Assertions.h"
+#include "wtf/CryptographicallyRandomNumber.h"
 #include "wtf/Vector.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringBuilder.h"
 
 using std::swap;
 
 namespace blink {
 
 namespace {
 
 const unsigned invalidChildCount = ~0U;
 
 } // namespace
 
 FrameTree::FrameTree(Frame* thisFrame)
     : m_thisFrame(thisFrame)
     , m_scopedChildCount(invalidChildCount)
 {
 }
 
 FrameTree::~FrameTree()
 {
 }
 
 void FrameTree::setName(const AtomicString& name, const AtomicString& fallbackName)
 {
     m_name = name;
-    if (!parent()) {
-        m_uniqueName = name;
-        return;
-    }
 
-    // Remove our old frame name so it's not considered in calculateUniqueNameForChildFrame.
+    // Remove our old frame name so it's not considered in calculateUniqueNameForChildFrame
+    // and ensureUniquenessOfUniqueName calls below.
     m_uniqueName = AtomicString();
 
-    m_uniqueName = parent()->tree().calculateUniqueNameForChildFrame(true, name, fallbackName);
+    // Calculate a new unique name based on inputs.
+    if (parent()) {
+        setUniqueName(
+            parent()->tree().calculateUniqueNameForChildFrame(true, name, fallbackName));
+    } else if (name.isEmpty()) {
+        // Only main frame can have an empty unique name, so for main frames
+        // emptiness guarantees uniquness.
+        setUniqueName(name);
+    } else {
+        setUniqueName(ensureUniquenessOfUniqueName(name));
+    }
 }
 
 void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicString& uniqueName)
 {
     if (!parent()) {
         ASSERT(uniqueName == name);
     } else {
         ASSERT(!uniqueName.isEmpty());
     }
 
     m_name = name;
+
+    // TODO(lukasza): We would like to assert uniqueness below (i.e. by calling
+    // setUniqueName), but
+    // 1) uniqueness is currently violated by provisional/old frame pairs.
+    // 2) there is an unresolved race between 2 OOPIFs, that can result in a
+    //    non-unique |uniqueName| - see https://crbug.com/558680#c14.
     m_uniqueName = uniqueName;
 }
 
 Frame* FrameTree::parent() const
 {
     if (!m_thisFrame->client())
         return nullptr;
     return m_thisFrame->client()->parent();
 }
 
@@ skipping 31 matching lines @@
 
 Frame* FrameTree::lastChild() const
 {
     if (!m_thisFrame->client())
         return nullptr;
     return m_thisFrame->client()->lastChild();
 }
 
 bool FrameTree::uniqueNameExists(const AtomicString& name) const
 {
+    // Before recalculating or checking unique name, we set m_uniqueName
+    // to an empty string (so the soon-to-be-removed name does not count
+    // as a collision).  This means that uniqueNameExists would return
+    // false positives when called with an empty |name|.
+    ASSERT(!name.isEmpty());
+
     for (Frame* frame = top(); frame; frame = frame->tree().traverseNext()) {
         if (frame->tree().uniqueName() == name)
             return true;
     }
     return false;
 }
 
 AtomicString FrameTree::calculateUniqueNameForNewChildFrame(
     const AtomicString& name,
     const AtomicString& fallbackName) const
@@ skipping 38 matching lines @@
     for (int i = chain.size() - 1; i >= 0; --i) {
         frame = chain[i];
         uniqueName.append('/');
         uniqueName.append(frame->tree().uniqueName());
     }
 
     uniqueName.appendLiteral("/<!--frame");
     uniqueName.appendNumber(childCount() - (existingChildFrame ? 1 : 0));
     uniqueName.appendLiteral("-->-->");
 
-    return uniqueName.toAtomicString();
+    return ensureUniquenessOfUniqueName(uniqueName.toAtomicString());
+}
+
+AtomicString FrameTree::ensureUniquenessOfUniqueName(const AtomicString& potentiallyUniqueName) const
+{
+    if (!uniqueNameExists(potentiallyUniqueName))
+        return potentiallyUniqueName;
+
+    StringBuilder uniqueNameBuilder;
+    uniqueNameBuilder.append(potentiallyUniqueName);
+    do {
+        uniqueNameBuilder.append('-');
+        uniqueNameBuilder.appendNumber(cryptographicallyRandomNumber());
+    } while (uniqueNameExists(uniqueNameBuilder.toAtomicString()));

[dcheng, 2016/05/12 05:31:18]

So according to the spec, a valid name cannot start with a '_'. Obviously that's
not enforced in Blink today, but is it possible to set a browsing context name
to a name that starts with an underscore in other browsers?

I ask this because it seems like it'd be nicer to just put unique names like
this in a separate namespace.

Alternatively, is there any way to retroactively add namespaces to the unique
names so they can't collide?

e.g.

name:foobar (if specified by the page, and not autogenerated)
gen:<!--frame1//frame2-->--> (if generated)

+    return uniqueNameBuilder.toAtomicString();
+}
+
+void FrameTree::setUniqueName(const AtomicString& uniqueName)
+{
+    if (parent()) {
+        ASSERT(!uniqueName.isEmpty() && !uniqueNameExists(uniqueName));
+    } else {
+        ASSERT(uniqueName.isEmpty() || !uniqueNameExists(uniqueName));
+    }
+
+    m_uniqueName = uniqueName;
 }
 
 Frame* FrameTree::scopedChild(unsigned index) const
 {
     unsigned scopedIndex = 0;
     for (Frame* child = firstChild(); child; child = child->tree().nextSibling()) {
         if (child->client()->inShadowTree())
             continue;
         if (scopedIndex == index)
             return child;
@@ skipping 225 matching lines @@
 {
     if (!frame) {
         printf("Null input frame\n");
         return;
     }
 
     printFrames(frame->tree().top(), frame, 0);
 }
 
 #endif