Various assertion failures occur when executing script in the midst of DOM insertion

https://bugs.webkit.org/show_bug.cgi?id=132482

Reviewed by Darin Adler.

Source/WebCore:

Prior to this change, when an element containing a <script> child was inserted into a document, the script was
executed in ScriptElement::insertedInto(). That script can access nodes that follow it in the newly-inserted
hierarchy but are not yet fully inserted, leading to at least the following problems:

    - The script could remove a node that is not yet marked as in the document.
    - The script could remove a named <map> that has yet to be added to TreeScope::m_imageMapsByName.
    - The script could remove a form control that has yet to be added to FormController::m_formElementsWithState.

These scenarios all result in assertion failures. This change ensures that each node in the newly-inserted
hierarchy is fully inserted before executing any scripts.

Tests: fast/dom/element-removed-while-inserting-parent-crash.html
       fast/dom/named-map-removed-while-inserting-parent-crash.html
       fast/forms/form-control-removed-while-inserting-parent-crash.html
       svg/dom/element-removed-while-inserting-parent-crash.html

* dom/ScriptElement.cpp:
(WebCore::ScriptElement::shouldNotifySubtreeInsertions): Renamed from insertedInto().
Returned true in the case where insertedInto() would've called prepareScript().
(WebCore::ScriptElement::didNotifySubtreeInsertions): Called prepareScript().
(WebCore::ScriptElement::insertedInto): Renamed to shouldNotifySubtreeInsertions().
* dom/ScriptElement.h:
* html/HTMLScriptElement.cpp:
(WebCore::HTMLScriptElement::insertedInto): If shouldNotifySubtreeInsertions() is true, returned InsertionShouldCallDidNotifySubtreeInsertions.
Otherwise, returned InsertionDone.
(WebCore::HTMLScriptElement::didNotifySubtreeInsertions): Called ScriptElement::didNotifySubtreeInsertions().
* html/HTMLScriptElement.h:
* svg/SVGScriptElement.cpp:
(WebCore::SVGScriptElement::insertedInto): Did the same as HTMLScriptElement::insertedInto().
(WebCore::SVGScriptElement::didNotifySubtreeInsertions): Called ScriptElement::didNotifySubtreeInsertions().
* svg/SVGScriptElement.h:

LayoutTests:

Wrote named-map-removed-while-inserting-parent-crash.html by reducing the test case attached to bug 132482.
The remaining tests were taken from blink r132482.

* fast/dom/element-removed-while-inserting-parent-crash-expected.txt: Added.
* fast/dom/element-removed-while-inserting-parent-crash.html: Added.
* fast/dom/named-map-removed-while-inserting-parent-crash-expected.txt: Added.
* fast/dom/named-map-removed-while-inserting-parent-crash.html: Added.
* fast/forms/form-control-removed-while-inserting-parent-crash-expected.txt: Added.
* fast/forms/form-control-removed-while-inserting-parent-crash.html: Added.
* svg/dom/element-removed-while-inserting-parent-crash-expected.txt: Added.
* svg/dom/element-removed-while-inserting-parent-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185769 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 0c5f50e..0834f58 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2015-06-19  Andy Estes  <aestes@apple.com>
+
+        Various assertion failures occur when executing script in the midst of DOM insertion
+        https://bugs.webkit.org/show_bug.cgi?id=132482
+
+        Reviewed by Darin Adler.
+
+        Wrote named-map-removed-while-inserting-parent-crash.html by reducing the test case attached to bug 132482.
+        The remaining tests were taken from blink r132482.
+
+        * fast/dom/element-removed-while-inserting-parent-crash-expected.txt: Added.
+        * fast/dom/element-removed-while-inserting-parent-crash.html: Added.
+        * fast/dom/named-map-removed-while-inserting-parent-crash-expected.txt: Added.
+        * fast/dom/named-map-removed-while-inserting-parent-crash.html: Added.
+        * fast/forms/form-control-removed-while-inserting-parent-crash-expected.txt: Added.
+        * fast/forms/form-control-removed-while-inserting-parent-crash.html: Added.
+        * svg/dom/element-removed-while-inserting-parent-crash-expected.txt: Added.
+        * svg/dom/element-removed-while-inserting-parent-crash.html: Added.
+
 2015-06-19  Csaba Osztrogonác  <ossy@webkit.org>
 
         Remove the useless LayoutTests/platform/wincairo/TestExpectations file

diff --git a/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash-expected.txt b/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
+++ b/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash-expected.txt
@@ -0,0 +1 @@
+PASS

diff --git a/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash.html b/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash.html
new file mode 100644 (file)
index 0000000..b0b324d
--- /dev/null
+++ b/LayoutTests/fast/dom/element-removed-while-inserting-parent-crash.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var element = document.createElement();
+
+var script = document.createElement('script');
+script.textContent = 'document.currentScript.nextSibling.remove()';
+
+var container = document.createElement('div');
+container.appendChild(script);
+container.appendChild(element);
+
+document.body.appendChild(container);
+document.body.innerHTML = 'PASS';
+</script>
+</body>
+</html>

diff --git a/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash-expected.txt b/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
+++ b/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash-expected.txt
@@ -0,0 +1 @@
+PASS

diff --git a/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash.html b/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash.html
new file mode 100644 (file)
index 0000000..dc5af63
--- /dev/null
+++ b/LayoutTests/fast/dom/named-map-removed-while-inserting-parent-crash.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+
+<!-- Ensures that TreeScope::m_imageMapsByName is created -->
+<map name></map>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var map = document.createElement('map');
+map.name = 'map';
+
+var script = document.createElement('script');
+script.textContent = 'document.currentScript.parentNode.remove()';
+
+var container = document.createElement('div');
+container.appendChild(script);
+container.appendChild(map);
+
+document.body.appendChild(container);
+document.body.innerHTML = 'PASS';
+</script>
+</body>
+</html>

diff --git a/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash-expected.txt b/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
+++ b/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash-expected.txt
@@ -0,0 +1 @@
+PASS

diff --git a/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash.html b/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash.html
new file mode 100644 (file)
index 0000000..0d19393
--- /dev/null
+++ b/LayoutTests/fast/forms/form-control-removed-while-inserting-parent-crash.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var input = document.createElement('input');
+
+var script = document.createElement('script');
+script.textContent = 'document.currentScript.parentNode.remove()';
+
+var container = document.createElement('div');
+container.appendChild(script);
+container.appendChild(input);
+
+document.body.appendChild(container);
+document.body.innerHTML = 'PASS';
+</script>
+</body>
+</html>

diff --git a/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash-expected.txt b/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
+++ b/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash-expected.txt
@@ -0,0 +1 @@
+PASS

diff --git a/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash.html b/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash.html
new file mode 100644 (file)
index 0000000..34dc23f
--- /dev/null
+++ b/LayoutTests/svg/dom/element-removed-while-inserting-parent-crash.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var element = document.createElementNS('http://www.w3.org/2000/svg');
+
+var script = document.createElementNS('http://www.w3.org/2000/svg', 'script');
+script.id = 'script';
+script.textContent = 'document.getElementById(\'script\').nextSibling.remove()'; // document.currentScript doesn't work for SVGScriptElement.
+
+var container = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+container.appendChild(script);
+container.appendChild(element);
+
+document.body.appendChild(container);
+document.body.innerHTML = 'PASS';
+</script>
+</body>
+</html>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 02e25ac..50ac55c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,42 @@
+2015-06-19  Andy Estes  <aestes@apple.com>
+
+        Various assertion failures occur when executing script in the midst of DOM insertion
+        https://bugs.webkit.org/show_bug.cgi?id=132482
+
+        Reviewed by Darin Adler.
+
+        Prior to this change, when an element containing a <script> child was inserted into a document, the script was
+        executed in ScriptElement::insertedInto(). That script can access nodes that follow it in the newly-inserted
+        hierarchy but are not yet fully inserted, leading to at least the following problems:
+
+            - The script could remove a node that is not yet marked as in the document.
+            - The script could remove a named <map> that has yet to be added to TreeScope::m_imageMapsByName.
+            - The script could remove a form control that has yet to be added to FormController::m_formElementsWithState.
+
+        These scenarios all result in assertion failures. This change ensures that each node in the newly-inserted
+        hierarchy is fully inserted before executing any scripts.
+
+        Tests: fast/dom/element-removed-while-inserting-parent-crash.html
+               fast/dom/named-map-removed-while-inserting-parent-crash.html
+               fast/forms/form-control-removed-while-inserting-parent-crash.html
+               svg/dom/element-removed-while-inserting-parent-crash.html
+
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::shouldNotifySubtreeInsertions): Renamed from insertedInto().
+        Returned true in the case where insertedInto() would've called prepareScript().
+        (WebCore::ScriptElement::didNotifySubtreeInsertions): Called prepareScript().
+        (WebCore::ScriptElement::insertedInto): Renamed to shouldNotifySubtreeInsertions().
+        * dom/ScriptElement.h:
+        * html/HTMLScriptElement.cpp:
+        (WebCore::HTMLScriptElement::insertedInto): If shouldNotifySubtreeInsertions() is true, returned InsertionShouldCallDidNotifySubtreeInsertions.
+        Otherwise, returned InsertionDone.
+        (WebCore::HTMLScriptElement::didNotifySubtreeInsertions): Called ScriptElement::didNotifySubtreeInsertions().
+        * html/HTMLScriptElement.h:
+        * svg/SVGScriptElement.cpp:
+        (WebCore::SVGScriptElement::insertedInto): Did the same as HTMLScriptElement::insertedInto().
+        (WebCore::SVGScriptElement::didNotifySubtreeInsertions): Called ScriptElement::didNotifySubtreeInsertions().
+        * svg/SVGScriptElement.h:
+
 2015-06-19  Brent Fulgham  <bfulgham@apple.com>
 
         All calls of ImageBuffer::create should null check the return value

diff --git a/Source/WebCore/dom/ScriptElement.cpp b/Source/WebCore/dom/ScriptElement.cpp
index 4824d7c..bb9ff9e 100644 (file)
--- a/Source/WebCore/dom/ScriptElement.cpp
+++ b/Source/WebCore/dom/ScriptElement.cpp
@@ -79,10 +79,15 @@ ScriptElement::~ScriptElement()
     stopLoadRequest();
 }
 
-void ScriptElement::insertedInto(ContainerNode& insertionPoint)
+bool ScriptElement::shouldNotifySubtreeInsertions(ContainerNode& insertionPoint)
 {
-    if (insertionPoint.inDocument() && !m_parserInserted)
-        prepareScript(); // FIXME: Provide a real starting line number here.
+    return insertionPoint.inDocument() && !m_parserInserted;
+}
+
+void ScriptElement::didNotifySubtreeInsertions()
+{
+    ASSERT(!m_parserInserted);
+    prepareScript(); // FIXME: Provide a real starting line number here.
 }
 
 void ScriptElement::childrenChanged()

diff --git a/Source/WebCore/dom/ScriptElement.h b/Source/WebCore/dom/ScriptElement.h
index 0aa97a9..bdbe667 100644 (file)
--- a/Source/WebCore/dom/ScriptElement.h
+++ b/Source/WebCore/dom/ScriptElement.h
@@ -69,7 +69,8 @@ protected:
     bool forceAsync() const { return m_forceAsync; }
 
     // Helper functions used by our parent classes.
-    void insertedInto(ContainerNode&);
+    bool shouldNotifySubtreeInsertions(ContainerNode&);
+    void didNotifySubtreeInsertions();
     void childrenChanged();
     void handleSourceAttribute(const String& sourceUrl);
     void handleAsyncAttribute();

diff --git a/Source/WebCore/html/HTMLScriptElement.cpp b/Source/WebCore/html/HTMLScriptElement.cpp
index 54a872a..402477c 100644 (file)
--- a/Source/WebCore/html/HTMLScriptElement.cpp
+++ b/Source/WebCore/html/HTMLScriptElement.cpp
@@ -70,8 +70,12 @@ void HTMLScriptElement::parseAttribute(const QualifiedName& name, const AtomicSt
 Node::InsertionNotificationRequest HTMLScriptElement::insertedInto(ContainerNode& insertionPoint)
 {
     HTMLElement::insertedInto(insertionPoint);
-    ScriptElement::insertedInto(insertionPoint);
-    return InsertionDone;
+    return shouldNotifySubtreeInsertions(insertionPoint) ? InsertionShouldCallDidNotifySubtreeInsertions : InsertionDone;
+}
+
+void HTMLScriptElement::didNotifySubtreeInsertions()
+{
+    ScriptElement::didNotifySubtreeInsertions();
 }
 
 void HTMLScriptElement::setText(const String &value)

diff --git a/Source/WebCore/html/HTMLScriptElement.h b/Source/WebCore/html/HTMLScriptElement.h
index 0675c5d..9c1e84c 100644 (file)
--- a/Source/WebCore/html/HTMLScriptElement.h
+++ b/Source/WebCore/html/HTMLScriptElement.h
@@ -46,6 +46,7 @@ private:
 
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) override;
     virtual InsertionNotificationRequest insertedInto(ContainerNode&) override;
+    virtual void didNotifySubtreeInsertions() override;
     virtual void childrenChanged(const ChildChange&) override;
 
     virtual bool isURLAttribute(const Attribute&) const override;

diff --git a/Source/WebCore/svg/SVGScriptElement.cpp b/Source/WebCore/svg/SVGScriptElement.cpp
index 3a7771d..55797b0 100644 (file)
--- a/Source/WebCore/svg/SVGScriptElement.cpp
+++ b/Source/WebCore/svg/SVGScriptElement.cpp
@@ -77,10 +77,14 @@ void SVGScriptElement::svgAttributeChanged(const QualifiedName& attrName)
 Node::InsertionNotificationRequest SVGScriptElement::insertedInto(ContainerNode& rootParent)
 {
     SVGElement::insertedInto(rootParent);
-    ScriptElement::insertedInto(rootParent);
     if (rootParent.inDocument())
         SVGExternalResourcesRequired::insertedIntoDocument(this);
-    return InsertionDone;
+    return shouldNotifySubtreeInsertions(rootParent) ? InsertionShouldCallDidNotifySubtreeInsertions : InsertionDone;
+}
+
+void SVGScriptElement::didNotifySubtreeInsertions()
+{
+    ScriptElement::didNotifySubtreeInsertions();
 }
 
 void SVGScriptElement::childrenChanged(const ChildChange& change)

diff --git a/Source/WebCore/svg/SVGScriptElement.h b/Source/WebCore/svg/SVGScriptElement.h
index 517abf6..aa88187 100644 (file)
--- a/Source/WebCore/svg/SVGScriptElement.h
+++ b/Source/WebCore/svg/SVGScriptElement.h
@@ -42,6 +42,7 @@ private:
 
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) override;
     virtual InsertionNotificationRequest insertedInto(ContainerNode&) override;
+    virtual void didNotifySubtreeInsertions() override;
     virtual void childrenChanged(const ChildChange&) override;
 
     virtual void svgAttributeChanged(const QualifiedName&) override;