Skip to content

GitLab

T
tangled
Commit 5913a51c authored by davve@opera.com's avatar davve@opera.com
Browse files
Options

Revert "Remove children of title node before updating" 

It causes performance problems. See
https://code.google.com/p/chromium/issues/detail?id=331076

Micro-benchmarks like http://jsperf.com/setting-document-title regress
about one magnitude.

We need to figure out whether the spec should change or if the CL
could be written in a less performance-intrusive way, or if it's a
performance penalty we accept. So revert for now.

The follow-up "Inhibit title updates when removing children before
setting new value" is reverted too since it has no value without the
first patch.

Reverted CLs:

https://codereview.chromium.org/128603002
https://codereview.chromium.org/107513013

BUG=331076

Review URL: https://codereview.chromium.org/138213005

git-svn-id: svn://svn.chromium.org/blink/trunk@165117 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent e0d14ba8
Hide whitespace changes
Inline Side-by-side
Showing with 18 additions and 90 deletions
third_party/WebKit/LayoutTests/fast/dom/document-set-title-mutations-expected.txt deleted 100644 → 0
View file @ e0d14ba8
Test for mutations to childList when setting document.title.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS mutations.length is 1
PASS mutations[0].type is "childList"
PASS mutations[0].addedNodes.length is 1
PASS mutations[0].removedNodes.length is 1
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/dom/document-set-title-mutations.html deleted 100644 → 0
View file @ e0d14ba8
<!doctype html>
<title>old</title>
<script src="../../resources/js-test.js"></script>
<script>
window.jsTestIsAsync = true;
description("Test for mutations to childList when setting document.title.");
var mutations;
function finish() {
shouldBe('mutations.length', '1');
shouldBe('mutations[0].type', '"childList"');
shouldBe('mutations[0].addedNodes.length', '1');
shouldBe('mutations[0].removedNodes.length', '1');
finishJSTest();
}
var titleElement = document.querySelector('title');
var i = 0;
var observer = new MutationObserver(function(mutations) {
window.mutations = mutations;
});
observer.observe(titleElement, { childList: true });
document.title = "new";
setTimeout(finish, 0);
</script>
<body>
</body>
third_party/WebKit/LayoutTests/fast/dom/document-set-title-no-reuse-expected.txt deleted 100644 → 0
View file @ e0d14ba8
Test that setting document.title does not reuse <title>'s textnode child
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
PASS oldnode.textContent != "bbb" is true
PASS oldnode != document.getElementsByTagName("title")[0].firstChild is true
third_party/WebKit/LayoutTests/fast/dom/document-set-title-no-reuse.html deleted 100644 → 0
View file @ e0d14ba8
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<script type="text/javascript">
description("Test that setting document.title does not reuse &lt;title&gt;'s textnode child");
onload = function()
{
document.title = "aaa";
oldnode = document.getElementsByTagName("title")[0].firstChild;
document.title = "bbb";
shouldBeTrue('oldnode.textContent != "bbb"');
shouldBeTrue('oldnode != document.getElementsByTagName("title")[0].firstChild');
}
</script>
third_party/WebKit/LayoutTests/fast/dom/title-text-property-3.html deleted 100644 → 0
View file @ e0d14ba8
<title>Test for single title change</title>
<script>
document.querySelector('title').appendChild(document.createTextNode(" - part II"));
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpTitleChanges();
}
document.querySelector('title').text = "new";
</script>
third_party/WebKit/Source/core/dom/Document.cpp
View file @ 5913a51c
......@@ -1299,10 +1299,10 @@ void Document::setTitle(const String& title)
}
}
updateTitle(title);
if (m_titleElement && m_titleElement->hasTagName(titleTag))
toHTMLTitleElement(m_titleElement)->setText(title);
else
updateTitle(title);
}
void Document::setTitleElement(const String& title, Element* titleElement)
......
third_party/WebKit/Source/core/html/HTMLTitleElement.cpp
View file @ 5913a51c
......@@ -25,7 +25,6 @@
#include "HTMLNames.h"
#include "bindings/v8/ExceptionStatePlaceholder.h"
#include "core/dom/ChildListMutationScope.h"
#include "core/dom/Document.h"
#include "core/dom/Text.h"
#include "core/rendering/style/RenderStyle.h"
......@@ -38,7 +37,6 @@ using namespace HTMLNames;
inline HTMLTitleElement::HTMLTitleElement(Document& document)
: HTMLElement(titleTag, document)
, m_ignoreTitleUpdatesWhenChildrenChange(false)
{
setHasCustomStyleCallbacks();
ScriptWrappable::init(this);
......@@ -67,7 +65,7 @@ void HTMLTitleElement::removedFrom(ContainerNode* insertionPoint)
void HTMLTitleElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
{
HTMLElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
if (inDocument() && !isInShadowTree() && !m_ignoreTitleUpdatesWhenChildrenChange)
if (inDocument() && !isInShadowTree())
document().setTitleElement(text(), this);
}
......@@ -86,14 +84,23 @@ String HTMLTitleElement::text() const
void HTMLTitleElement::setText(const String &value)
{
RefPtr<Node> protectFromMutationEvents(this);
ChildListMutationScope mutation(*this);
// Avoid calling Document::setTitleElement() during intermediate steps.
m_ignoreTitleUpdatesWhenChildrenChange = true;
removeChildren();
m_ignoreTitleUpdatesWhenChildrenChange = false;
int numChildren = childNodeCount();
if (numChildren == 1 && firstChild()->isTextNode()) {
toText(firstChild())->setData(value);
document().setTitleElement(text(), this);
} else {
// We make a copy here because entity of "value" argument can be Document::m_title,
// which goes empty during removeChildren() invocation below,
// which causes HTMLTitleElement::childrenChanged(), which ends up Document::setTitle().
String valueCopy(value);
appendChild(document().createTextNode(value.impl()), IGNORE_EXCEPTION);
if (numChildren > 0)
removeChildren();
appendChild(document().createTextNode(valueCopy.impl()), IGNORE_EXCEPTION);
}
}
}
third_party/WebKit/Source/core/html/HTMLTitleElement.h
View file @ 5913a51c
......@@ -39,8 +39,6 @@ private:
virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
virtual void removedFrom(ContainerNode*) OVERRIDE;
virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0) OVERRIDE;
bool m_ignoreTitleUpdatesWhenChildrenChange;
};
DEFINE_NODE_TYPE_CASTS(HTMLTitleElement, hasTagName(HTMLNames::titleTag));
......
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