Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Side by Side Diff: third_party/WebKit/Source/core/page/FrameTree.cpp

Issue 2317203002: Avoid mutating frame unique name after first real commit. (Closed)
Patch Set: Tweaking the comment describing the format of unique name. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (C) Research In Motion Limited 2010. All rights reserved. 2 * Copyright (C) Research In Motion Limited 2010. All rights reserved.
3 * Copyright (C) 2006 Apple Computer, Inc. 3 * Copyright (C) 2006 Apple Computer, Inc.
4 * 4 *
5 * This library is free software; you can redistribute it and/or 5 * This library is free software; you can redistribute it and/or
6 * modify it under the terms of the GNU Library General Public 6 * modify it under the terms of the GNU Library General Public
7 * License as published by the Free Software Foundation; either 7 * License as published by the Free Software Foundation; either
8 * version 2 of the License, or (at your option) any later version. 8 * version 2 of the License, or (at your option) any later version.
9 * 9 *
10 * This library is distributed in the hope that it will be useful, 10 * This library is distributed in the hope that it will be useful,
(...skipping 44 matching lines...) Expand 10 before | Expand all | Expand 10 after
55 void FrameTree::setName(const AtomicString& name) 55 void FrameTree::setName(const AtomicString& name)
56 { 56 {
57 // This method should only be called for local frames 57 // This method should only be called for local frames
58 // (remote frames should be updated via setPrecalculatedName). 58 // (remote frames should be updated via setPrecalculatedName).
59 DCHECK(m_thisFrame->isLocalFrame()); 59 DCHECK(m_thisFrame->isLocalFrame());
60 60
61 // When this method is called, m_uniqueName should be already initialized. 61 // When this method is called, m_uniqueName should be already initialized.
62 // This assert helps ensure that early return (a few lines below) won't 62 // This assert helps ensure that early return (a few lines below) won't
63 // result in an uninitialized m_uniqueName. 63 // result in an uninitialized m_uniqueName.
64 DCHECK(!m_uniqueName.isNull() 64 DCHECK(!m_uniqueName.isNull()
65 || (m_uniqueName.isNull() && m_name.isNull() && !parent())); 65 || (m_uniqueName.isNull() && !parent()));
66 66
67 // Do not recalculate m_uniqueName if there is no real change of m_name. 67 // Do not recalculate m_uniqueName if there is no real change of m_name.
68 // This is not just a performance optimization - other code relies on the 68 // This is not just a performance optimization - other code relies on the
69 // assumption that unique name shouldn't change if the assigned name didn't 69 // assumption that unique name shouldn't change if the assigned name didn't
70 // change (i.e. code in content::FrameTreeNode::SetFrameName). 70 // change (i.e. code in content::FrameTreeNode::SetFrameName).
71 if (m_name == name) 71 if (m_name == name)
72 return; 72 return;
73 73
74 m_name = name; 74 m_name = name;
75 75
76 // https://crbug.com/607205: Make sure m_uniqueName doesn't change after
77 // initial navigation - session history depends on this.
78 if (toLocalFrame(m_thisFrame)->loader().stateMachine()->committedFirstRealDo cumentLoad())
79 return;
80
76 // Remove our old frame name so it's not considered in calculateUniqueNameFo rChildFrame 81 // Remove our old frame name so it's not considered in calculateUniqueNameFo rChildFrame
77 // and appendUniqueSuffix calls below. 82 // and appendUniqueSuffix calls below.
78 m_uniqueName = AtomicString(); 83 m_uniqueName = AtomicString();
79 84
80 // Calculate a new unique name based on inputs. 85 // Calculate a new unique name based on inputs.
81 if (parent()) { 86 if (parent()) {
82 setUniqueName( 87 setUniqueName(
83 parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nullAtom)); 88 parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nullAtom));
84 } else if (name.isEmpty() || !uniqueNameExists(name)) { 89 } else if (name.isEmpty() || !uniqueNameExists(name)) {
85 // Only main frame can have an empty unique name, so for main frames 90 // Only main frame can have an empty unique name, so for main frames
86 // emptiness guarantees uniquness. 91 // emptiness guarantees uniquness.
87 setUniqueName(name); 92 setUniqueName(name);
88 } else { 93 } else {
89 setUniqueName(appendUniqueSuffix(name, "<!--framePosition")); 94 setUniqueName(appendUniqueSuffix(name, "<!--framePosition"));
90 } 95 }
91 } 96 }
92 97
93 void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicStrin g& uniqueName) 98 void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicStrin g& uniqueName)
94 { 99 {
95 if (!parent()) { 100 m_name = name;
96 DCHECK(uniqueName == name);
97 } else {
98 DCHECK(!uniqueName.isEmpty());
99 }
100 101
101 m_name = name; 102 // Non-main frames should have a non-empty unique name.
103 DCHECK(!parent() || !uniqueName.isEmpty());
102 104
103 // TODO(lukasza): We would like to assert uniqueness below (i.e. by calling 105 // TODO(lukasza): We would like to assert uniqueness below (i.e. by calling
104 // setUniqueName), but 106 // setUniqueName), but
105 // 1) uniqueness is currently violated by provisional/old frame pairs. 107 // 1) uniqueness is currently violated by provisional/old frame pairs.
106 // 2) there is an unresolved race between 2 OOPIFs, that can result in a 108 // 2) there is an unresolved race between 2 OOPIFs, that can result in a
107 // non-unique |uniqueName| - see https://crbug.com/558680#c14. 109 // non-unique |uniqueName| - see https://crbug.com/558680#c14.
108 m_uniqueName = uniqueName; 110 m_uniqueName = uniqueName;
109 } 111 }
110 112
111 Frame* FrameTree::parent() const 113 Frame* FrameTree::parent() const
(...skipping 198 matching lines...) Expand 10 before | Expand all | Expand 10 after
310 // generated in the past on user's disk). This incremental, 312 // generated in the past on user's disk). This incremental,
311 // backcompatibility-aware approach has resulted so far in the following 313 // backcompatibility-aware approach has resulted so far in the following
312 // rather baroque format... : 314 // rather baroque format... :
313 // 315 //
314 // uniqueName ::= <assignedName> | <generatedName> 316 // uniqueName ::= <assignedName> | <generatedName>
315 // (generatedName is used if assignedName is 317 // (generatedName is used if assignedName is
316 // 1) non-unique / conflicts with other frame's unique name or 318 // 1) non-unique / conflicts with other frame's unique name or
317 // 2) assignedName is empty for a non-main frame) 319 // 2) assignedName is empty for a non-main frame)
318 // 320 //
319 // assignedName ::= value of iframe's name attribute 321 // assignedName ::= value of iframe's name attribute
320 // or value assigned to window.name 322 // or value assigned to window.name (*before* the first
323 // real commit - afterwards unique name stays immutable).
321 // 324 //
322 // generatedName ::= oldGeneratedName newUniqueSuffix? 325 // generatedName ::= oldGeneratedName newUniqueSuffix?
323 // (newUniqueSuffix is only present if oldGeneratedName was 326 // (newUniqueSuffix is only present if oldGeneratedName was
324 // not unique after all) 327 // not unique after all)
325 // 328 //
326 // oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" chil dCount "-->-->" 329 // oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" chil dCount "-->-->"
327 // (main frame is special - oldGeneratedName for main frame 330 // (main frame is special - oldGeneratedName for main frame
328 // is always the frame's assignedName; oldGeneratedName is 331 // is always the frame's assignedName; oldGeneratedName is
329 // generated by generateUniqueNameCandidate method). 332 // generated by generateUniqueNameCandidate method).
330 // 333 //
(...skipping 263 matching lines...) Expand 10 before | Expand all | Expand 10 after
594 { 597 {
595 if (!frame) { 598 if (!frame) {
596 printf("Null input frame\n"); 599 printf("Null input frame\n");
597 return; 600 return;
598 } 601 }
599 602
600 printFrames(frame->tree().top(), frame, 0); 603 printFrames(frame->tree().top(), frame, 0);
601 } 604 }
602 605
603 #endif 606 #endif
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698