Commit 5140a41c authored by lukasza's avatar lukasza Committed by Commit bot

Main frame's unique name can always be null.

This simplifies the code a little bit + avoids consuming memory for what
is ultimately an unused value.

BUG=647392
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2329523003
Cr-Commit-Position: refs/heads/master@{#418976}
parent 7daa95f3
...@@ -232,6 +232,15 @@ void FrameTreeNode::SetFrameName(const std::string& name, ...@@ -232,6 +232,15 @@ void FrameTreeNode::SetFrameName(const std::string& name,
DCHECK_EQ(unique_name, replication_state_.unique_name); DCHECK_EQ(unique_name, replication_state_.unique_name);
return; return;
} }
if (parent()) {
// Non-main frames should have a non-empty unique name.
DCHECK(!unique_name.empty());
} else {
// Unique name of main frames should always stay empty.
DCHECK(unique_name.empty());
}
RecordUniqueNameLength(unique_name.size()); RecordUniqueNameLength(unique_name.size());
render_manager_.OnDidUpdateName(name, unique_name); render_manager_.OnDidUpdateName(name, unique_name);
replication_state_.name = name; replication_state_.name = name;
......
...@@ -1552,9 +1552,9 @@ void WebContentsImpl::Init(const WebContents::CreateParams& params) { ...@@ -1552,9 +1552,9 @@ void WebContentsImpl::Init(const WebContents::CreateParams& params) {
params.main_frame_routing_id, params.main_frame_routing_id,
main_frame_widget_routing_id); main_frame_widget_routing_id);
// blink::FrameTree::setName uses |name| as the |unique_name| for the main // blink::FrameTree::setName always keeps |unique_name| empty in case of a
// frame - let's do the same thing here. // main frame - let's do the same thing here.
std::string unique_name = params.main_frame_name; std::string unique_name;
frame_tree_.root()->SetFrameName(params.main_frame_name, unique_name); frame_tree_.root()->SetFrameName(params.main_frame_name, unique_name);
WebContentsViewDelegate* delegate = WebContentsViewDelegate* delegate =
......
...@@ -13,5 +13,5 @@ curr-> http://127.0.0.1:8000/navigation/resources/new-window-sandboxed-iframe-d ...@@ -13,5 +13,5 @@ curr-> http://127.0.0.1:8000/navigation/resources/new-window-sandboxed-iframe-d
=============================================== ===============================================
============== Back Forward List ============== ============== Back Forward List ==============
curr-> http://127.0.0.1:8000/navigation/resources/notify-done.html (in frame "_top") curr-> http://127.0.0.1:8000/navigation/resources/notify-done.html
=============================================== ===============================================
...@@ -78,29 +78,30 @@ void FrameTree::setName(const AtomicString& name) ...@@ -78,29 +78,30 @@ void FrameTree::setName(const AtomicString& name)
if (toLocalFrame(m_thisFrame)->loader().stateMachine()->committedFirstRealDocumentLoad()) if (toLocalFrame(m_thisFrame)->loader().stateMachine()->committedFirstRealDocumentLoad())
return; return;
// Remove our old frame name so it's not considered in calculateUniqueNameForChildFrame // Leave main frame's unique name set to a null string.
// and appendUniqueSuffix calls below. if (!parent())
return;
// Remove our old frame name so it's not considered in
// calculateUniqueNameForChildFrame call below.
m_uniqueName = AtomicString(); m_uniqueName = AtomicString();
// Calculate a new unique name based on inputs. // Calculate a new unique name based on inputs.
if (parent()) { setUniqueName(
setUniqueName( parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nullAtom));
parent()->tree().calculateUniqueNameForChildFrame(m_thisFrame, name, nullAtom));
} else if (name.isEmpty() || !uniqueNameExists(name)) {
// Only main frame can have an empty unique name, so for main frames
// emptiness guarantees uniquness.
setUniqueName(name);
} else {
setUniqueName(appendUniqueSuffix(name, "<!--framePosition"));
}
} }
void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicString& uniqueName) void FrameTree::setPrecalculatedName(const AtomicString& name, const AtomicString& uniqueName)
{ {
m_name = name; m_name = name;
// Non-main frames should have a non-empty unique name. if (parent()) {
DCHECK(!parent() || !uniqueName.isEmpty()); // Non-main frames should have a non-empty unique name.
DCHECK(!uniqueName.isEmpty());
} else {
// Unique name of main frames should always stay empty.
DCHECK(uniqueName.isEmpty());
}
// TODO(lukasza): We would like to assert uniqueness below (i.e. by calling // TODO(lukasza): We would like to assert uniqueness below (i.e. by calling
// setUniqueName), but // setUniqueName), but
...@@ -313,10 +314,9 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame( ...@@ -313,10 +314,9 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame(
// backcompatibility-aware approach has resulted so far in the following // backcompatibility-aware approach has resulted so far in the following
// rather baroque format... : // rather baroque format... :
// //
// uniqueName ::= <assignedName> | <generatedName> // uniqueName ::= <nullForMainFrame> | <assignedName> | <generatedName>
// (generatedName is used if assignedName is // (generatedName is used if assignedName is
// 1) non-unique / conflicts with other frame's unique name or // non-unique / conflicts with other frame's unique name.
// 2) assignedName is empty for a non-main frame)
// //
// assignedName ::= value of iframe's name attribute // assignedName ::= value of iframe's name attribute
// or value assigned to window.name (*before* the first // or value assigned to window.name (*before* the first
...@@ -327,9 +327,7 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame( ...@@ -327,9 +327,7 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame(
// not unique after all) // not unique after all)
// //
// oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" childCount "-->-->" // oldGeneratedName ::= "<!--framePath //" ancestorChain "/<!--frame" childCount "-->-->"
// (main frame is special - oldGeneratedName for main frame // (oldGeneratedName is generated by generateUniqueNameCandidate method).
// is always the frame's assignedName; oldGeneratedName is
// generated by generateUniqueNameCandidate method).
// //
// childCount ::= current number of siblings // childCount ::= current number of siblings
// //
...@@ -344,20 +342,17 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame( ...@@ -344,20 +342,17 @@ AtomicString FrameTree::calculateUniqueNameForChildFrame(
// newUniqueSuffix ::= "<!--framePosition" framePosition "/" retryNumber "-->" // newUniqueSuffix ::= "<!--framePosition" framePosition "/" retryNumber "-->"
// //
// framePosition ::= "-" numberOfSiblingsBeforeChild [ framePosition-forParent? ] // framePosition ::= "-" numberOfSiblingsBeforeChild [ framePosition-forParent? ]
// | <empty string for main frame>
// //
// retryNumber ::= smallest non-negative integer resulting in unique name // retryNumber ::= smallest non-negative integer resulting in unique name
} }
void FrameTree::setUniqueName(const AtomicString& uniqueName) void FrameTree::setUniqueName(const AtomicString& uniqueName)
{ {
// Main frame is the only frame that can have an empty unique name. // Only subframes can have a non-null unique name - setUniqueName should
if (parent()) { // only be called for subframes and never for a main frame.
DCHECK(!uniqueName.isEmpty() && !uniqueNameExists(uniqueName)); DCHECK(parent());
} else {
DCHECK(uniqueName.isEmpty() || !uniqueNameExists(uniqueName));
}
DCHECK(!uniqueName.isEmpty() && !uniqueNameExists(uniqueName));
m_uniqueName = uniqueName; m_uniqueName = uniqueName;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment