- Revision
- 129288
- Author
- [email protected]
- Date
- 2012-09-21 18:27:52 -0700 (Fri, 21 Sep 2012)
Log Message
Remove bogus assertions from ChildListMutationScope
https://bugs.webkit.org/show_bug.cgi?id=97372
Reviewed by Ryosuke Niwa.
Source/WebCore:
Some asserts (and their accompanying comment) were trying to enforce
proper usage of ChildListMutationScope from WebCore, but in the
presence of MutationEvents they could fail due to arbitrary script
execution.
This change gets rid of those asserts and adds tests exercising
the (pre-existing) codepaths for handling these out-of-order cases.
Without this patch, these tests ASSERT in debug builds.
Tests: fast/mutation/added-out-of-order.html
fast/mutation/removed-out-of-order.html
* dom/ChildListMutationScope.cpp:
(WebCore::ChildListMutationAccumulator::childAdded):
(WebCore::ChildListMutationAccumulator::willRemoveChild):
* dom/ChildListMutationScope.h:
(WebCore):
LayoutTests:
* fast/mutation/added-out-of-order-expected.txt: Added.
* fast/mutation/added-out-of-order.html: Added.
* fast/mutation/removed-out-of-order-expected.txt: Added.
* fast/mutation/removed-out-of-order.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (129287 => 129288)
--- trunk/LayoutTests/ChangeLog 2012-09-22 01:18:54 UTC (rev 129287)
+++ trunk/LayoutTests/ChangeLog 2012-09-22 01:27:52 UTC (rev 129288)
@@ -1,3 +1,15 @@
+2012-09-21 Adam Klein <[email protected]>
+
+ Remove bogus assertions from ChildListMutationScope
+ https://bugs.webkit.org/show_bug.cgi?id=97372
+
+ Reviewed by Ryosuke Niwa.
+
+ * fast/mutation/added-out-of-order-expected.txt: Added.
+ * fast/mutation/added-out-of-order.html: Added.
+ * fast/mutation/removed-out-of-order-expected.txt: Added.
+ * fast/mutation/removed-out-of-order.html: Added.
+
2012-09-21 Dan Bernstein <[email protected]>
REGRESSION (r129176): Incorrect line breaking when kerning occurs between a space and the following character
Added: trunk/LayoutTests/fast/mutation/added-out-of-order-expected.txt (0 => 129288)
--- trunk/LayoutTests/fast/mutation/added-out-of-order-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/mutation/added-out-of-order-expected.txt 2012-09-22 01:27:52 UTC (rev 129288)
@@ -0,0 +1,19 @@
+Test MutationEvents interfering with MutationObservers: adding nodes 'out of order'
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS mutations.length is 3
+PASS mutations[0].addedNodes.length is 0
+PASS mutations[0].removedNodes.length is 1
+PASS mutations[0].removedNodes[0].tagName is 'SPAN'
+PASS mutations[1].addedNodes.length is 1
+PASS mutations[1].removedNodes.length is 0
+PASS mutations[1].addedNodes[0].tagName is 'DIV'
+PASS mutations[2].addedNodes.length is 1
+PASS mutations[2].removedNodes.length is 0
+PASS mutations[2].addedNodes[0].nodeValue is 'hello world'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/mutation/added-out-of-order.html (0 => 129288)
--- trunk/LayoutTests/fast/mutation/added-out-of-order.html (rev 0)
+++ trunk/LayoutTests/fast/mutation/added-out-of-order.html 2012-09-22 01:27:52 UTC (rev 129288)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<div id="sandbox" style="display:none"><span></span></div>
+<script src=""
+<script>
+description("Test MutationEvents interfering with MutationObservers: adding nodes 'out of order'");
+var sandbox = document.getElementById('sandbox');
+var inserted = false;
+sandbox.addEventListener('DOMNodeRemoved', function() {
+ if (!inserted) {
+ sandbox.appendChild(document.createElement('div'));
+ inserted = true;
+ }
+});
+var observer = new WebKitMutationObserver(function(){});
+observer.observe(sandbox, {childList: true});
+sandbox.textContent = 'hello world';
+
+var mutations = observer.takeRecords();
+shouldBe("mutations.length", "3");
+shouldBe("mutations[0].addedNodes.length", "0");
+shouldBe("mutations[0].removedNodes.length", "1");
+shouldBe("mutations[0].removedNodes[0].tagName", "'SPAN'");
+shouldBe("mutations[1].addedNodes.length", "1");
+shouldBe("mutations[1].removedNodes.length", "0");
+shouldBe("mutations[1].addedNodes[0].tagName", "'DIV'");
+shouldBe("mutations[2].addedNodes.length", "1");
+shouldBe("mutations[2].removedNodes.length", "0");
+shouldBe("mutations[2].addedNodes[0].nodeValue", "'hello world'");
+</script>
+<script src=""
Added: trunk/LayoutTests/fast/mutation/removed-out-of-order-expected.txt (0 => 129288)
--- trunk/LayoutTests/fast/mutation/removed-out-of-order-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/mutation/removed-out-of-order-expected.txt 2012-09-22 01:27:52 UTC (rev 129288)
@@ -0,0 +1,17 @@
+Test MutationEvents interfering with MutationObservers: removing nodes 'out of order'
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS mutations.length is 2
+PASS mutations[0].addedNodes.length is 1
+PASS mutations[0].removedNodes.length is 0
+PASS mutations[0].addedNodes[0].tagName is 'B'
+PASS mutations[1].addedNodes.length is 1
+PASS mutations[1].removedNodes.length is 1
+PASS mutations[1].removedNodes[0].tagName is 'B'
+PASS mutations[1].addedNodes[0].tagName is 'I'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/mutation/removed-out-of-order.html (0 => 129288)
--- trunk/LayoutTests/fast/mutation/removed-out-of-order.html (rev 0)
+++ trunk/LayoutTests/fast/mutation/removed-out-of-order.html 2012-09-22 01:27:52 UTC (rev 129288)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<div id="sandbox" style="display:none"></div>
+<script src=""
+<script>
+description("Test MutationEvents interfering with MutationObservers: removing nodes 'out of order'");
+var sandbox = document.getElementById('sandbox');
+var removed = false;
+sandbox.addEventListener('DOMNodeInserted', function() {
+ if (!removed) {
+ sandbox.removeChild(sandbox.firstChild);
+ removed = true;
+ }
+});
+var observer = new WebKitMutationObserver(function(){});
+observer.observe(sandbox, {childList: true});
+sandbox.innerHTML = '<b></b><i></i>';
+
+var mutations = observer.takeRecords();
+shouldBe("mutations.length", "2");
+shouldBe("mutations[0].addedNodes.length", "1");
+shouldBe("mutations[0].removedNodes.length", "0");
+shouldBe("mutations[0].addedNodes[0].tagName", "'B'");
+shouldBe("mutations[1].addedNodes.length", "1");
+shouldBe("mutations[1].removedNodes.length", "1");
+shouldBe("mutations[1].removedNodes[0].tagName", "'B'");
+shouldBe("mutations[1].addedNodes[0].tagName", "'I'");
+</script>
+</script>
+<script src=""
Modified: trunk/Source/WebCore/ChangeLog (129287 => 129288)
--- trunk/Source/WebCore/ChangeLog 2012-09-22 01:18:54 UTC (rev 129287)
+++ trunk/Source/WebCore/ChangeLog 2012-09-22 01:27:52 UTC (rev 129288)
@@ -1,3 +1,28 @@
+2012-09-21 Adam Klein <[email protected]>
+
+ Remove bogus assertions from ChildListMutationScope
+ https://bugs.webkit.org/show_bug.cgi?id=97372
+
+ Reviewed by Ryosuke Niwa.
+
+ Some asserts (and their accompanying comment) were trying to enforce
+ proper usage of ChildListMutationScope from WebCore, but in the
+ presence of MutationEvents they could fail due to arbitrary script
+ execution.
+
+ This change gets rid of those asserts and adds tests exercising
+ the (pre-existing) codepaths for handling these out-of-order cases.
+ Without this patch, these tests ASSERT in debug builds.
+
+ Tests: fast/mutation/added-out-of-order.html
+ fast/mutation/removed-out-of-order.html
+
+ * dom/ChildListMutationScope.cpp:
+ (WebCore::ChildListMutationAccumulator::childAdded):
+ (WebCore::ChildListMutationAccumulator::willRemoveChild):
+ * dom/ChildListMutationScope.h:
+ (WebCore):
+
2012-09-21 Dan Bernstein <[email protected]>
REGRESSION (r129176): Incorrect line breaking when kerning occurs between a space and the following character
Modified: trunk/Source/WebCore/dom/ChildListMutationScope.cpp (129287 => 129288)
--- trunk/Source/WebCore/dom/ChildListMutationScope.cpp 2012-09-22 01:18:54 UTC (rev 129287)
+++ trunk/Source/WebCore/dom/ChildListMutationScope.cpp 2012-09-22 01:27:52 UTC (rev 129288)
@@ -87,10 +87,10 @@
void ChildListMutationAccumulator::childAdded(PassRefPtr<Node> prpChild)
{
+ ASSERT(hasObservers());
+
RefPtr<Node> child = prpChild;
- ASSERT(hasObservers());
- ASSERT(isAddedNodeInOrder(child.get()));
if (!isAddedNodeInOrder(child.get()))
enqueueMutationRecord();
@@ -110,10 +110,10 @@
void ChildListMutationAccumulator::willRemoveChild(PassRefPtr<Node> prpChild)
{
+ ASSERT(hasObservers());
+
RefPtr<Node> child = prpChild;
- ASSERT(hasObservers());
- ASSERT(m_addedNodes.isEmpty() && isRemovedNodeInOrder(child.get()));
if (!m_addedNodes.isEmpty() || !isRemovedNodeInOrder(child.get()))
enqueueMutationRecord();
Modified: trunk/Source/WebCore/dom/ChildListMutationScope.h (129287 => 129288)
--- trunk/Source/WebCore/dom/ChildListMutationScope.h 2012-09-22 01:18:54 UTC (rev 129287)
+++ trunk/Source/WebCore/dom/ChildListMutationScope.h 2012-09-22 01:27:52 UTC (rev 129288)
@@ -46,11 +46,6 @@
class MutationObserverInterestGroup;
// ChildListMutationAccumulator is not meant to be used directly; ChildListMutationScope is the public interface.
-//
-// ChildListMutationAccumulator expects that all removals from a parent take place in order
-// and precede any additions. If this is violated (i.e. because of code changes elsewhere
-// in WebCore) it will likely result in both (a) ASSERTions failing, and (b) mutation records
-// being enqueued for delivery before the outer-most scope closes.
class ChildListMutationAccumulator : public RefCounted<ChildListMutationAccumulator> {
public:
static PassRefPtr<ChildListMutationAccumulator> getOrCreate(Node*);
