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

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

Issue 2329523003: Main frame's unique name can always be null. (Closed)
Patch Set: 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
« no previous file with comments | « third_party/WebKit/LayoutTests/http/tests/navigation/new-window-sandboxed-iframe-expected.txt ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 60 matching lines...) Expand 10 before | Expand all | Expand 10 after
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 76 // https://crbug.com/607205: Make sure m_uniqueName doesn't change after
77 // initial navigation - session history depends on this. 77 // initial navigation - session history depends on this.
78 if (toLocalFrame(m_thisFrame)->loader().stateMachine()->committedFirstRealDo cumentLoad()) 78 if (toLocalFrame(m_thisFrame)->loader().stateMachine()->committedFirstRealDo cumentLoad())
79 return; 79 return;
80 80
81 // Remove our old frame name so it's not considered in calculateUniqueNameFo rChildFrame 81 // Leave main frame's unique name set to a null string.
82 // and appendUniqueSuffix calls below. 82 if (!parent())
83 return;
84
85 // Remove our old frame name so it's not considered in
86 // calculateUniqueNameForChildFrame call below.
83 m_uniqueName = AtomicString(); 87 m_uniqueName = AtomicString();
84 88
85 // Calculate a new unique name based on inputs. 89 // Calculate a new unique name based on inputs.
86 if (parent()) { 90 setUniqueName(
87 setUniqueName( 91 parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nul lAtom));
88 parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nullAtom));
89 } else if (name.isEmpty() || !uniqueNameExists(name)) {
90 // Only main frame can have an empty unique name, so for main frames
91 // emptiness guarantees uniquness.
92 setUniqueName(name);
93 } else {
94 setUniqueName(appendUniqueSuffix(name, "<!--framePosition"));
95 }
96 } 92 }
97 93
98 void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicStrin g& uniqueName) 94 void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicStrin g& uniqueName)
99 { 95 {
100 m_name = name; 96 m_name = name;
101 97
98 // Leave main frame's unique name set to a null string.
99 if (!parent())
dcheng 2016/09/10 02:54:49 Is this non-empty sometimes for parent frames? I d
Łukasz Anforowicz 2016/09/12 23:58:34 I am not sure if I understand the question, but I
dcheng 2016/09/13 00:44:23 Right, but I don't think the main frame is ever *c
Łukasz Anforowicz 2016/09/13 01:14:29 Agreed.
dcheng 2016/09/13 01:38:00 Right, but the unique name we calculated in that c
Łukasz Anforowicz 2016/09/13 16:45:14 It turns out [1] that when setPrecalculatedName is
100 return;
101
102 // Non-main frames should have a non-empty unique name. 102 // Non-main frames should have a non-empty unique name.
103 DCHECK(!parent() || !uniqueName.isEmpty()); 103 DCHECK(!uniqueName.isEmpty());
104 104
105 // 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
106 // setUniqueName), but 106 // setUniqueName), but
107 // 1) uniqueness is currently violated by provisional/old frame pairs. 107 // 1) uniqueness is currently violated by provisional/old frame pairs.
108 // 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
109 // non-unique |uniqueName| - see https://crbug.com/558680#c14. 109 // non-unique |uniqueName| - see https://crbug.com/558680#c14.
110 m_uniqueName = uniqueName; 110 m_uniqueName = uniqueName;
111 } 111 }
112 112
113 Frame* FrameTree::parent() const 113 Frame* FrameTree::parent() const
(...skipping 192 matching lines...) Expand 10 before | Expand all | Expand 10 after
306 306
307 // Description of the current unique name format 307 // Description of the current unique name format
308 // --------------------------------------------- 308 // ---------------------------------------------
309 // 309 //
310 // Changing the format of unique name is undesirable, because it breaks 310 // Changing the format of unique name is undesirable, because it breaks
311 // backcompatibility of session history (which stores unique names 311 // backcompatibility of session history (which stores unique names
312 // generated in the past on user's disk). This incremental, 312 // generated in the past on user's disk). This incremental,
313 // backcompatibility-aware approach has resulted so far in the following 313 // backcompatibility-aware approach has resulted so far in the following
314 // rather baroque format... : 314 // rather baroque format... :
315 // 315 //
316 // uniqueName ::= <assignedName> | <generatedName> 316 // uniqueName ::= <nullForMainFrame> | <assignedName> | <generatedName>
317 // (generatedName is used if assignedName is 317 // (generatedName is used if assignedName is
318 // 1) non-unique / conflicts with other frame's unique name or 318 // non-unique / conflicts with other frame's unique name.
319 // 2) assignedName is empty for a non-main frame)
320 // 319 //
321 // assignedName ::= value of iframe's name attribute 320 // assignedName ::= value of iframe's name attribute
322 // or value assigned to window.name (*before* the first 321 // or value assigned to window.name (*before* the first
323 // real commit - afterwards unique name stays immutable). 322 // real commit - afterwards unique name stays immutable).
324 // 323 //
325 // generatedName ::= oldGeneratedName newUniqueSuffix? 324 // generatedName ::= oldGeneratedName newUniqueSuffix?
326 // (newUniqueSuffix is only present if oldGeneratedName was 325 // (newUniqueSuffix is only present if oldGeneratedName was
327 // not unique after all) 326 // not unique after all)
328 // 327 //
329 // oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" chil dCount "-->-->" 328 // oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" chil dCount "-->-->"
330 // (main frame is special - oldGeneratedName for main frame 329 // (oldGeneratedName is generated by generateUniqueNameCand idate method).
331 // is always the frame's assignedName; oldGeneratedName is
332 // generated by generateUniqueNameCandidate method).
333 // 330 //
334 // childCount ::= current number of siblings 331 // childCount ::= current number of siblings
335 // 332 //
336 // ancestorChain ::= concatenated unique names of ancestor chain, 333 // ancestorChain ::= concatenated unique names of ancestor chain,
337 // terminated on the first ancestor (if any) starting wi th "<!--framePath" 334 // terminated on the first ancestor (if any) starting wi th "<!--framePath"
338 // each ancestor's unique name is separated by "/" chara cter 335 // each ancestor's unique name is separated by "/" chara cter
339 // ancestorChain example1: "grandparent/parent" 336 // ancestorChain example1: "grandparent/parent"
340 // (ancestor's unique names : #1--^ | #2-^ ) 337 // (ancestor's unique names : #1--^ | #2-^ )
341 // ancestorChain example2: "<!--framePath //foo/bar/<!--frame42-->-->/blah /foobar" 338 // ancestorChain example2: "<!--framePath //foo/bar/<!--frame42-->-->/blah /foobar"
342 // (ancestor's unique names : ^--#1--^ | #2 | #3-^ ) 339 // (ancestor's unique names : ^--#1--^ | #2 | #3-^ )
343 // 340 //
344 // newUniqueSuffix ::= "<!--framePosition" framePosition "/" retryNumber " -->" 341 // newUniqueSuffix ::= "<!--framePosition" framePosition "/" retryNumber " -->"
345 // 342 //
346 // framePosition ::= "-" numberOfSiblingsBeforeChild [ framePosition-forPa rent? ] 343 // framePosition ::= "-" numberOfSiblingsBeforeChild [ framePosition-forPa rent? ]
347 // | <empty string for main frame>
348 // 344 //
349 // retryNumber ::= smallest non-negative integer resulting in unique name 345 // retryNumber ::= smallest non-negative integer resulting in unique name
350 } 346 }
351 347
352 void FrameTree::setUniqueName(const AtomicString& uniqueName) 348 void FrameTree::setUniqueName(const AtomicString& uniqueName)
353 { 349 {
354 // Main frame is the only frame that can have an empty unique name. 350 DCHECK(parent()); // Only subframes can have a custom-tailored unique name.
355 if (parent()) { 351 DCHECK(!uniqueName.isEmpty() && !uniqueNameExists(uniqueName));
356 DCHECK(!uniqueName.isEmpty() && !uniqueNameExists(uniqueName));
357 } else {
358 DCHECK(uniqueName.isEmpty() || !uniqueNameExists(uniqueName));
359 }
360
361 m_uniqueName = uniqueName; 352 m_uniqueName = uniqueName;
362 } 353 }
363 354
364 Frame* FrameTree::scopedChild(unsigned index) const 355 Frame* FrameTree::scopedChild(unsigned index) const
365 { 356 {
366 unsigned scopedIndex = 0; 357 unsigned scopedIndex = 0;
367 for (Frame* child = firstChild(); child; child = child->tree().nextSibling() ) { 358 for (Frame* child = firstChild(); child; child = child->tree().nextSibling() ) {
368 if (child->client()->inShadowTree()) 359 if (child->client()->inShadowTree())
369 continue; 360 continue;
370 if (scopedIndex == index) 361 if (scopedIndex == index)
(...skipping 226 matching lines...) Expand 10 before | Expand all | Expand 10 after
597 { 588 {
598 if (!frame) { 589 if (!frame) {
599 printf("Null input frame\n"); 590 printf("Null input frame\n");
600 return; 591 return;
601 } 592 }
602 593
603 printFrames(frame->tree().top(), frame, 0); 594 printFrames(frame->tree().top(), frame, 0);
604 } 595 }
605 596
606 #endif 597 #endif
OLDNEW
« no previous file with comments | « third_party/WebKit/LayoutTests/http/tests/navigation/new-window-sandboxed-iframe-expected.txt ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698