<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[179101] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/179101">179101</a></dd>
<dt>Author</dt> <dd>darin@apple.com</dd>
<dt>Date</dt> <dd>2015-01-25 19:22:09 -0800 (Sun, 25 Jan 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Streamline SVGUseElement shadow tree handling and make it use SVGElementInstance less
https://bugs.webkit.org/show_bug.cgi?id=140875
Reviewed by Anders Carlsson.
Refactoring of code that is pretty well covered by existing tests, so
not adding new tests.
Inspired by work Rob Buis did in Blink:
http://src.chromium.org/viewvc/blink?view=revision&revision=173273
Althgouh that is less than half of what ended up in this patch.
* dom/ContainerNode.h: Fixed NoEventDispatchAssertion so it can be
copied without causing an underflow of NoEventDispatchAssertion::s_count.
Made the copy constructor call the default constructor. Also changed it
to be based on ASSERT_DISABLED rather than NDEBUG and tweaked it a bit.
* dom/ElementIteratorAssertions.h: Removed an unnecessary include and
an unnecessary default constructor. Changed to use WTF::Optional instead
of WTF::OwnPtr to handle NoEventDispatchAssertion, which makes this class
copyable and assignable, which in turn makes the iterators based on this
copyable and assignable, which is what I needed in SVGUseElement code.
Also simplified code in a couple places.
* dom/TypedElementDescendantIterator.h:
(WebCore::TypedElementDescendantIteratorAdapter<ElementType>::from):
Fixed an error where the arguments to Traversal::next were passed backwards.
This led to incomplete iteration in SVGUseElement code, and an immediate
assertion failure. Probably could use some unit test coverage, too.
(WebCore::TypedElementDescendantConstIteratorAdapter<ElementType>::from):
Ditto.
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::animatedInstanceRoot): Deleted.
(WebCore::SVGUseElement::transferSizeAttributesToShadowTreeTargetClone):
Removed the originalElement argument, since we can use the correspondingElement
to get back to it. Removed the useElement argument and changed this into a
member function.
(WebCore::SVGUseElement::svgAttributeChanged): Updated for above changes.
(WebCore::subtreeContainsDisallowedElement): Deleted this function, because
it was only used to optimize by not calling removeDisallowedElementsFromSubtree,
but that function is already similarly efficient when called to do nothing, so
the preflight was not useful.
(WebCore::SVGUseElement::clearResourceReferences): Call userAgentShadowRoot
instead of shadowRoot for clarity.
(WebCore::SVGUseElement::buildPendingResource): Pass a reference instead of
a pointer to buildShadowAndInstanceTree, since it's guaranteed to not be null.
(WebCore::SVGUseElement::shadowTreeTargetClone): Added. Returns the SVG element
inside the shadow tree that corresponds to the use element's target.
(WebCore::SVGUseElement::buildShadowAndInstanceTree): Changed argument type
to a reference instead of a pointer. Removed comments explaining why we have
an instance tree, since soon we will not have one. Removed many comments that
simply state the names of the functions they are commenting on and perhaps a tiny
bit more. Changed to not use m_targetElementInstance as much, dealing with the
shadow tree directly instead of through the instance tree.
(WebCore::SVGUseElement::toClipPath): Use shadowTreeTargetClone instead of
getting at the element through m_targetElementInstance.
(WebCore::SVGUseElement::rendererClipChild): Ditto.
(WebCore::removeDisallowedElementsFromSubtree): Removed the inline keyword,
since there's no good reason to inline thif function's body. Improved local
variable names and used a modern for loop. Also moved the comment about why
this function is used here inside the function instead of repeating it at
each call site.
(WebCore::SVGUseElement::buildShadowTree): Changed to take a reference
instead of a pointer. Moved the check to see if the target is disallowed
out of this function and into buildShadowAndInstanceTree, which needs to
handle that failure explicitly. Tightened up the code a bit, using Ref instead
of RefPtr, putting the comment about removeDisallowedElementsFromSubtree into
that function itself, and removing the unneeded subtreeContainsDisallowedElement
check entirely.
(WebCore::SVGUseElement::expandUseElementsInShadowTree): Removed the argument,
getting the shadow tree from the shadowTree function instead. Walk the tree
iteratively instead of recursively, using the descendantsOfType function.
Rearranged and streamlined the logic.
(WebCore::SVGUseElement::expandSymbolElementsInShadowTree): Ditto.
(WebCore::SVGUseElement::transferEventListenersToShadowTree): Ditto.
(WebCore::SVGUseElement::transferAttributesToShadowTreeReplacement): Renamed
this to avoid the term "replaced element", which is not a reasonable way to
refer to the <g> element in the shadow tree that replaces the <use> element.
Changed the argument type to SVGGElement to make it harder to misuse this
function by accident, and made the use element be "this" instead of passing
it as an argument.
(WebCore::SVGUseElement::selfHasRelativeLengths): Call hasRelativeLengths
on the target inside the shadow tree rather than the original target, which
makes more sense anyway, and is straightforward now that we have the
shadowTreeTargetClone function. Removes use of m_targetElementInstance here.
* svg/SVGUseElement.h: Updated for above changes.
* svg/SVGUseElement.idl: Removed animatedInstanceRoot and tweaked formatting.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomContainerNodeh">trunk/Source/WebCore/dom/ContainerNode.h</a></li>
<li><a href="#trunkSourceWebCoredomElementIteratorAssertionsh">trunk/Source/WebCore/dom/ElementIteratorAssertions.h</a></li>
<li><a href="#trunkSourceWebCoredomTypedElementDescendantIteratorh">trunk/Source/WebCore/dom/TypedElementDescendantIterator.h</a></li>
<li><a href="#trunkSourceWebCoresvgSVGUseElementcpp">trunk/Source/WebCore/svg/SVGUseElement.cpp</a></li>
<li><a href="#trunkSourceWebCoresvgSVGUseElementh">trunk/Source/WebCore/svg/SVGUseElement.h</a></li>
<li><a href="#trunkSourceWebCoresvgSVGUseElementidl">trunk/Source/WebCore/svg/SVGUseElement.idl</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/ChangeLog 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -1,3 +1,98 @@
</span><ins>+2015-01-25 Darin Adler <darin@apple.com>
+
+ Streamline SVGUseElement shadow tree handling and make it use SVGElementInstance less
+ https://bugs.webkit.org/show_bug.cgi?id=140875
+
+ Reviewed by Anders Carlsson.
+
+ Refactoring of code that is pretty well covered by existing tests, so
+ not adding new tests.
+
+ Inspired by work Rob Buis did in Blink:
+
+ http://src.chromium.org/viewvc/blink?view=revision&revision=173273
+
+ Althgouh that is less than half of what ended up in this patch.
+
+ * dom/ContainerNode.h: Fixed NoEventDispatchAssertion so it can be
+ copied without causing an underflow of NoEventDispatchAssertion::s_count.
+ Made the copy constructor call the default constructor. Also changed it
+ to be based on ASSERT_DISABLED rather than NDEBUG and tweaked it a bit.
+
+ * dom/ElementIteratorAssertions.h: Removed an unnecessary include and
+ an unnecessary default constructor. Changed to use WTF::Optional instead
+ of WTF::OwnPtr to handle NoEventDispatchAssertion, which makes this class
+ copyable and assignable, which in turn makes the iterators based on this
+ copyable and assignable, which is what I needed in SVGUseElement code.
+ Also simplified code in a couple places.
+
+ * dom/TypedElementDescendantIterator.h:
+ (WebCore::TypedElementDescendantIteratorAdapter<ElementType>::from):
+ Fixed an error where the arguments to Traversal::next were passed backwards.
+ This led to incomplete iteration in SVGUseElement code, and an immediate
+ assertion failure. Probably could use some unit test coverage, too.
+ (WebCore::TypedElementDescendantConstIteratorAdapter<ElementType>::from):
+ Ditto.
+
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::animatedInstanceRoot): Deleted.
+ (WebCore::SVGUseElement::transferSizeAttributesToShadowTreeTargetClone):
+ Removed the originalElement argument, since we can use the correspondingElement
+ to get back to it. Removed the useElement argument and changed this into a
+ member function.
+ (WebCore::SVGUseElement::svgAttributeChanged): Updated for above changes.
+ (WebCore::subtreeContainsDisallowedElement): Deleted this function, because
+ it was only used to optimize by not calling removeDisallowedElementsFromSubtree,
+ but that function is already similarly efficient when called to do nothing, so
+ the preflight was not useful.
+ (WebCore::SVGUseElement::clearResourceReferences): Call userAgentShadowRoot
+ instead of shadowRoot for clarity.
+ (WebCore::SVGUseElement::buildPendingResource): Pass a reference instead of
+ a pointer to buildShadowAndInstanceTree, since it's guaranteed to not be null.
+ (WebCore::SVGUseElement::shadowTreeTargetClone): Added. Returns the SVG element
+ inside the shadow tree that corresponds to the use element's target.
+ (WebCore::SVGUseElement::buildShadowAndInstanceTree): Changed argument type
+ to a reference instead of a pointer. Removed comments explaining why we have
+ an instance tree, since soon we will not have one. Removed many comments that
+ simply state the names of the functions they are commenting on and perhaps a tiny
+ bit more. Changed to not use m_targetElementInstance as much, dealing with the
+ shadow tree directly instead of through the instance tree.
+ (WebCore::SVGUseElement::toClipPath): Use shadowTreeTargetClone instead of
+ getting at the element through m_targetElementInstance.
+ (WebCore::SVGUseElement::rendererClipChild): Ditto.
+ (WebCore::removeDisallowedElementsFromSubtree): Removed the inline keyword,
+ since there's no good reason to inline thif function's body. Improved local
+ variable names and used a modern for loop. Also moved the comment about why
+ this function is used here inside the function instead of repeating it at
+ each call site.
+ (WebCore::SVGUseElement::buildShadowTree): Changed to take a reference
+ instead of a pointer. Moved the check to see if the target is disallowed
+ out of this function and into buildShadowAndInstanceTree, which needs to
+ handle that failure explicitly. Tightened up the code a bit, using Ref instead
+ of RefPtr, putting the comment about removeDisallowedElementsFromSubtree into
+ that function itself, and removing the unneeded subtreeContainsDisallowedElement
+ check entirely.
+ (WebCore::SVGUseElement::expandUseElementsInShadowTree): Removed the argument,
+ getting the shadow tree from the shadowTree function instead. Walk the tree
+ iteratively instead of recursively, using the descendantsOfType function.
+ Rearranged and streamlined the logic.
+ (WebCore::SVGUseElement::expandSymbolElementsInShadowTree): Ditto.
+ (WebCore::SVGUseElement::transferEventListenersToShadowTree): Ditto.
+ (WebCore::SVGUseElement::transferAttributesToShadowTreeReplacement): Renamed
+ this to avoid the term "replaced element", which is not a reasonable way to
+ refer to the <g> element in the shadow tree that replaces the <use> element.
+ Changed the argument type to SVGGElement to make it harder to misuse this
+ function by accident, and made the use element be "this" instead of passing
+ it as an argument.
+ (WebCore::SVGUseElement::selfHasRelativeLengths): Call hasRelativeLengths
+ on the target inside the shadow tree rather than the original target, which
+ makes more sense anyway, and is straightforward now that we have the
+ shadowTreeTargetClone function. Removes use of m_targetElementInstance here.
+
+ * svg/SVGUseElement.h: Updated for above changes.
+
+ * svg/SVGUseElement.idl: Removed animatedInstanceRoot and tweaked formatting.
+
</ins><span class="cx"> 2015-01-25 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove 'font' shorthand property special casing
</span></span></pre></div>
<a id="trunkSourceWebCoredomContainerNodeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ContainerNode.h (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ContainerNode.h 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/dom/ContainerNode.h 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -2,7 +2,7 @@
</span><span class="cx"> * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
</span><span class="cx"> * (C) 1999 Antti Koivisto (koivisto@kde.org)
</span><span class="cx"> * (C) 2001 Dirk Mueller (mueller@kde.org)
</span><del>- * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010, 2011 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2004-2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="cx"> * modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -47,16 +47,21 @@
</span><span class="cx"> public:
</span><span class="cx"> NoEventDispatchAssertion()
</span><span class="cx"> {
</span><del>-#ifndef NDEBUG
</del><ins>+#if !ASSERT_DISABLED
</ins><span class="cx"> if (!isMainThread())
</span><span class="cx"> return;
</span><del>- s_count++;
</del><ins>+ ++s_count;
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ NoEventDispatchAssertion(const NoEventDispatchAssertion&)
+ : NoEventDispatchAssertion()
+ {
+ }
+
</ins><span class="cx"> ~NoEventDispatchAssertion()
</span><span class="cx"> {
</span><del>-#ifndef NDEBUG
</del><ins>+#if !ASSERT_DISABLED
</ins><span class="cx"> if (!isMainThread())
</span><span class="cx"> return;
</span><span class="cx"> ASSERT(s_count);
</span><span class="lines">@@ -64,18 +69,20 @@
</span><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#ifndef NDEBUG
</del><span class="cx"> static bool isEventDispatchForbidden()
</span><span class="cx"> {
</span><del>- if (!isMainThread())
- return false;
- return s_count;
</del><ins>+#if ASSERT_DISABLED
+ return false;
+#else
+ return isMainThread() && s_count;
+#endif
</ins><span class="cx"> }
</span><del>-#endif
</del><span class="cx">
</span><ins>+#if !ASSERT_DISABLED
+
</ins><span class="cx"> private:
</span><del>-#ifndef NDEBUG
</del><span class="cx"> WEBCORE_EXPORT static unsigned s_count;
</span><ins>+
</ins><span class="cx"> #endif
</span><span class="cx"> };
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementIteratorAssertionsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ElementIteratorAssertions.h (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ElementIteratorAssertions.h 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/dom/ElementIteratorAssertions.h 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2013 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -26,45 +26,40 @@
</span><span class="cx"> #ifndef ElementIteratorAssertions_h
</span><span class="cx"> #define ElementIteratorAssertions_h
</span><span class="cx">
</span><del>-#include "Document.h"
</del><span class="cx"> #include "Element.h"
</span><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> class ElementIteratorAssertions {
</span><span class="cx"> public:
</span><del>- ElementIteratorAssertions();
- ElementIteratorAssertions(const Element* first);
</del><ins>+ ElementIteratorAssertions(const Element* first = nullptr);
</ins><span class="cx"> bool domTreeHasMutated() const;
</span><span class="cx"> void dropEventDispatchAssertion();
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> const Document* m_document;
</span><span class="cx"> uint64_t m_initialDOMTreeVersion;
</span><del>- OwnPtr<NoEventDispatchAssertion> m_noEventDispatchAssertion;
</del><ins>+ Optional<NoEventDispatchAssertion> m_eventDispatchAssertion;
</ins><span class="cx"> };
</span><span class="cx">
</span><del>-inline ElementIteratorAssertions::ElementIteratorAssertions()
- : m_document(nullptr)
- , m_initialDOMTreeVersion(0)
-{
-}
</del><ins>+// FIXME: No real point in doing these as inlines; they are for debugging and we usually turn off inlining in debug builds.
</ins><span class="cx">
</span><span class="cx"> inline ElementIteratorAssertions::ElementIteratorAssertions(const Element* first)
</span><span class="cx"> : m_document(first ? &first->document() : nullptr)
</span><del>- , m_initialDOMTreeVersion(m_document ? m_document->domTreeVersion() : 0)
- , m_noEventDispatchAssertion(m_document ? adoptPtr(new NoEventDispatchAssertion) : nullptr)
</del><ins>+ , m_initialDOMTreeVersion(first ? m_document->domTreeVersion() : 0)
</ins><span class="cx"> {
</span><ins>+ if (first)
+ m_eventDispatchAssertion = NoEventDispatchAssertion();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline bool ElementIteratorAssertions::domTreeHasMutated() const
</span><span class="cx"> {
</span><del>- return m_initialDOMTreeVersion && m_document && m_document->domTreeVersion() != m_initialDOMTreeVersion;
</del><ins>+ return m_document && m_document->domTreeVersion() != m_initialDOMTreeVersion;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline void ElementIteratorAssertions::dropEventDispatchAssertion()
</span><span class="cx"> {
</span><del>- m_noEventDispatchAssertion = nullptr;
</del><ins>+ m_eventDispatchAssertion = Nullopt;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoredomTypedElementDescendantIteratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/TypedElementDescendantIterator.h (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/TypedElementDescendantIterator.h 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/dom/TypedElementDescendantIterator.h 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2013 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -155,7 +155,7 @@
</span><span class="cx"> ASSERT(descendant.isDescendantOf(&m_root));
</span><span class="cx"> if (is<ElementType>(descendant))
</span><span class="cx"> return TypedElementDescendantIterator<ElementType>(m_root, downcast<ElementType>(&descendant));
</span><del>- ElementType* next = Traversal<ElementType>::next(&m_root, &descendant);
</del><ins>+ ElementType* next = Traversal<ElementType>::next(&descendant, &m_root);
</ins><span class="cx"> return TypedElementDescendantIterator<ElementType>(m_root, next);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -204,7 +204,7 @@
</span><span class="cx"> ASSERT(descendant.isDescendantOf(&m_root));
</span><span class="cx"> if (is<ElementType>(descendant))
</span><span class="cx"> return TypedElementDescendantConstIterator<ElementType>(m_root, downcast<ElementType>(&descendant));
</span><del>- const ElementType* next = Traversal<ElementType>::next(&m_root, &descendant);
</del><ins>+ const ElementType* next = Traversal<ElementType>::next(&descendant, &m_root);
</ins><span class="cx"> return TypedElementDescendantConstIterator<ElementType>(m_root, next);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGUseElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -5,6 +5,7 @@
</span><span class="cx"> * Copyright (C) 2011 Torch Mobile (Beijing) Co. Ltd. All rights reserved.
</span><span class="cx"> * Copyright (C) 2012 University of Szeged
</span><span class="cx"> * Copyright (C) 2012 Renata Hodovan <reni@webkit.org>
</span><ins>+ * Copyright (C) 2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="cx"> * modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -115,12 +116,6 @@
</span><span class="cx"> return m_targetElementInstance.get();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-SVGElementInstance* SVGUseElement::animatedInstanceRoot() const
-{
- // FIXME: Implement me.
- return 0;
-}
-
</del><span class="cx"> bool SVGUseElement::isSupportedAttribute(const QualifiedName& attrName)
</span><span class="cx"> {
</span><span class="cx"> static NeverDestroyed<HashSet<QualifiedName>> supportedAttributes;
</span><span class="lines">@@ -213,7 +208,7 @@
</span><span class="cx"> return 0;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static void updateWidthAndHeight(SVGElement& shadowElement, const SVGUseElement& useElement, const SVGElement& originalElement)
</del><ins>+void SVGUseElement::transferSizeAttributesToShadowTreeTargetClone(SVGElement& shadowElement) const
</ins><span class="cx"> {
</span><span class="cx"> // FIXME: The check for valueInSpecifiedUnits being non-zero below is a workaround for the fact
</span><span class="cx"> // that we currently have no good way to tell whether a particular animatable attribute is a value
</span><span class="lines">@@ -223,13 +218,13 @@
</span><span class="cx"> // If attributes width and/or height are provided on the 'use' element, then these attributes
</span><span class="cx"> // will be transferred to the generated 'svg'. If attributes width and/or height are not specified,
</span><span class="cx"> // the generated 'svg' element will use values of 100% for these attributes.
</span><del>- shadowElement.setAttribute(SVGNames::widthAttr, (useElement.widthIsValid() && useElement.width().valueInSpecifiedUnits()) ? AtomicString(useElement.width().valueAsString()) : "100%");
- shadowElement.setAttribute(SVGNames::heightAttr, (useElement.heightIsValid() && useElement.height().valueInSpecifiedUnits()) ? AtomicString(useElement.height().valueAsString()) : "100%");
</del><ins>+ shadowElement.setAttribute(SVGNames::widthAttr, (widthIsValid() && width().valueInSpecifiedUnits()) ? AtomicString(width().valueAsString()) : "100%");
+ shadowElement.setAttribute(SVGNames::heightAttr, (heightIsValid() && height().valueInSpecifiedUnits()) ? AtomicString(height().valueAsString()) : "100%");
</ins><span class="cx"> } else if (is<SVGSVGElement>(shadowElement)) {
</span><span class="cx"> // Spec (<use> on <svg>): If attributes width and/or height are provided on the 'use' element, then these
</span><span class="cx"> // values will override the corresponding attributes on the 'svg' in the generated tree.
</span><del>- shadowElement.setAttribute(SVGNames::widthAttr, (useElement.widthIsValid() && useElement.width().valueInSpecifiedUnits()) ? AtomicString(useElement.width().valueAsString()) : originalElement.getAttribute(SVGNames::widthAttr));
- shadowElement.setAttribute(SVGNames::heightAttr, (useElement.heightIsValid() && useElement.height().valueInSpecifiedUnits()) ? AtomicString(useElement.height().valueAsString()) : originalElement.getAttribute(SVGNames::heightAttr));
</del><ins>+ shadowElement.setAttribute(SVGNames::widthAttr, (widthIsValid() && width().valueInSpecifiedUnits()) ? AtomicString(width().valueAsString()) : shadowElement.correspondingElement()->getAttribute(SVGNames::widthAttr));
+ shadowElement.setAttribute(SVGNames::heightAttr, (heightIsValid() && height().valueInSpecifiedUnits()) ? AtomicString(height().valueAsString()) : shadowElement.correspondingElement()->getAttribute(SVGNames::heightAttr));
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -248,8 +243,7 @@
</span><span class="cx"> // FIXME: It's unnecessarily inefficient to do this work any time we change "x" or "y".
</span><span class="cx"> // FIXME: It's unnecessarily inefficient to update both width and height each time either is changed.
</span><span class="cx"> ASSERT(m_targetElementInstance->shadowTreeElement());
</span><del>- ASSERT(m_targetElementInstance->correspondingElement());
- updateWidthAndHeight(*m_targetElementInstance->shadowTreeElement(), *this, *m_targetElementInstance->correspondingElement());
</del><ins>+ transferSizeAttributesToShadowTreeTargetClone(*m_targetElementInstance->shadowTreeElement());
</ins><span class="cx"> }
</span><span class="cx"> if (auto* renderer = this->renderer())
</span><span class="cx"> RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
</span><span class="lines">@@ -329,21 +323,11 @@
</span><span class="cx"> return !allowedElementTags.get().contains<SVGAttributeHashTranslator>(element.tagQName());
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static bool subtreeContainsDisallowedElement(SVGElement& start)
-{
- for (auto& element : descendantsOfType<Element>(start)) {
- if (isDisallowedElement(element))
- return true;
- }
-
- return false;
-}
-
</del><span class="cx"> void SVGUseElement::clearResourceReferences()
</span><span class="cx"> {
</span><span class="cx"> // FIXME: We should try to optimize this, to at least allow partial reclones.
</span><del>- if (ShadowRoot* shadowTreeRootElement = shadowRoot())
- shadowTreeRootElement->removeChildren();
</del><ins>+ if (ShadowRoot* root = userAgentShadowRoot())
+ root->removeChildren();
</ins><span class="cx">
</span><span class="cx"> if (m_targetElementInstance) {
</span><span class="cx"> m_targetElementInstance->detach();
</span><span class="lines">@@ -379,15 +363,23 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (target->isSVGElement()) {
</span><del>- buildShadowAndInstanceTree(downcast<SVGElement>(target));
</del><ins>+ buildShadowAndInstanceTree(downcast<SVGElement>(*target));
</ins><span class="cx"> invalidateDependentShadowTrees();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ASSERT(!m_needsShadowTreeRecreation);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::buildShadowAndInstanceTree(SVGElement* target)
</del><ins>+SVGElement* SVGUseElement::shadowTreeTargetClone() const
</ins><span class="cx"> {
</span><ins>+ auto* root = userAgentShadowRoot();
+ if (!root)
+ return nullptr;
+ return downcast<SVGElement>(root->firstChild());
+}
+
+void SVGUseElement::buildShadowAndInstanceTree(SVGElement& target)
+{
</ins><span class="cx"> ASSERT(!m_targetElementInstance);
</span><span class="cx">
</span><span class="cx"> // Do not build the shadow/instance tree for <use> elements living in a shadow tree.
</span><span class="lines">@@ -396,80 +388,52 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> // Do not allow self-referencing.
</span><del>- // 'target' may be null, if it's a non SVG namespaced element.
- if (!target || target == this)
</del><ins>+ if (&target == this)
</ins><span class="cx"> return;
</span><span class="cx">
</span><del>- // Why a seperated instance/shadow tree? SVG demands it:
- // The instance tree is accesable from JavaScript, and has to
- // expose a 1:1 copy of the referenced tree, whereas internally we need
- // to alter the tree for correct "use-on-symbol", "use-on-svg" support.
-
- // Build instance tree. Create root SVGElementInstance object for the first sub-tree node.
- //
</del><ins>+ // Build instance tree.
</ins><span class="cx"> // Spec: If the 'use' element references a simple graphics element such as a 'rect', then there is only a
</span><span class="cx"> // single SVGElementInstance object, and the correspondingElement attribute on this SVGElementInstance object
</span><span class="cx"> // is the SVGRectElement that corresponds to the referenced 'rect' element.
</span><del>- m_targetElementInstance = SVGElementInstance::create(this, this, target);
</del><ins>+ m_targetElementInstance = SVGElementInstance::create(this, this, &target);
</ins><span class="cx">
</span><span class="cx"> // Eventually enter recursion to build SVGElementInstance objects for the sub-tree children
</span><span class="cx"> bool foundProblem = false;
</span><del>- buildInstanceTree(target, m_targetElementInstance.get(), foundProblem, false);
</del><ins>+ buildInstanceTree(&target, m_targetElementInstance.get(), foundProblem, false);
</ins><span class="cx">
</span><span class="cx"> if (instanceTreeIsLoading(m_targetElementInstance.get()))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- // SVG specification does not say a word about <use> & cycles. My view on this is: just ignore it!
</del><ins>+ // SVG specification does not say a word about <use> and cycles. My view on this is: just ignore it!
</ins><span class="cx"> // Non-appearing <use> content is easier to debug, then half-appearing content.
</span><span class="cx"> if (foundProblem) {
</span><span class="cx"> clearResourceReferences();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // Assure instance tree building was successfull
</del><ins>+ // Assure instance tree building was successful.
</ins><span class="cx"> ASSERT(m_targetElementInstance);
</span><span class="cx"> ASSERT(!m_targetElementInstance->shadowTreeElement());
</span><span class="cx"> ASSERT(m_targetElementInstance->correspondingUseElement() == this);
</span><span class="cx"> ASSERT(m_targetElementInstance->directUseElement() == this);
</span><del>- ASSERT(m_targetElementInstance->correspondingElement() == target);
</del><ins>+ ASSERT(m_targetElementInstance->correspondingElement() == &target);
</ins><span class="cx">
</span><del>- ShadowRoot* shadowTreeRootElement = shadowRoot();
- ASSERT(shadowTreeRootElement);
-
- // Build shadow tree from instance tree
- // This also handles the special cases: <use> on <symbol>, <use> on <svg>.
- buildShadowTree(target, m_targetElementInstance.get());
-
- // Expand all <use> elements in the shadow tree.
- // Expand means: replace the actual <use> element by what it references.
- expandUseElementsInShadowTree(shadowTreeRootElement);
-
- // Expand all <symbol> elements in the shadow tree.
- // Expand means: replace the actual <symbol> element by the <svg> element.
- expandSymbolElementsInShadowTree(shadowTreeRootElement);
-
- // Now that the shadow tree is completly expanded, we can associate
- // shadow tree elements <-> instances in the instance tree.
- associateInstancesWithShadowTreeElements(shadowTreeRootElement->firstChild(), m_targetElementInstance.get());
-
- ASSERT(m_targetElementInstance->shadowTreeElement());
- ASSERT(m_targetElementInstance->correspondingElement() == target);
- updateWidthAndHeight(*m_targetElementInstance->shadowTreeElement(), *this, *target);
-
- // If no shadow tree element is present, this means that the reference root
- // element was removed, as it is disallowed (ie. <use> on <foreignObject>)
- // Do NOT leave an inconsistent instance tree around, instead destruct it.
- if (!m_targetElementInstance->shadowTreeElement()) {
</del><ins>+ if (isDisallowedElement(target)) {
</ins><span class="cx"> clearResourceReferences();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- ASSERT(m_targetElementInstance->shadowTreeElement()->parentNode() == shadowTreeRootElement);
</del><ins>+ buildShadowTree(target);
+ expandUseElementsInShadowTree();
+ expandSymbolElementsInShadowTree();
</ins><span class="cx">
</span><del>- // Transfer event listeners assigned to the referenced element to our shadow tree elements.
- transferEventListenersToShadowTree(m_targetElementInstance.get());
</del><ins>+ ASSERT(shadowTreeTargetClone());
+ SVGElement& shadowTreeTargetClone = *this->shadowTreeTargetClone();
+ associateInstancesWithShadowTreeElements(&shadowTreeTargetClone, m_targetElementInstance.get());
</ins><span class="cx">
</span><del>- // Update relative length information.
</del><ins>+ transferSizeAttributesToShadowTreeTargetClone(shadowTreeTargetClone);
+
+ transferEventListenersToShadowTree();
</ins><span class="cx"> updateRelativeLengthsInformation();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -493,7 +457,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(path.isEmpty());
</span><span class="cx">
</span><del>- SVGElement* element = m_targetElementInstance ? m_targetElementInstance->shadowTreeElement() : nullptr;
</del><ins>+ SVGElement* element = shadowTreeTargetClone();
</ins><span class="cx"> if (is<SVGGraphicsElement>(element)) {
</span><span class="cx"> if (!isDirectReference(*element)) {
</span><span class="cx"> // Spec: Indirect references are an error (14.3.5)
</span><span class="lines">@@ -510,16 +474,11 @@
</span><span class="cx">
</span><span class="cx"> RenderElement* SVGUseElement::rendererClipChild() const
</span><span class="cx"> {
</span><del>- if (!m_targetElementInstance)
- return nullptr;
-
- auto* element = m_targetElementInstance->shadowTreeElement();
</del><ins>+ auto* element = shadowTreeTargetClone();
</ins><span class="cx"> if (!element)
</span><span class="cx"> return nullptr;
</span><del>-
</del><span class="cx"> if (!isDirectReference(*element))
</span><span class="cx"> return nullptr;
</span><del>-
</del><span class="cx"> return element->renderer();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -608,164 +567,132 @@
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void removeDisallowedElementsFromSubtree(SVGElement& subtree)
</del><ins>+static void removeDisallowedElementsFromSubtree(SVGElement& subtree)
</ins><span class="cx"> {
</span><ins>+ // Remove disallowed elements after the fact rather than not cloning them in the first place.
+ // This optimizes for the normal case where none of those elements are present.
+
+ // This function is used only on elements in subtrees that are not yet in documents, so
+ // mutation events are not a factor; there are no event listeners to handle those events.
+ // Assert that it's not in a document to make sure callers are still using it this way.
</ins><span class="cx"> ASSERT(!subtree.inDocument());
</span><del>- Vector<Element*> toRemove;
- auto it = descendantsOfType<Element>(subtree).begin();
- auto end = descendantsOfType<Element>(subtree).end();
- while (it != end) {
</del><ins>+
+ Vector<Element*> disallowedElements;
+ auto descendants = descendantsOfType<Element>(subtree);
+ auto end = descendants.end();
+ for (auto it = descendants.begin(); it != end; ) {
</ins><span class="cx"> if (isDisallowedElement(*it)) {
</span><del>- toRemove.append(&*it);
</del><ins>+ disallowedElements.append(&*it);
</ins><span class="cx"> it.traverseNextSkippingChildren();
</span><span class="cx"> continue;
</span><span class="cx"> }
</span><span class="cx"> ++it;
</span><span class="cx"> }
</span><del>- // The subtree is not in document so this won't generate events that could mutate the tree.
- for (unsigned i = 0; i < toRemove.size(); ++i)
- toRemove[i]->parentNode()->removeChild(toRemove[i]);
</del><ins>+ for (Element* element : disallowedElements)
+ element->parentNode()->removeChild(element);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance)
</del><ins>+void SVGUseElement::buildShadowTree(SVGElement& target)
</ins><span class="cx"> {
</span><del>- ASSERT(target); // FIXME: Don't be a pointer!
-
- // For instance <use> on <foreignObject> (direct case).
- if (isDisallowedElement(*target))
- return;
-
- RefPtr<SVGElement> newChild = static_pointer_cast<SVGElement>(targetInstance->correspondingElement()->cloneElementWithChildren(document()));
-
- // We don't walk the target tree element-by-element, and clone each element,
- // but instead use cloneElementWithChildren(). This is an optimization for the common
- // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
- // Though if there are disallowed elements in the subtree, we have to remove them.
- // For instance: <use> on <g> containing <foreignObject> (indirect case).
- if (subtreeContainsDisallowedElement(*newChild))
- removeDisallowedElementsFromSubtree(*newChild);
-
- shadowRoot()->appendChild(newChild.release());
</del><ins>+ Ref<SVGElement> clonedTarget = static_pointer_cast<SVGElement>(target.cloneElementWithChildren(document())).releaseNonNull();
+ removeDisallowedElementsFromSubtree(clonedTarget.get());
+ ensureUserAgentShadowRoot().appendChild(WTF::move(clonedTarget));
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::expandUseElementsInShadowTree(Node* element)
</del><ins>+void SVGUseElement::expandUseElementsInShadowTree()
</ins><span class="cx"> {
</span><span class="cx"> // Why expand the <use> elements in the shadow tree here, and not just
</span><del>- // do this directly in buildShadowTree, if we encounter a <use> element?
- //
- // Short answer: Because we may miss to expand some elements. Ie. if a <symbol>
- // contains <use> tags, we'd miss them. So once we're done with settin' up the
- // actual shadow tree (after the special case modification for svg/symbol) we have
- // to walk it completely and expand all <use> elements.
- if (is<SVGUseElement>(*element)) {
- SVGUseElement& use = downcast<SVGUseElement>(*element);
- ASSERT(!use.cachedDocumentIsStillLoading());
</del><ins>+ // do this directly in buildShadowTree, as we encounter each <use> element?
+ // Because we might miss expanding some elements if we did it then. If a <symbol>
+ // contained <use> elements, we'd miss those.
</ins><span class="cx">
</span><del>- ASSERT(referencedDocument());
- Element* targetElement = SVGURIReference::targetElementFromIRIString(use.href(), *referencedDocument());
- SVGElement* target = nullptr;
- if (targetElement && targetElement->isSVGElement())
- target = downcast<SVGElement>(targetElement);
</del><ins>+ auto descendants = descendantsOfType<SVGUseElement>(*userAgentShadowRoot());
+ auto end = descendants.end();
+ for (auto it = descendants.begin(); it != end; ) {
+ SVGUseElement& original = *it;
+ it = end; // Efficiently quiets assertions due to the outstanding iterator.
</ins><span class="cx">
</span><del>- // Don't ASSERT(target) here, it may be "pending", too.
- // Setup sub-shadow tree root node
- RefPtr<SVGGElement> cloneParent = SVGGElement::create(SVGNames::gTag, *referencedDocument());
- use.cloneChildNodes(cloneParent.get());
</del><ins>+ ASSERT(!original.cachedDocumentIsStillLoading());
</ins><span class="cx">
</span><span class="cx"> // Spec: In the generated content, the 'use' will be replaced by 'g', where all attributes from the
</span><span class="cx"> // 'use' element except for x, y, width, height and xlink:href are transferred to the generated 'g' element.
</span><del>- transferUseAttributesToReplacedElement(&use, cloneParent.get());
</del><span class="cx">
</span><del>- if (target && !isDisallowedElement(*target)) {
- RefPtr<Element> newChild = target->cloneElementWithChildren(document());
- updateWidthAndHeight(downcast<SVGElement>(*newChild), use, *target);
- cloneParent->appendChild(newChild.release());
</del><ins>+ // FIXME: Is it right to use referencedDocument() here instead of just document()?
+ // Can a shadow tree within this document really contain elements that are in a
+ // different document?
+ ASSERT(referencedDocument());
+ auto replacement = SVGGElement::create(SVGNames::gTag, *referencedDocument());
+
+ original.transferAttributesToShadowTreeReplacement(replacement.get());
+ original.cloneChildNodes(replacement.ptr());
+
+ RefPtr<SVGElement> clonedTarget;
+ Element* targetCandidate = SVGURIReference::targetElementFromIRIString(original.href(), *referencedDocument());
+ if (is<SVGElement>(targetCandidate) && !isDisallowedElement(downcast<SVGElement>(*targetCandidate))) {
+ SVGElement& originalTarget = downcast<SVGElement>(*targetCandidate);
+ clonedTarget = static_pointer_cast<SVGElement>(originalTarget.cloneElementWithChildren(document()));
+ // Set the corresponding element here so transferSizeAttributesToShadowTreeTargetClone
+ // can use it. It will be set again later in associateInstancesWithShadowTreeElements,
+ // but it does no harm to set it twice.
+ clonedTarget->setCorrespondingElement(&originalTarget);
+ replacement->appendChild(clonedTarget);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- // We don't walk the target tree element-by-element, and clone each element,
- // but instead use cloneElementWithChildren(). This is an optimization for the common
- // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
- // Though if there are disallowed elements in the subtree, we have to remove them.
- // For instance: <use> on <g> containing <foreignObject> (indirect case).
- if (subtreeContainsDisallowedElement(*cloneParent))
- removeDisallowedElementsFromSubtree(*cloneParent);
</del><ins>+ removeDisallowedElementsFromSubtree(replacement.get());
</ins><span class="cx">
</span><del>- RefPtr<Node> replacingElement(cloneParent.get());
</del><ins>+ // Replace <use> with the <g> element we created.
+ original.parentNode()->replaceChild(replacement.ptr(), &original);
</ins><span class="cx">
</span><del>- // Replace <use> with referenced content.
- ASSERT(use.parentNode());
- use.parentNode()->replaceChild(cloneParent.release(), &use);
</del><ins>+ // Call transferSizeAttributesToShadowTreeTargetClone after putting the cloned elements into the
+ // shadow tree so it can use SVGElement::correspondingElement without triggering an assertion.
+ if (clonedTarget)
+ original.transferSizeAttributesToShadowTreeTargetClone(*clonedTarget);
</ins><span class="cx">
</span><del>- // Expand the siblings because the *element* is replaced and we will
- // lose the sibling chain when we are back from recursion.
- element = replacingElement.get();
- for (RefPtr<Node> sibling = element->nextSibling(); sibling; sibling = sibling->nextSibling())
- expandUseElementsInShadowTree(sibling.get());
</del><ins>+ // Continue iterating from the <g> element since the <use> element was replaced.
+ it = descendants.from(replacement.get());
</ins><span class="cx"> }
</span><del>-
- for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling())
- expandUseElementsInShadowTree(child.get());
</del><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::expandSymbolElementsInShadowTree(Node* node)
</del><ins>+void SVGUseElement::expandSymbolElementsInShadowTree()
</ins><span class="cx"> {
</span><del>- if (is<SVGSymbolElement>(*node)) {
</del><ins>+ auto descendants = descendantsOfType<SVGSymbolElement>(*userAgentShadowRoot());
+ auto end = descendants.end();
+ for (auto it = descendants.begin(); it != end; ) {
+ SVGSymbolElement& original = *it;
+ it = end; // Efficiently quiets assertions due to the outstanding iterator.
+
</ins><span class="cx"> // Spec: The referenced 'symbol' and its contents are deep-cloned into the generated tree,
</span><span class="cx"> // with the exception that the 'symbol' is replaced by an 'svg'. This generated 'svg' will
</span><span class="cx"> // always have explicit values for attributes width and height. If attributes width and/or
</span><span class="cx"> // height are provided on the 'use' element, then these attributes will be transferred to
</span><span class="cx"> // the generated 'svg'. If attributes width and/or height are not specified, the generated
</span><span class="cx"> // 'svg' element will use values of 100% for these attributes.
</span><del>- RefPtr<SVGSVGElement> svgElement = SVGSVGElement::create(SVGNames::svgTag, *referencedDocument());
</del><span class="cx">
</span><del>- // Transfer all data (attributes, etc.) from <symbol> to the new <svg> element.
- svgElement->cloneDataFromElement(downcast<SVGSymbolElement>(*node));
</del><ins>+ // FIXME: Is it right to use referencedDocument() here instead of just document()?
+ // Can a shadow tree within this document really contain elements that are in a
+ // different document?
+ ASSERT(referencedDocument());
+ auto replacement = SVGSVGElement::create(SVGNames::svgTag, *referencedDocument());
+ replacement->cloneDataFromElement(original);
+ original.cloneChildNodes(replacement.ptr());
+ removeDisallowedElementsFromSubtree(replacement.get());
</ins><span class="cx">
</span><del>- // Only clone symbol children, and add them to the new <svg> element
- for (Node* child = node->firstChild(); child; child = child->nextSibling()) {
- RefPtr<Node> newChild = child->cloneNode(true);
- svgElement->appendChild(newChild.release());
- }
</del><ins>+ // Replace <symbol> with the <svg> element we created.
+ original.parentNode()->replaceChild(replacement.ptr(), &original);
</ins><span class="cx">
</span><del>- // We don't walk the target tree element-by-element, and clone each element,
- // but instead use cloneNode(deep=true). This is an optimization for the common
- // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
- // Though if there are disallowed elements in the subtree, we have to remove them.
- // For instance: <use> on <g> containing <foreignObject> (indirect case).
- if (subtreeContainsDisallowedElement(*svgElement))
- removeDisallowedElementsFromSubtree(*svgElement);
-
- RefPtr<SVGSVGElement> replacingElement(svgElement.get());
-
- // Replace <symbol> with <svg>.
- node->parentNode()->replaceChild(svgElement.release(), node);
-
- // Expand the siblings because the *element* is replaced and we will
- // lose the sibling chain when we are back from recursion.
- node = replacingElement.get();
- for (RefPtr<Node> sibling = node->nextSibling(); sibling; sibling = sibling->nextSibling())
- expandSymbolElementsInShadowTree(sibling.get());
</del><ins>+ // Continue iterating from the <svg> element since the <symbol> element was replaced.
+ it = descendants.from(replacement.get());
</ins><span class="cx"> }
</span><del>-
- for (RefPtr<Node> child = node->firstChild(); child; child = child->nextSibling())
- expandSymbolElementsInShadowTree(child.get());
</del><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::transferEventListenersToShadowTree(SVGElementInstance* target)
</del><ins>+void SVGUseElement::transferEventListenersToShadowTree()
</ins><span class="cx"> {
</span><del>- if (!target)
- return;
-
- SVGElement* originalElement = target->correspondingElement();
- ASSERT(originalElement);
-
- if (SVGElement* shadowTreeElement = target->shadowTreeElement()) {
- if (EventTargetData* data = originalElement->eventTargetData())
- data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(shadowTreeElement);
</del><ins>+ ASSERT(userAgentShadowRoot());
+ for (auto& descendant : descendantsOfType<SVGElement>(*userAgentShadowRoot())) {
+ ASSERT(descendant.correspondingElement());
+ if (EventTargetData* data = descendant.correspondingElement()->eventTargetData())
+ data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(&descendant);
</ins><span class="cx"> }
</span><del>-
- for (SVGElementInstance* instance = target->firstChild(); instance; instance = instance->nextSibling())
- transferEventListenersToShadowTree(instance);
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void SVGUseElement::associateInstancesWithShadowTreeElements(Node* target, SVGElementInstance* targetInstance)
</span><span class="lines">@@ -859,36 +786,24 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void SVGUseElement::transferUseAttributesToReplacedElement(SVGElement* from, SVGElement* to) const
</del><ins>+void SVGUseElement::transferAttributesToShadowTreeReplacement(SVGGElement& replacement) const
</ins><span class="cx"> {
</span><del>- ASSERT(from);
- ASSERT(to);
</del><ins>+ replacement.cloneDataFromElement(*this);
</ins><span class="cx">
</span><del>- to->cloneDataFromElement(*from);
-
- to->removeAttribute(SVGNames::xAttr);
- to->removeAttribute(SVGNames::yAttr);
- to->removeAttribute(SVGNames::widthAttr);
- to->removeAttribute(SVGNames::heightAttr);
- to->removeAttribute(XLinkNames::hrefAttr);
</del><ins>+ replacement.removeAttribute(SVGNames::xAttr);
+ replacement.removeAttribute(SVGNames::yAttr);
+ replacement.removeAttribute(SVGNames::widthAttr);
+ replacement.removeAttribute(SVGNames::heightAttr);
+ replacement.removeAttribute(XLinkNames::hrefAttr);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool SVGUseElement::selfHasRelativeLengths() const
</span><span class="cx"> {
</span><del>- if (x().isRelative()
- || y().isRelative()
- || width().isRelative()
- || height().isRelative())
</del><ins>+ if (x().isRelative() || y().isRelative() || width().isRelative() || height().isRelative())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><del>- if (!m_targetElementInstance)
- return false;
-
- SVGElement* element = m_targetElementInstance->correspondingElement();
- if (!element)
- return false;
-
- return element->hasRelativeLengths();
</del><ins>+ auto* target = shadowTreeTargetClone();
+ return target && target->hasRelativeLengths();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void SVGUseElement::notifyFinished(CachedResource* resource)
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGUseElementh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGUseElement.h (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/svg/SVGUseElement.h 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/svg/SVGUseElement.h 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -1,6 +1,7 @@
</span><span class="cx"> /*
</span><span class="cx"> * Copyright (C) 2004, 2005, 2006, 2007, 2008 Nikolas Zimmermann <zimmermann@kde.org>
</span><span class="cx"> * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
</span><ins>+ * Copyright (C) 2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="cx"> * modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -34,6 +35,7 @@
</span><span class="cx">
</span><span class="cx"> class CachedSVGDocument;
</span><span class="cx"> class SVGElementInstance;
</span><ins>+class SVGGElement;
</ins><span class="cx">
</span><span class="cx"> class SVGUseElement final : public SVGGraphicsElement,
</span><span class="cx"> public SVGExternalResourcesRequired,
</span><span class="lines">@@ -44,7 +46,6 @@
</span><span class="cx"> virtual ~SVGUseElement();
</span><span class="cx">
</span><span class="cx"> SVGElementInstance* instanceRoot();
</span><del>- SVGElementInstance* animatedInstanceRoot() const;
</del><span class="cx"> SVGElementInstance* instanceForShadowTreeElement(Node*) const;
</span><span class="cx"> void invalidateShadowTree();
</span><span class="cx"> void invalidateDependentShadowTrees();
</span><span class="lines">@@ -73,7 +74,7 @@
</span><span class="cx"> virtual void toClipPath(Path&) override;
</span><span class="cx">
</span><span class="cx"> void clearResourceReferences();
</span><del>- void buildShadowAndInstanceTree(SVGElement* target);
</del><ins>+ void buildShadowAndInstanceTree(SVGElement& target);
</ins><span class="cx"> void detachInstance();
</span><span class="cx">
</span><span class="cx"> virtual bool haveLoadedRequiredResources() override { return SVGExternalResourcesRequired::haveLoadedRequiredResources(); }
</span><span class="lines">@@ -85,19 +86,19 @@
</span><span class="cx"> void buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundCycle, bool foundUse);
</span><span class="cx"> bool hasCycleUseReferencing(SVGUseElement*, SVGElementInstance* targetInstance, SVGElement*& newTarget);
</span><span class="cx">
</span><del>- // Shadow tree handling
- void buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance);
</del><ins>+ // Shadow tree handling.
+ void buildShadowTree(SVGElement& target);
+ void expandUseElementsInShadowTree();
+ void expandSymbolElementsInShadowTree();
+ SVGElement* shadowTreeTargetClone() const;
+ void transferEventListenersToShadowTree();
+ void transferAttributesToShadowTreeReplacement(SVGGElement&) const;
+ void transferSizeAttributesToShadowTreeTargetClone(SVGElement&) const;
</ins><span class="cx">
</span><del>- void expandUseElementsInShadowTree(Node* element);
- void expandSymbolElementsInShadowTree(Node* element);
-
</del><span class="cx"> // "Tree connector"
</span><span class="cx"> void associateInstancesWithShadowTreeElements(Node* target, SVGElementInstance* targetInstance);
</span><span class="cx"> SVGElementInstance* instanceForShadowTreeElement(Node* element, SVGElementInstance* instance) const;
</span><span class="cx">
</span><del>- void transferUseAttributesToReplacedElement(SVGElement* from, SVGElement* to) const;
- void transferEventListenersToShadowTree(SVGElementInstance* target);
-
</del><span class="cx"> BEGIN_DECLARE_ANIMATED_PROPERTIES(SVGUseElement)
</span><span class="cx"> DECLARE_ANIMATED_LENGTH(X, x)
</span><span class="cx"> DECLARE_ANIMATED_LENGTH(Y, y)
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGUseElementidl"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGUseElement.idl (179100 => 179101)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/svg/SVGUseElement.idl 2015-01-26 02:11:14 UTC (rev 179100)
+++ trunk/Source/WebCore/svg/SVGUseElement.idl 2015-01-26 03:22:09 UTC (rev 179101)
</span><span class="lines">@@ -24,13 +24,12 @@
</span><span class="cx"> */
</span><span class="cx">
</span><span class="cx"> interface SVGUseElement : SVGGraphicsElement {
</span><del>- readonly attribute SVGAnimatedLength x;
- readonly attribute SVGAnimatedLength y;
- readonly attribute SVGAnimatedLength width;
- readonly attribute SVGAnimatedLength height;
</del><ins>+ readonly attribute SVGAnimatedLength x;
+ readonly attribute SVGAnimatedLength y;
+ readonly attribute SVGAnimatedLength width;
+ readonly attribute SVGAnimatedLength height;
</ins><span class="cx">
</span><span class="cx"> readonly attribute SVGElementInstance instanceRoot;
</span><del>- readonly attribute SVGElementInstance animatedInstanceRoot;
</del><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> SVGUseElement implements SVGExternalResourcesRequired;
</span></span></pre>
</div>
</div>
</body>
</html>