Title: [208378] trunk/Source/WebCore
- Revision
- 208378
- Author
- [email protected]
- Date
- 2016-11-03 23:21:06 -0700 (Thu, 03 Nov 2016)
Log Message
REGRESSION (r207717): DumpRenderTree crashed in com.apple.WebCore: WebCore::Style::Scope::flushPendingUpdate + 16
https://bugs.webkit.org/show_bug.cgi?id=164397
<rdar://problem/29100135>
Reviewed by Ryosuke Niwa.
The problem here was that we were leaving stale pointers to Document::m_inDocumentShadowRoots set when
using fast-path document teardown.
(Patch and stories mostly by rniwa).
* dom/Document.cpp:
(WebCore::Document::~Document):
(WebCore::Document::didInsertInDocumentShadowRoot):
(WebCore::Document::didRemoveInDocumentShadowRoot):
Improve asserts.
* dom/Element.cpp:
(WebCore::Element::removeShadowRoot):
Remove the superfluous call to notifyChildNodeRemoved in Element::removeShadowRoot to
avoid invoking notifyChildNodeRemoved during a document teardown, which is incorrect. It's sufficient that
~ShadowRoot calls ContainerNode::removeDetachedChildren(), and in turn removeDetachedChildrenInContainer()
since the latter function tears down nodes via the deletion queue during a document destruction and use
notifyChildNodeRemoved() on nodes that outlive the shadow root.
* dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::~ShadowRoot):
Take care to clean up inDocumentShadowRoots for fast-pathed destruction too.
(WebCore::ShadowRoot::insertedInto):
(WebCore::ShadowRoot::removedFrom):
Improve ShadowRoot's insertedInto and removedFrom so that they only try to add and remove itself from
m_inDocumentShadowRoots when the connected-ness changes.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (208377 => 208378)
--- trunk/Source/WebCore/ChangeLog 2016-11-04 06:18:01 UTC (rev 208377)
+++ trunk/Source/WebCore/ChangeLog 2016-11-04 06:21:06 UTC (rev 208378)
@@ -1,3 +1,43 @@
+2016-11-03 Antti Koivisto <[email protected]>
+
+ REGRESSION (r207717): DumpRenderTree crashed in com.apple.WebCore: WebCore::Style::Scope::flushPendingUpdate + 16
+ https://bugs.webkit.org/show_bug.cgi?id=164397
+ <rdar://problem/29100135>
+
+ Reviewed by Ryosuke Niwa.
+
+ The problem here was that we were leaving stale pointers to Document::m_inDocumentShadowRoots set when
+ using fast-path document teardown.
+
+ (Patch and stories mostly by rniwa).
+
+ * dom/Document.cpp:
+ (WebCore::Document::~Document):
+ (WebCore::Document::didInsertInDocumentShadowRoot):
+ (WebCore::Document::didRemoveInDocumentShadowRoot):
+
+ Improve asserts.
+
+ * dom/Element.cpp:
+ (WebCore::Element::removeShadowRoot):
+
+ Remove the superfluous call to notifyChildNodeRemoved in Element::removeShadowRoot to
+ avoid invoking notifyChildNodeRemoved during a document teardown, which is incorrect. It's sufficient that
+ ~ShadowRoot calls ContainerNode::removeDetachedChildren(), and in turn removeDetachedChildrenInContainer()
+ since the latter function tears down nodes via the deletion queue during a document destruction and use
+ notifyChildNodeRemoved() on nodes that outlive the shadow root.
+
+ * dom/ShadowRoot.cpp:
+ (WebCore::ShadowRoot::~ShadowRoot):
+
+ Take care to clean up inDocumentShadowRoots for fast-pathed destruction too.
+
+ (WebCore::ShadowRoot::insertedInto):
+ (WebCore::ShadowRoot::removedFrom):
+
+ Improve ShadowRoot's insertedInto and removedFrom so that they only try to add and remove itself from
+ m_inDocumentShadowRoots when the connected-ness changes.
+
2016-11-03 Simon Fraser <[email protected]>
Give all the geometry classes a single-argument scale() function for consistency
Modified: trunk/Source/WebCore/dom/Document.cpp (208377 => 208378)
--- trunk/Source/WebCore/dom/Document.cpp 2016-11-04 06:18:01 UTC (rev 208377)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-11-04 06:21:06 UTC (rev 208378)
@@ -591,6 +591,7 @@
ASSERT(m_ranges.isEmpty());
ASSERT(!m_parentTreeScope);
ASSERT(!m_disabledFieldsetElementsCount);
+ ASSERT(m_inDocumentShadowRoots.isEmpty());
#if ENABLE(DEVICE_ORIENTATION) && PLATFORM(IOS)
m_deviceMotionClient->deviceMotionControllerDestroyed();
@@ -6980,6 +6981,7 @@
void Document::didInsertInDocumentShadowRoot(ShadowRoot& shadowRoot)
{
ASSERT(shadowRoot.inDocument());
+ ASSERT(!m_inDocumentShadowRoots.contains(&shadowRoot));
m_inDocumentShadowRoots.add(&shadowRoot);
}
Modified: trunk/Source/WebCore/dom/Element.cpp (208377 => 208378)
--- trunk/Source/WebCore/dom/Element.cpp 2016-11-04 06:18:01 UTC (rev 208377)
+++ trunk/Source/WebCore/dom/Element.cpp 2016-11-04 06:21:06 UTC (rev 208378)
@@ -1784,8 +1784,6 @@
oldRoot->setHost(nullptr);
oldRoot->setParentTreeScope(&document());
-
- notifyChildNodeRemoved(*this, *oldRoot);
}
static bool canAttachAuthorShadowRoot(const Element& element)
Modified: trunk/Source/WebCore/dom/ShadowRoot.cpp (208377 => 208378)
--- trunk/Source/WebCore/dom/ShadowRoot.cpp 2016-11-04 06:18:01 UTC (rev 208377)
+++ trunk/Source/WebCore/dom/ShadowRoot.cpp 2016-11-04 06:21:06 UTC (rev 208378)
@@ -70,6 +70,9 @@
ShadowRoot::~ShadowRoot()
{
+ if (inDocument())
+ document().didRemoveInDocumentShadowRoot(*this);
+
// We cannot let ContainerNode destructor call willBeDeletedFrom()
// for this ShadowRoot instance because TreeScope destructor
// clears Node::m_treeScope thus ContainerNode is no longer able
@@ -84,17 +87,18 @@
Node::InsertionNotificationRequest ShadowRoot::insertedInto(ContainerNode& insertionPoint)
{
- auto result = DocumentFragment::insertedInto(insertionPoint);
- if (inDocument())
+ bool wasInDocument = inDocument();
+ DocumentFragment::insertedInto(insertionPoint);
+ if (insertionPoint.inDocument() && !wasInDocument)
document().didInsertInDocumentShadowRoot(*this);
- return result;
+ return InsertionDone;
}
void ShadowRoot::removedFrom(ContainerNode& insertionPoint)
{
- if (inDocument())
+ DocumentFragment::removedFrom(insertionPoint);
+ if (insertionPoint.inDocument() && !inDocument())
document().didRemoveInDocumentShadowRoot(*this);
- DocumentFragment::removedFrom(insertionPoint);
}
Style::Scope& ShadowRoot::styleScope()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
