- Revision
- 225868
- Author
- [email protected]
- Date
- 2017-12-13 13:12:42 -0800 (Wed, 13 Dec 2017)
Log Message
Update the SVG use element's shadow trees explicitly before the style recall
https://bugs.webkit.org/show_bug.cgi?id=180729
<rdar://problem/36009806>
Reviewed by Antti Koivisto.
Update the SVG use element's shadow trees before start resolving styles. Document now has a hash set of all
connected SVG use elements with invalidated shadow trees. SVGUseElement adds itself to this set when its
shadow tree gets invalidated, or it gets newly connected to a document with the invalidated shadow tree.
SVGUseElement removes itself from this set when it updates its shadow tree or it gets disconnected from
a document with the invalidated shadow tree.
No new tests. Covered by existing tests.
* dom/Document.cpp:
(WebCore::Document::~Document): Assert that m_svgUseElements has been cleared.
(WebCore::Document::resolveStyle): Update the shadow trees of SVG use elements with invalidated shadow trees.
(WebCore::Document::addSVGUseElement): Added.
(WebCore::Document::removeSVGUseElement): Added.
* dom/Document.h:
(WebCore::Document::svgUseElements const): Added.
* dom/Element.cpp:
(WebCore::Element::cloneElementWithChildren): Removed EventAllowedScope since the SVG use element's shadow
tree is no longer updated when there is a NoEventDispatchAssertion in the stack.
(WebCore::Element::cloneElementWithoutChildren): Ditto.
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveComposedTree): No longer drops the assertion now that SVG use element's
shadow tree is updated before calling this function.
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::insertedIntoAncestor): Add the element to the document's hash set if this use element's
shadow tree needs to be updated (m_shadowTreeNeedsUpdate is set), and it got newly connected.
(WebCore::SVGUseElement::removedFromAncestor): Ditto for removal. We have to check m_shadowTreeNeedsUpdate before
calling SVGGraphicsElement::removedFromAncestor or updateExternalDocument since either function can invoke
m_shadowTreeNeedsUpdate to true.
(WebCore::SVGUseElement::willRecalcStyle): Deleted. We no longer call updateShadowTree during style recalc.
(WebCore::SVGUseElement::updateShadowTree): Remove this element from the document's hah set. We can't clear all
the entries in the map at once in Document::resolveStyle because updating the shadow trees of a SVG use element
can result in a sync IPC in ImageLoader::updateFromElement, which end up executing arbitrary author scripts.
(WebCore::SVGUseElement::cloneTarget const): Removed EventAllowedScope since the SVG use element's shadow tree
is no longer updated when there is a NoEventDispatchAssertion in the stack.
(WebCore::SVGUseElement::expandUseElementsInShadowTree const): Ditto.
(WebCore::SVGUseElement::expandSymbolElementsInShadowTree const): Ditto.
(WebCore::SVGUseElement::invalidateShadowTree): Add the element to the document's hash set if it's connected.
* svg/SVGUseElement.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (225867 => 225868)
--- trunk/Source/WebCore/ChangeLog 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/ChangeLog 2017-12-13 21:12:42 UTC (rev 225868)
@@ -1,3 +1,50 @@
+2017-12-13 Ryosuke Niwa <[email protected]>
+
+ Update the SVG use element's shadow trees explicitly before the style recall
+ https://bugs.webkit.org/show_bug.cgi?id=180729
+ <rdar://problem/36009806>
+
+ Reviewed by Antti Koivisto.
+
+ Update the SVG use element's shadow trees before start resolving styles. Document now has a hash set of all
+ connected SVG use elements with invalidated shadow trees. SVGUseElement adds itself to this set when its
+ shadow tree gets invalidated, or it gets newly connected to a document with the invalidated shadow tree.
+ SVGUseElement removes itself from this set when it updates its shadow tree or it gets disconnected from
+ a document with the invalidated shadow tree.
+
+ No new tests. Covered by existing tests.
+
+ * dom/Document.cpp:
+ (WebCore::Document::~Document): Assert that m_svgUseElements has been cleared.
+ (WebCore::Document::resolveStyle): Update the shadow trees of SVG use elements with invalidated shadow trees.
+ (WebCore::Document::addSVGUseElement): Added.
+ (WebCore::Document::removeSVGUseElement): Added.
+ * dom/Document.h:
+ (WebCore::Document::svgUseElements const): Added.
+ * dom/Element.cpp:
+ (WebCore::Element::cloneElementWithChildren): Removed EventAllowedScope since the SVG use element's shadow
+ tree is no longer updated when there is a NoEventDispatchAssertion in the stack.
+ (WebCore::Element::cloneElementWithoutChildren): Ditto.
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::resolveComposedTree): No longer drops the assertion now that SVG use element's
+ shadow tree is updated before calling this function.
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::insertedIntoAncestor): Add the element to the document's hash set if this use element's
+ shadow tree needs to be updated (m_shadowTreeNeedsUpdate is set), and it got newly connected.
+ (WebCore::SVGUseElement::removedFromAncestor): Ditto for removal. We have to check m_shadowTreeNeedsUpdate before
+ calling SVGGraphicsElement::removedFromAncestor or updateExternalDocument since either function can invoke
+ m_shadowTreeNeedsUpdate to true.
+ (WebCore::SVGUseElement::willRecalcStyle): Deleted. We no longer call updateShadowTree during style recalc.
+ (WebCore::SVGUseElement::updateShadowTree): Remove this element from the document's hah set. We can't clear all
+ the entries in the map at once in Document::resolveStyle because updating the shadow trees of a SVG use element
+ can result in a sync IPC in ImageLoader::updateFromElement, which end up executing arbitrary author scripts.
+ (WebCore::SVGUseElement::cloneTarget const): Removed EventAllowedScope since the SVG use element's shadow tree
+ is no longer updated when there is a NoEventDispatchAssertion in the stack.
+ (WebCore::SVGUseElement::expandUseElementsInShadowTree const): Ditto.
+ (WebCore::SVGUseElement::expandSymbolElementsInShadowTree const): Ditto.
+ (WebCore::SVGUseElement::invalidateShadowTree): Add the element to the document's hash set if it's connected.
+ * svg/SVGUseElement.h:
+
2017-12-13 Per Arne Vollan <[email protected]>
REGRESSION(225597): Can't select a text box or web view on a page when VO is on.
Modified: trunk/Source/WebCore/dom/Document.cpp (225867 => 225868)
--- trunk/Source/WebCore/dom/Document.cpp 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-12-13 21:12:42 UTC (rev 225868)
@@ -160,6 +160,7 @@
#include "SVGNames.h"
#include "SVGSVGElement.h"
#include "SVGTitleElement.h"
+#include "SVGUseElement.h"
#include "SVGZoomEvent.h"
#include "SWClientConnection.h"
#include "SchemeRegistry.h"
@@ -629,8 +630,9 @@
if (hasRareData())
clearRareData();
- ASSERT(m_listsInvalidatedAtDocument.isEmpty());
- ASSERT(m_collectionsInvalidatedAtDocument.isEmpty());
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_listsInvalidatedAtDocument.isEmpty());
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_collectionsInvalidatedAtDocument.isEmpty());
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_svgUseElements.isEmpty());
for (unsigned count : m_nodeListAndCollectionCounts)
ASSERT_UNUSED(count, !count);
@@ -1787,6 +1789,15 @@
RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());
AnimationUpdateBlock animationUpdateBlock(&m_frame->animation());
+ // FIXME: Do this update per tree scope.
+ {
+ auto elements = copyToVectorOf<RefPtr<SVGUseElement>>(m_svgUseElements);
+ // We can't clear m_svgUseElements here because updateShadowTree may end up executing arbitrary scripts
+ // which may insert new SVG use elements or remove existing ones inside sync IPC via ImageLoader::updateFromElement.
+ for (auto& element : elements)
+ element->updateShadowTree();
+ }
+
// FIXME: We should update style on our ancestor chain before proceeding (especially for seamless),
// however doing so currently causes several tests to crash, as Frame::setDocument calls Document::attach
// before setting the DOMWindow on the Frame, or the SecurityOrigin on the document. The attach, in turn
@@ -5251,6 +5262,18 @@
return *m_svgExtensions;
}
+void Document::addSVGUseElement(SVGUseElement& element)
+{
+ auto result = m_svgUseElements.add(&element);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(result.isNewEntry);
+}
+
+void Document::removeSVGUseElement(SVGUseElement& element)
+{
+ bool didRemove = m_svgUseElements.remove(&element);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(didRemove);
+}
+
bool Document::hasSVGRootNode() const
{
return documentElement() && documentElement()->hasTagName(SVGNames::svgTag);
Modified: trunk/Source/WebCore/dom/Document.h (225867 => 225868)
--- trunk/Source/WebCore/dom/Document.h 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/dom/Document.h 2017-12-13 21:12:42 UTC (rev 225868)
@@ -154,6 +154,7 @@
class RequestAnimationFrameCallback;
class SVGDocumentExtensions;
class SVGSVGElement;
+class SVGUseElement;
class SWClientConnection;
class ScriptElementData;
class ScriptModuleLoader;
@@ -1077,6 +1078,10 @@
WEBCORE_EXPORT const SVGDocumentExtensions* svgExtensions();
WEBCORE_EXPORT SVGDocumentExtensions& accessSVGExtensions();
+ void addSVGUseElement(SVGUseElement&);
+ void removeSVGUseElement(SVGUseElement&);
+ HashSet<SVGUseElement*> const svgUseElements() const { return m_svgUseElements; }
+
void initSecurityContext();
void initContentSecurityPolicy();
@@ -1612,6 +1617,7 @@
RefPtr<XPathEvaluator> m_xpathEvaluator;
std::unique_ptr<SVGDocumentExtensions> m_svgExtensions;
+ HashSet<SVGUseElement*> m_svgUseElements;
#if ENABLE(DASHBOARD_SUPPORT)
Vector<AnnotatedRegionValue> m_annotatedRegions;
Modified: trunk/Source/WebCore/dom/Element.cpp (225867 => 225868)
--- trunk/Source/WebCore/dom/Element.cpp 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/dom/Element.cpp 2017-12-13 21:12:42 UTC (rev 225868)
@@ -349,11 +349,6 @@
Ref<Element> Element::cloneElementWithChildren(Document& targetDocument)
{
Ref<Element> clone = cloneElementWithoutChildren(targetDocument);
-
- // It's safe to dispatch events on the cloned node since author scripts have no access to it yet.
- // This is needed for SVGUseElement::cloneTarget.
- NoEventDispatchAssertion::EventAllowedScope allowedScope(clone.get());
-
cloneChildNodes(clone);
return clone;
}
@@ -362,10 +357,6 @@
{
Ref<Element> clone = cloneElementWithoutAttributesAndChildren(targetDocument);
- // It's safe to dispatch events on the cloned node since author scripts have no access to it yet.
- // This is needed for SVGUseElement::cloneTarget.
- NoEventDispatchAssertion::EventAllowedScope allowedScope(clone.get());
-
// This will catch HTML elements in the wrong namespace that are not correctly copied.
// This is a sanity check as HTML overloads some of the DOM methods.
ASSERT(isHTMLElement() == clone->isHTMLElement());
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (225867 => 225868)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-12-13 21:12:42 UTC (rev 225868)
@@ -411,9 +411,6 @@
auto it = descendants.begin();
auto end = descendants.end();
- // FIXME: SVG <use> element may cause tree mutations during style recalc.
- it.dropAssertions();
-
while (it != end) {
popParentsToDepth(it.depth());
Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (225867 => 225868)
--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2017-12-13 21:12:42 UTC (rev 225868)
@@ -108,6 +108,8 @@
{
SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument) {
+ if (m_shadowTreeNeedsUpdate)
+ document().addSVGUseElement(*this);
SVGExternalResourcesRequired::insertedIntoDocument(this);
invalidateShadowTree();
updateExternalDocument();
@@ -117,9 +119,17 @@
void SVGUseElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
{
+ // Check m_shadowTreeNeedsUpdate before calling SVGElement::removedFromAncestor which calls SVGElement::invalidateInstances
+ // and SVGUseElement::updateExternalDocument which calls invalidateShadowTree().
+ if (removalType.disconnectedFromDocument) {
+ if (m_shadowTreeNeedsUpdate)
+ document().removeSVGUseElement(*this);
+ }
SVGGraphicsElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
- clearShadowTree();
- updateExternalDocument();
+ if (removalType.disconnectedFromDocument) {
+ clearShadowTree();
+ updateExternalDocument();
+ }
}
inline Document* SVGUseElement::externalDocument() const
@@ -181,14 +191,6 @@
SVGGraphicsElement::svgAttributeChanged(attrName);
}
-void SVGUseElement::willRecalcStyle(Style::Change change)
-{
- // FIXME: Shadow tree should be updated before style recalc.
- if (m_shadowTreeNeedsUpdate)
- updateShadowTree();
- SVGGraphicsElement::willRecalcStyle(change);
-}
-
static HashSet<AtomicString> createAllowedElementSet()
{
// Spec: "Any 'svg', 'symbol', 'g', graphics element or other 'use' is potentially a template object that can be re-used
@@ -234,9 +236,14 @@
// FIXME: It's expensive to re-clone the entire tree every time. We should find a more efficient way to handle this.
clearShadowTree();
- if (isInShadowTree() || !isConnected())
+ if (!isConnected())
return;
+ document().removeSVGUseElement(*this);
+ // FIXME: Enable SVG use elements in shadow trees.
+ if (isInShadowTree())
+ return;
+
String targetID;
auto* target = findTarget(&targetID);
if (!target) {
@@ -245,9 +252,7 @@
}
{
- // Safe because the cloned shadow tree has never been exposed to author scripts.
auto& shadowRoot = ensureUserAgentShadowRoot();
- NoEventDispatchAssertion::EventAllowedScope scope(shadowRoot);
cloneTarget(shadowRoot, *target);
expandUseElementsInShadowTree();
expandSymbolElementsInShadowTree();
@@ -436,8 +441,6 @@
void SVGUseElement::cloneTarget(ContainerNode& container, SVGElement& target) const
{
Ref<SVGElement> targetClone = static_cast<SVGElement&>(target.cloneElementWithChildren(document()).get());
- // Safe because the newy cloned nodes in the shadow tree has not been exposed to author scripts yet.
- NoEventDispatchAssertion::EventAllowedScope scope(targetClone);
associateClonesWithOriginals(targetClone.get(), target);
removeDisallowedElementsFromSubtree(targetClone.get());
removeSymbolElementsFromSubtree(targetClone.get());
@@ -470,8 +473,6 @@
// 'use' element except for x, y, width, height and xlink:href are transferred to the generated 'g' element.
auto replacementClone = SVGGElement::create(document());
- // Safe because the use element's shadow tree is not exposed to author scripts, and we don't fire synchronous events during layout & DOM layout.
- NoEventDispatchAssertion::EventAllowedScope scope(replacementClone);
cloneDataAndChildren(replacementClone.get(), originalClone);
@@ -506,8 +507,6 @@
// 'svg' element will use values of 100% for these attributes.
auto replacementClone = SVGSVGElement::create(document());
- // Safe because the newly created SVG element and the newly created shadow tree has not been exposed to author scripts yet.
- NoEventDispatchAssertion::EventAllowedScope scope(replacementClone);
cloneDataAndChildren(replacementClone.get(), originalClone);
originalClone.parentNode()->replaceChild(replacementClone, originalClone);
@@ -533,6 +532,8 @@
m_shadowTreeNeedsUpdate = true;
invalidateStyleAndRenderersForSubtree();
invalidateDependentShadowTrees();
+ if (isConnected())
+ document().addSVGUseElement(*this);
}
void SVGUseElement::invalidateDependentShadowTrees()
Modified: trunk/Source/WebCore/svg/SVGUseElement.h (225867 => 225868)
--- trunk/Source/WebCore/svg/SVGUseElement.h 2017-12-13 20:54:39 UTC (rev 225867)
+++ trunk/Source/WebCore/svg/SVGUseElement.h 2017-12-13 21:12:42 UTC (rev 225868)
@@ -50,6 +50,8 @@
virtual ~SVGUseElement();
void invalidateShadowTree();
+ bool shadowTreeNeedsUpdate() const { return m_shadowTreeNeedsUpdate; }
+ void updateShadowTree();
RenderElement* rendererClipChild() const;
@@ -62,7 +64,6 @@
void buildPendingResource() override;
void parseAttribute(const QualifiedName&, const AtomicString&) override;
void svgAttributeChanged(const QualifiedName&) override;
- void willRecalcStyle(Style::Change) override;
RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
Path toClipPath() override;
bool haveLoadedRequiredResources() override;
@@ -81,7 +82,6 @@
void cloneTarget(ContainerNode&, SVGElement& target) const;
RefPtr<SVGElement> targetClone() const;
- void updateShadowTree();
void expandUseElementsInShadowTree() const;
void expandSymbolElementsInShadowTree() const;
void transferEventListenersToShadowTree() const;
