Attachment #186147 for bug #89351

View Details Raw Unified Return to Bug 89351

| Differences between



2013-02-01  Elliott Sprehn  <esprehn@chromium.org>

        HTML parser should queue MutationRecords for its operations
        https://bugs.webkit.org/show_bug.cgi?id=89351

        Reviewed by NOBODY (OOPS!).

        Generate mutation records inside the parser. This is done by using a
        ChildListMutationScope in the ContainerNode::parser* methods and then
        adding delivery before each <script> element would be processed by
        the parser.

        Test: fast/dom/MutationObserver/parser-mutations.html

        * dom/ContainerNode.cpp:
        (WebCore::ContainerNode::takeAllChildrenFrom):
        (WebCore::ContainerNode::parserInsertBefore):
        (WebCore::ContainerNode::parserRemoveChild):
        (WebCore::ContainerNode::parserAppendChild):
        * html/parser/HTMLScriptRunner.cpp:
        (WebCore::HTMLScriptRunner::executeParsingBlockingScript):
        (WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent):
        (WebCore::HTMLScriptRunner::execute):
        (WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad):
        (WebCore::HTMLScriptRunner::executeScriptsWaitingForStylesheets):
        (WebCore::HTMLScriptRunner::executeScriptsWaitingForParsing):
        (WebCore::HTMLScriptRunner::runScript):

2013-02-01  Philip Rogers  <pdr@google.com>

        Prevent skipped repaints for children of inner SVG elements

Lines 114-119 void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent) a/Source/WebCore/dom/ContainerNode.cpp_sec1
- a/Source/WebCore/dom/ContainerNode.cpp +9 lines
114	// FIXME: We need a no mutation event version of adoptNode.	114	// FIXME: We need a no mutation event version of adoptNode.
115	RefPtr<Node> child = document()->adoptNode(children[i].release(), ec);	115	RefPtr<Node> child = document()->adoptNode(children[i].release(), ec);
116	ASSERT(!ec);	116	ASSERT(!ec);
		117	ChildListMutationScope(oldParent).willRemoveChild(child.get());
		118	child->notifyMutationObserversNodeWillDetach();
117	parserAppendChild(child.get());	119	parserAppendChild(child.get());
118	// FIXME: Together with adoptNode above, the tree scope might get updated recursively twice	120	// FIXME: Together with adoptNode above, the tree scope might get updated recursively twice
119	// (if the document changed or oldParent was in a shadow tree, AND *this is in a shadow tree).	121	// (if the document changed or oldParent was in a shadow tree, AND *this is in a shadow tree).
Lines 344-349 void ContainerNode::parserInsertBefore(PassRefPtr<Node> newChild, Node* nextChil a/Source/WebCore/dom/ContainerNode.cpp_sec2
344		346
345	newChild->updateAncestorConnectedSubframeCountForInsertion();	347	newChild->updateAncestorConnectedSubframeCountForInsertion();
346		348
		349	ChildListMutationScope(this).childAdded(newChild.get());
		350
347	childrenChanged(true, newChild->previousSibling(), nextChild, 1);	351	childrenChanged(true, newChild->previousSibling(), nextChild, 1);
348	ChildNodeInsertionNotifier(this).notify(newChild.get());	352	ChildNodeInsertionNotifier(this).notify(newChild.get());
349	}	353	}
Lines 567-572 void ContainerNode::parserRemoveChild(Node* oldChild) a/Source/WebCore/dom/ContainerNode.cpp_sec3
567		571
568	oldChild->updateAncestorConnectedSubframeCountForRemoval();	572	oldChild->updateAncestorConnectedSubframeCountForRemoval();
569		573
		574	ChildListMutationScope(this).willRemoveChild(oldChild);
		575	oldChild->notifyMutationObserversNodeWillDetach();
		576
570	removeBetween(prev, next, oldChild);	577	removeBetween(prev, next, oldChild);
571		578
572	childrenChanged(true, prev, next, -1);	579	childrenChanged(true, prev, next, -1);
Lines 718-723 void ContainerNode::parserAppendChild(PassRefPtr<Node> newChild) a/Source/WebCore/dom/ContainerNode.cpp_sec4
718		725
719	newChild->updateAncestorConnectedSubframeCountForInsertion();	726	newChild->updateAncestorConnectedSubframeCountForInsertion();
720		727
		728	ChildListMutationScope(this).childAdded(newChild.get());
		729
721	childrenChanged(true, last, 0, 1);	730	childrenChanged(true, last, 0, 1);
722	ChildNodeInsertionNotifier(this).notify(newChild.get());	731	ChildNodeInsertionNotifier(this).notify(newChild.get());
723	}	732	}

Lines 36-41 a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec1
- a/Source/WebCore/html/parser/HTMLScriptRunner.cpp -9 / +20 lines
36	#include "HTMLNames.h"	36	#include "HTMLNames.h"
37	#include "HTMLScriptRunnerHost.h"	37	#include "HTMLScriptRunnerHost.h"
38	#include "IgnoreDestructiveWriteCountIncrementer.h"	38	#include "IgnoreDestructiveWriteCountIncrementer.h"
		39	#include "MutationObserver.h"
39	#include "NestingLevelIncrementer.h"	40	#include "NestingLevelIncrementer.h"
40	#include "NotImplemented.h"	41	#include "NotImplemented.h"
41	#include "ScriptElement.h"	42	#include "ScriptElement.h"
Lines 110-116 bool HTMLScriptRunner::isPendingScriptReady(const PendingScript& script) a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec2
110	void HTMLScriptRunner::executeParsingBlockingScript()	111	void HTMLScriptRunner::executeParsingBlockingScript()
111	{	112	{
112	ASSERT(m_document);	113	ASSERT(m_document);
113	ASSERT(!m_scriptNestingLevel);	114	ASSERT(!isExecutingScript());
114	ASSERT(m_document->haveStylesheetsLoaded());	115	ASSERT(m_document->haveStylesheetsLoaded());
115	ASSERT(isPendingScriptReady(m_parserBlockingScript));	116	ASSERT(isPendingScriptReady(m_parserBlockingScript));
116		117
Lines 127-132 void HTMLScriptRunner::executePendingScriptAndDispatchEvent(PendingScript& pendi a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec3
127	if (pendingScript.cachedScript() && pendingScript.watchingForLoad())	128	if (pendingScript.cachedScript() && pendingScript.watchingForLoad())
128	stopWatchingForLoad(pendingScript);	129	stopWatchingForLoad(pendingScript);
129		130
		131	if (!isExecutingScript())
		132	MutationObserver::deliverAllMutations();
		133
130	// Clear the pending script before possible rentrancy from executeScript()	134	// Clear the pending script before possible rentrancy from executeScript()
131	RefPtr<Element> element = pendingScript.releaseElementAndClear();	135	RefPtr<Element> element = pendingScript.releaseElementAndClear();
132	if (ScriptElement* scriptElement = toScriptElement(element.get())) {	136	if (ScriptElement* scriptElement = toScriptElement(element.get())) {
Lines 140-146 void HTMLScriptRunner::executePendingScriptAndDispatchEvent(PendingScript& pendi a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec4
140	element->dispatchEvent(createScriptLoadEvent());	144	element->dispatchEvent(createScriptLoadEvent());
141	}	145	}
142	}	146	}
143	ASSERT(!m_scriptNestingLevel);	147	ASSERT(!isExecutingScript());
144	}	148	}
145		149
146	void HTMLScriptRunner::watchForLoad(PendingScript& pendingScript)	150	void HTMLScriptRunner::watchForLoad(PendingScript& pendingScript)
Lines 170-176 void HTMLScriptRunner::execute(PassRefPtr<Element> scriptElement, const TextPosi a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec5
170	runScript(scriptElement.get(), scriptStartPosition);	174	runScript(scriptElement.get(), scriptStartPosition);
171		175
172	if (hasParserBlockingScript()) {	176	if (hasParserBlockingScript()) {
173	if (m_scriptNestingLevel)	177	if (isExecutingScript())
174	return; // Unwind to the outermost HTMLScriptRunner::execute before continuing parsing.	178	return; // Unwind to the outermost HTMLScriptRunner::execute before continuing parsing.
175	// If preload scanner got created, it is missing the source after the current insertion point. Append it and scan.	179	// If preload scanner got created, it is missing the source after the current insertion point. Append it and scan.
176	if (!hadPreloadScanner && m_host->hasPreloadScanner())	180	if (!hadPreloadScanner && m_host->hasPreloadScanner())
Lines 192-198 void HTMLScriptRunner::executeParsingBlockingScripts() a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec6
192		196
193	void HTMLScriptRunner::executeScriptsWaitingForLoad(CachedResource* cachedScript)	197	void HTMLScriptRunner::executeScriptsWaitingForLoad(CachedResource* cachedScript)
194	{	198	{
195	ASSERT(!m_scriptNestingLevel);	199	ASSERT(!isExecutingScript());
196	ASSERT(hasParserBlockingScript());	200	ASSERT(hasParserBlockingScript());
197	ASSERT_UNUSED(cachedScript, m_parserBlockingScript.cachedScript() == cachedScript);	201	ASSERT_UNUSED(cachedScript, m_parserBlockingScript.cachedScript() == cachedScript);
198	ASSERT(m_parserBlockingScript.cachedScript()->isLoaded());	202	ASSERT(m_parserBlockingScript.cachedScript()->isLoaded());
Lines 205-211 void HTMLScriptRunner::executeScriptsWaitingForStylesheets() a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec7
205	// Callers should check hasScriptsWaitingForStylesheets() before calling	209	// Callers should check hasScriptsWaitingForStylesheets() before calling
206	// to prevent parser or script re-entry during </style> parsing.	210	// to prevent parser or script re-entry during </style> parsing.
207	ASSERT(hasScriptsWaitingForStylesheets());	211	ASSERT(hasScriptsWaitingForStylesheets());
208	ASSERT(!m_scriptNestingLevel);	212	ASSERT(!isExecutingScript());
209	ASSERT(m_document->haveStylesheetsLoaded());	213	ASSERT(m_document->haveStylesheetsLoaded());
210	executeParsingBlockingScripts();	214	executeParsingBlockingScripts();
211	}	215	}
Lines 213-219 void HTMLScriptRunner::executeScriptsWaitingForStylesheets() a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec8
213	bool HTMLScriptRunner::executeScriptsWaitingForParsing()	217	bool HTMLScriptRunner::executeScriptsWaitingForParsing()
214	{	218	{
215	while (!m_scriptsToExecuteAfterParsing.isEmpty()) {	219	while (!m_scriptsToExecuteAfterParsing.isEmpty()) {
216	ASSERT(!m_scriptNestingLevel);	220	ASSERT(!isExecutingScript());
217	ASSERT(!hasParserBlockingScript());	221	ASSERT(!hasParserBlockingScript());
218	ASSERT(m_scriptsToExecuteAfterParsing.first().cachedScript());	222	ASSERT(m_scriptsToExecuteAfterParsing.first().cachedScript());
219	if (!m_scriptsToExecuteAfterParsing.first().cachedScript()->isLoaded()) {	223	if (!m_scriptsToExecuteAfterParsing.first().cachedScript()->isLoaded()) {
Lines 274-282 void HTMLScriptRunner::runScript(Element* script, const TextPosition& scriptStar a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec9
274	ASSERT(m_document);	278	ASSERT(m_document);
275	ASSERT(!hasParserBlockingScript());	279	ASSERT(!hasParserBlockingScript());
276	{	280	{
277	InsertionPointRecord insertionPointRecord(m_host->inputStream());
278	NestingLevelIncrementer nestingLevelIncrementer(m_scriptNestingLevel);
279
280	ScriptElement* scriptElement = toScriptElement(script);	281	ScriptElement* scriptElement = toScriptElement(script);
281		282
282	// This contains both and ASSERTION and a null check since we should not	283	// This contains both and ASSERTION and a null check since we should not
Lines 287-292 void HTMLScriptRunner::runScript(Element* script, const TextPosition& scriptStar a/Source/WebCore/html/parser/HTMLScriptRunner.cpp_sec10
287	if (!scriptElement)	288	if (!scriptElement)
288	return;	289	return;
289		290
		291	// FIXME: This may be too agressive as we always deliver mutations at
		292	// every script element, even if it's not ready to execute yet. There's
		293	// unfortuantely no obvious way to tell if prepareScript is going to
		294	// execute the script from out here.
		295	if (!isExecutingScript())
		296	MutationObserver::deliverAllMutations();
		297
		298	InsertionPointRecord insertionPointRecord(m_host->inputStream());
		299	NestingLevelIncrementer nestingLevelIncrementer(m_scriptNestingLevel);
		300
290	scriptElement->prepareScript(scriptStartPosition);	301	scriptElement->prepareScript(scriptStartPosition);
291		302
292	if (!scriptElement->willBeParserExecuted())	303	if (!scriptElement->willBeParserExecuted())



2013-02-01  Elliott Sprehn  <esprehn@chromium.org>

        HTML parser should queue MutationRecords for its operations
        https://bugs.webkit.org/show_bug.cgi?id=89351

        Reviewed by NOBODY (OOPS!).

        Add tests for parser generated mutations.

        * fast/dom/MutationObserver/parser-mutations-expected.txt: Added.
        * fast/dom/MutationObserver/parser-mutations.html: Added.

2013-02-01  Philip Rogers  <pdr@google.com>

        Prevent skipped repaints for children of inner SVG elements

Line 0 a/LayoutTests/fast/dom/MutationObserver/parser-mutations-expected.txt_sec1
- a/LayoutTests/fast/dom/MutationObserver/parser-mutations-expected.txt +6 lines
	1	PASS mutations.length is 6
	2	PASS mutations[1].type is "childList"
	3	PASS mutations[1].target.tagName is "BODY"
	4	PASS mutations[1].addedNodes.length is 1
	5	PASS mutations[1].addedNodes[0].tagName is "P"
	6	Mutation records should be delivered for all parser mutations after the above script.



<!DOCTYPE html>

<body>
<script src="../../js/resources/js-test-pre.js"></script>

<script>
    if (window.testRunner)
        testRunner.dumpAsText();

    var observer = new WebKitMutationObserver(function(mutations, observer) {
        window.mutations = mutations;
    });
    observer.observe(document.body, {childList: true, subtree:true});
</script>

<p>
    Mutation records should be delivered for all parser mutations after the above script.
</p>

<script>
    shouldBe('mutations.length', '6');
    shouldBeEqualToString('mutations[1].type', 'childList');
    shouldBeEqualToString('mutations[1].target.tagName', 'BODY');
    shouldBe('mutations[1].addedNodes.length', '1');
    shouldBeEqualToString('mutations[1].addedNodes[0].tagName', 'P');
</script>
</body>

Return to Bug 89351

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +28 lines
		1	2013-02-01 Elliott Sprehn <esprehn@chromium.org>
		2
		3	HTML parser should queue MutationRecords for its operations
		4	https://bugs.webkit.org/show_bug.cgi?id=89351
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Generate mutation records inside the parser. This is done by using a
		9	ChildListMutationScope in the ContainerNode::parser* methods and then
		10	adding delivery before each <script> element would be processed by
		11	the parser.
		12
		13	Test: fast/dom/MutationObserver/parser-mutations.html
		14
		15	* dom/ContainerNode.cpp:
		16	(WebCore::ContainerNode::takeAllChildrenFrom):
		17	(WebCore::ContainerNode::parserInsertBefore):
		18	(WebCore::ContainerNode::parserRemoveChild):
		19	(WebCore::ContainerNode::parserAppendChild):
		20	* html/parser/HTMLScriptRunner.cpp:
		21	(WebCore::HTMLScriptRunner::executeParsingBlockingScript):
		22	(WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent):
		23	(WebCore::HTMLScriptRunner::execute):
		24	(WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad):
		25	(WebCore::HTMLScriptRunner::executeScriptsWaitingForStylesheets):
		26	(WebCore::HTMLScriptRunner::executeScriptsWaitingForParsing):
		27	(WebCore::HTMLScriptRunner::runScript):
		28
1	2013-02-01 Philip Rogers <pdr@google.com>	29	2013-02-01 Philip Rogers <pdr@google.com>
2		30
3	Prevent skipped repaints for children of inner SVG elements	31	Prevent skipped repaints for children of inner SVG elements

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +12 lines
		1	2013-02-01 Elliott Sprehn <esprehn@chromium.org>
		2
		3	HTML parser should queue MutationRecords for its operations
		4	https://bugs.webkit.org/show_bug.cgi?id=89351
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Add tests for parser generated mutations.
		9
		10	* fast/dom/MutationObserver/parser-mutations-expected.txt: Added.
		11	* fast/dom/MutationObserver/parser-mutations.html: Added.
		12
1	2013-02-01 Philip Rogers <pdr@google.com>	13	2013-02-01 Philip Rogers <pdr@google.com>
2		14
3	Prevent skipped repaints for children of inner SVG elements	15	Prevent skipped repaints for children of inner SVG elements

Line 0 a/LayoutTests/fast/dom/MutationObserver/parser-mutations.html_sec1
- a/LayoutTests/fast/dom/MutationObserver/parser-mutations.html +27 lines
	1	<!DOCTYPE html>
	2
	3	<body>
	4	<script src="../../js/resources/js-test-pre.js"></script>
	5
	6	<script>
	7	if (window.testRunner)
	8	testRunner.dumpAsText();
	9
	10	var observer = new WebKitMutationObserver(function(mutations, observer) {
	11	window.mutations = mutations;
	12	});
	13	observer.observe(document.body, {childList: true, subtree:true});
	14	</script>
	15
	16	<p>
	17	Mutation records should be delivered for all parser mutations after the above script.
	18	</p>
	19
	20	<script>
	21	shouldBe('mutations.length', '6');
	22	shouldBeEqualToString('mutations[1].type', 'childList');
	23	shouldBeEqualToString('mutations[1].target.tagName', 'BODY');
	24	shouldBe('mutations[1].addedNodes.length', '1');
	25	shouldBeEqualToString('mutations[1].addedNodes[0].tagName', 'P');
	26	</script>
	27	</body>