Title: [235956] trunk
- Revision
- 235956
- Author
- [email protected]
- Date
- 2018-09-12 15:36:51 -0700 (Wed, 12 Sep 2018)
Log Message
imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html hits assertion
https://bugs.webkit.org/show_bug.cgi?id=189493
Reviewed by Alex Christensen.
Source/WebCore:
The debug assertion was caused by RefPtr in FormAssociatedElement::formOwnerRemovedFromTree introduced
by r224390 and r223644 ref'ing ShadowRoot while calling removeDetachedChildren inside ~ShadowRoot.
When a form (or any other) element has more than one ref inside removeDetachedChildren,
addChildNodesToDeletionQueue calls notifyChildNodeRemoved in the tree oreder.
However, when a form associated element of this form element appears later in the tree order,
FormAssociatedElement::formOwnerRemovedFromTree can traverse up ancestors including the ShadowRoot.
Fixed the bug by using raw pointers instead. Luckily, there is no DOM mutations or other non-trivial
operations happening in this function so this should be safe.
Test: imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html
* html/FormAssociatedElement.cpp:
(WebCore::FormAssociatedElement::formOwnerRemovedFromTree): Fixed the bug.
LayoutTests:
Unskip the test now that it doesn't hit a debug assertion.
* TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (235955 => 235956)
--- trunk/LayoutTests/ChangeLog 2018-09-12 22:26:12 UTC (rev 235955)
+++ trunk/LayoutTests/ChangeLog 2018-09-12 22:36:51 UTC (rev 235956)
@@ -1,3 +1,14 @@
+2018-09-11 Ryosuke Niwa <[email protected]>
+
+ imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html hits assertion
+ https://bugs.webkit.org/show_bug.cgi?id=189493
+
+ Reviewed by Alex Christensen.
+
+ Unskip the test now that it doesn't hit a debug assertion.
+
+ * TestExpectations:
+
2018-09-12 Dan Bernstein <[email protected]>
[Cocoa] Complete support for Paste as Quotation
Modified: trunk/LayoutTests/TestExpectations (235955 => 235956)
--- trunk/LayoutTests/TestExpectations 2018-09-12 22:26:12 UTC (rev 235955)
+++ trunk/LayoutTests/TestExpectations 2018-09-12 22:36:51 UTC (rev 235956)
@@ -2237,8 +2237,6 @@
webkit.org/b/185308 legacy-animation-engine/animations/combo-transform-translate+scale.html [ Pass Failure ]
-webkit.org/b/189493 imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html [ Skip ]
-
fast/gradients/conic-repeating.html [ Skip ]
fast/gradients/conic.html [ Skip ]
fast/gradients/conic-off-center.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (235955 => 235956)
--- trunk/Source/WebCore/ChangeLog 2018-09-12 22:26:12 UTC (rev 235955)
+++ trunk/Source/WebCore/ChangeLog 2018-09-12 22:36:51 UTC (rev 235956)
@@ -1,3 +1,26 @@
+2018-09-11 Ryosuke Niwa <[email protected]>
+
+ imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html hits assertion
+ https://bugs.webkit.org/show_bug.cgi?id=189493
+
+ Reviewed by Alex Christensen.
+
+ The debug assertion was caused by RefPtr in FormAssociatedElement::formOwnerRemovedFromTree introduced
+ by r224390 and r223644 ref'ing ShadowRoot while calling removeDetachedChildren inside ~ShadowRoot.
+ When a form (or any other) element has more than one ref inside removeDetachedChildren,
+ addChildNodesToDeletionQueue calls notifyChildNodeRemoved in the tree oreder.
+
+ However, when a form associated element of this form element appears later in the tree order,
+ FormAssociatedElement::formOwnerRemovedFromTree can traverse up ancestors including the ShadowRoot.
+
+ Fixed the bug by using raw pointers instead. Luckily, there is no DOM mutations or other non-trivial
+ operations happening in this function so this should be safe.
+
+ Test: imported/w3c/web-platform-tests/shadow-dom/form-control-form-attribute.html
+
+ * html/FormAssociatedElement.cpp:
+ (WebCore::FormAssociatedElement::formOwnerRemovedFromTree): Fixed the bug.
+
2018-09-12 Dan Bernstein <[email protected]>
[Cocoa] Complete support for Paste as Quotation
Modified: trunk/Source/WebCore/html/FormAssociatedElement.cpp (235955 => 235956)
--- trunk/Source/WebCore/html/FormAssociatedElement.cpp 2018-09-12 22:26:12 UTC (rev 235955)
+++ trunk/Source/WebCore/html/FormAssociatedElement.cpp 2018-09-12 22:36:51 UTC (rev 235956)
@@ -122,8 +122,9 @@
void FormAssociatedElement::formOwnerRemovedFromTree(const Node& formRoot)
{
ASSERT(m_form);
- RefPtr<Node> rootNode = &asHTMLElement();
- for (auto ancestor = makeRefPtr(asHTMLElement().parentNode()); ancestor; ancestor = ancestor->parentNode()) {
+ // Can't use RefPtr here beacuse this function might be called inside ~ShadowRoot via addChildNodesToDeletionQueue. See webkit.org/b/189493.
+ Node* rootNode = &asHTMLElement();
+ for (auto* ancestor = asHTMLElement().parentNode(); ancestor; ancestor = ancestor->parentNode()) {
if (ancestor == m_form) {
// Form is our ancestor so we don't need to reset our owner, we also no longer
// need an id observer since we are no longer connected.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
