Returning early when setting a frame name to the same name.

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

Index: third_party/WebKit/Source/core/page/FrameTree.cpp
diff --git a/third_party/WebKit/Source/core/page/FrameTree.cpp b/third_party/WebKit/Source/core/page/FrameTree.cpp
index 827cbb629e79a1ef1051c50fd41c00950a8d6f15..58960f49e4cc02f63baddc753ceb4480bd7b875f 100644
--- a/third_party/WebKit/Source/core/page/FrameTree.cpp
+++ b/third_party/WebKit/Source/core/page/FrameTree.cpp
@@ -54,6 +54,14 @@ FrameTree::~FrameTree()
 
 void FrameTree::setName(const AtomicString& name)
 {
+    // Do not recalculate m_uniqueName if there is no real change of m_name
+    // *and* m_uniqueName has been already calculated.  This ensures that
+    // m_uniqueName stays the same in this scenario (regenerating m_uniqueName
+    // can result in a different unique name value, because it depends on the
+    // state of other frames in the frame tree).
+    if (m_name == name && !m_uniqueName.isNull())

[Łukasz Anforowicz, 2016/06/10 00:06:57]

This would be really easy to understand if this could just say |m_name == name|.
 I think this is not sufficient, because FrameTree constructor constructs a
half-baked object - it assumes that callers will eventually call setName or
setPrecalculatedName.  In such cases, we still want to generate a fresh
m_uniqueName (even if m_name == name - e.g. if name == null).

I've convinced myself that the condition above correctly handles the case of
m_uniqueName.isNull().  For non-main-frame, m_uniqueName can be null only if
unique name has not yet been initialized.  For main frame we allow a null or
empty m_uniqueName, so m_uniqueName can be null if 1) old frame name was null 
or 2) unique name has not yet been initialized.  For (2) we obviously want to
generate a unique case, but generating for (1) is also okay - we will always
generate null unique-name if main frame's assigned-name is null.

I don't see any better solution:

- We could distinguish between m_uniqueName being null and empty.  I don't think
this is any less complex.

- We could audit all calls to FrameTree::setName and check in what conditions
the call is made (only made for main frame?  only made right after creating the
frame?), but this seems fragile / unmaintainable.

[dcheng, 2016/06/10 23:51:14]

As we discussed, let's try to elaborate this comment a little more on why this
happens: I guess this is happening when creating a subframe with no default
name? But we should clarify that. Where is setName() getting called in that init
path? etc.

[Łukasz Anforowicz, 2016/06/14 23:58:42]

Unless there is an assert, as in patchset #3?  Hmmm....
I've expanded the comments, but when researching this, I've found out that
setName should never be called on an uninitialized FrameTree that belongs to a
subframe.  So - in the end, I decided to simplify the condition above by
replacing the second part with an assert.

Expanded comments: patchset #2
Simplified condition: patchset #3

+        return;
+
     m_name = name;
 
     // Remove our old frame name so it's not considered in calculateUniqueNameForChildFrame