Diff
Modified: trunk/LayoutTests/ChangeLog (122448 => 122449)
--- trunk/LayoutTests/ChangeLog 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/ChangeLog 2012-07-12 12:26:03 UTC (rev 122449)
@@ -1,3 +1,25 @@
+2012-07-12 Florin Malita <fmal...@chromium.org>
+
+ Incorrect handling of chained pending resources in SVGUseElement
+ https://bugs.webkit.org/show_bug.cgi?id=89686
+
+ Reviewed by Nikolas Zimmermann.
+
+ Added reftest to verify that nested/indirect <use> constructs are
+ handled correctly WRT pending resource tracking.
+
+ Also updated the svg/hixie/error/017-expected.txt to match the current
+ results. Per comments in SVGUseElement::buildShadowAndInstanceTree, the
+ new behavior is correct: <use> cycles are to be ignored.
+
+ * platform/chromium-win/svg/hixie/error/017-expected.txt:
+ * platform/efl/svg/hixie/error/017-expected.txt:
+ * platform/gtk/svg/hixie/error/017-expected.txt:
+ * platform/mac/svg/hixie/error/017-expected.txt:
+ * platform/qt/svg/hixie/error/017-expected.txt:
+ * svg/custom/use-nested-expected.svg: Added.
+ * svg/custom/use-nested.svg: Added.
+
2012-07-12 Zan Dobersek <zandober...@gmail.com>
Unreviewed GTK gardening, fixing a typo in test's filename in TestExpectations.
Modified: trunk/LayoutTests/platform/chromium-win/svg/hixie/error/017-expected.txt (122448 => 122449)
--- trunk/LayoutTests/platform/chromium-win/svg/hixie/error/017-expected.txt 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/platform/chromium-win/svg/hixie/error/017-expected.txt 2012-07-12 12:26:03 UTC (rev 122449)
@@ -5,12 +5,8 @@
RenderSVGHiddenContainer {defs} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
Modified: trunk/LayoutTests/platform/efl/svg/hixie/error/017-expected.txt (122448 => 122449)
--- trunk/LayoutTests/platform/efl/svg/hixie/error/017-expected.txt 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/platform/efl/svg/hixie/error/017-expected.txt 2012-07-12 12:26:03 UTC (rev 122449)
@@ -5,12 +5,8 @@
RenderSVGHiddenContainer {defs} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
Modified: trunk/LayoutTests/platform/gtk/svg/hixie/error/017-expected.txt (122448 => 122449)
--- trunk/LayoutTests/platform/gtk/svg/hixie/error/017-expected.txt 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/platform/gtk/svg/hixie/error/017-expected.txt 2012-07-12 12:26:03 UTC (rev 122449)
@@ -5,12 +5,8 @@
RenderSVGHiddenContainer {defs} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
Modified: trunk/LayoutTests/platform/mac/svg/hixie/error/017-expected.txt (122448 => 122449)
--- trunk/LayoutTests/platform/mac/svg/hixie/error/017-expected.txt 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/platform/mac/svg/hixie/error/017-expected.txt 2012-07-12 12:26:03 UTC (rev 122449)
@@ -5,12 +5,8 @@
RenderSVGHiddenContainer {defs} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
Modified: trunk/LayoutTests/platform/qt/svg/hixie/error/017-expected.txt (122448 => 122449)
--- trunk/LayoutTests/platform/qt/svg/hixie/error/017-expected.txt 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/LayoutTests/platform/qt/svg/hixie/error/017-expected.txt 2012-07-12 12:26:03 UTC (rev 122449)
@@ -5,12 +5,8 @@
RenderSVGHiddenContainer {defs} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
- RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
RenderSVGContainer {use} at (0,0) size 0x0
RenderSVGContainer {g} at (0,0) size 0x0
Added: trunk/LayoutTests/svg/custom/use-nested-expected.svg (0 => 122449)
--- trunk/LayoutTests/svg/custom/use-nested-expected.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/use-nested-expected.svg 2012-07-12 12:26:03 UTC (rev 122449)
@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <rect width="100" height="100" fill="green"/>
+ <rect x="200" width="100" height="100" fill="green"/>
+ <rect y="200" width="100" height="100" fill="green"/>
+ <rect x="200" y="200" width="100" height="100" fill="green"/>
+</svg>
Added: trunk/LayoutTests/svg/custom/use-nested.svg (0 => 122449)
--- trunk/LayoutTests/svg/custom/use-nested.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/use-nested.svg 2012-07-12 12:26:03 UTC (rev 122449)
@@ -0,0 +1,20 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <!-- Test for https://bugs.webkit.org/show_bug.cgi?id=89686 -->
+ <use id="target1" xlink:href=""
+ <use x="200" xlink:href=""
+ <use y="200" xlink:href=""
+
+ <defs>
+ <use id="target2" xlink:href=""
+ <use id="target3" xlink:href=""
+ <g id="target4">
+ <rect width="100" height="100" fill="green"/>
+ </g>
+ <g id="target5">
+ <use xlink:href=""
+ <use x="200" xlink:href=""
+ </g>
+ <rect id="target6" width="100" height="100" fill="green"/>
+ <rect id="target7" width="100" height="100" fill="green"/>
+ </defs>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (122448 => 122449)
--- trunk/Source/WebCore/ChangeLog 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/Source/WebCore/ChangeLog 2012-07-12 12:26:03 UTC (rev 122449)
@@ -1,3 +1,41 @@
+2012-07-12 Florin Malita <fmal...@chromium.org>
+
+ Incorrect handling of chained pending resources in SVGUseElement
+ https://bugs.webkit.org/show_bug.cgi?id=89686
+
+ Reviewed by Nikolas Zimmermann.
+
+ Currently SVGUseElement builds the shadow tree when the target first
+ becomes available. This is normally OK, but if the target itself (or
+ one of its children) is a <use> element with pending resources, then
+ the shadow expansion only captures the current state of the tree and
+ never gets updated when the pending resource becomes available.
+
+ In order to support arbitrary <use>-on-<use> constructs, this patch
+ tracks nested <use> dependencies and rebuilds the dependent trees
+ whenever the target gets updated.
+
+
+ Tests: svg/custom/use-nested-expected.svg
+ svg/custom/use-nested.svg
+
+ * svg/SVGElement.cpp:
+ (WebCore::SVGElement::removedFrom): removedFrom needs to be called up the inheritance chain
+ before invoking removeAllElementReferencesForTarget. Otherwise we could end up finding the
+ element being removed as a valid target in SVGUseElement::buildInstanceTree because its
+ InDocument flag is not cleared yet.
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::~SVGUseElement):
+ (WebCore::SVGUseElement::clearResourceReferences):
+ (WebCore::SVGUseElement::buildPendingResource):
+ (WebCore::SVGUseElement::buildShadowAndInstanceTree):
+ (WebCore::SVGUseElement::buildInstanceTree):
+ * svg/SVGUseElement.h:
+ (SVGUseElement):
+ Track <use> -> <use> dependencies using SVGDocumentExtensions'
+ m_elementDependencies framework and ensure dependent trees are rebuilt
+ when the target itself gets rebuilt.
+
2012-07-12 MORITA Hajime <morr...@google.com>
Typo: ParentTranversalDetails should be ParentTraversalDetails
Modified: trunk/Source/WebCore/svg/SVGElement.cpp (122448 => 122449)
--- trunk/Source/WebCore/svg/SVGElement.cpp 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/Source/WebCore/svg/SVGElement.cpp 2012-07-12 12:26:03 UTC (rev 122449)
@@ -173,12 +173,14 @@
void SVGElement::removedFrom(ContainerNode* rootParent)
{
- if (rootParent->inDocument()) {
+ bool wasInDocument = rootParent->inDocument();
+
+ StyledElement::removedFrom(rootParent);
+
+ if (wasInDocument) {
document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
}
-
- StyledElement::removedFrom(rootParent);
}
SVGSVGElement* SVGElement::ownerSVGElement() const
Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (122448 => 122449)
--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-07-12 12:26:03 UTC (rev 122449)
@@ -107,6 +107,8 @@
{
if (m_cachedDocument)
m_cachedDocument->removeClient(this);
+
+ clearResourceReferences();
}
void SVGUseElement::createShadowSubtree()
@@ -407,6 +409,9 @@
}
m_needsShadowTreeRecreation = false;
+
+ ASSERT(document());
+ document()->accessSVGExtensions()->removeAllTargetReferencesForElement(this);
}
void SVGUseElement::buildPendingResource()
@@ -414,12 +419,12 @@
if (!referencedDocument())
return;
clearResourceReferences();
- if (!inDocument())
+ if (!inDocument() || isInShadowTree())
return;
String id;
Element* target = SVGURIReference::targetElementFromIRIString(href(), document(), &id, externalDocument());
- if (!target) {
+ if (!target || !target->inDocument()) {
// If we can't find the target of an external element, just give up.
// We can't observe if the target somewhen enters the external document, nor should we do it.
if (externalDocument())
@@ -465,7 +470,7 @@
// Eventually enter recursion to build SVGElementInstance objects for the sub-tree children
bool foundProblem = false;
- buildInstanceTree(target, m_targetElementInstance.get(), foundProblem);
+ buildInstanceTree(target, m_targetElementInstance.get(), foundProblem, false);
if (instanceTreeIsLoading(m_targetElementInstance.get()))
return;
@@ -519,6 +524,10 @@
// Update relative length information.
updateRelativeLengthsInformation();
+ // Rebuild all dependent use elements.
+ ASSERT(document());
+ document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
+
// Eventually dump instance tree
#ifdef DUMP_INSTANCE_TREE
String text;
@@ -586,7 +595,7 @@
return 0;
}
-void SVGUseElement::buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundProblem)
+void SVGUseElement::buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundProblem, bool foundUse)
{
ASSERT(target);
ASSERT(targetInstance);
@@ -599,6 +608,14 @@
foundProblem = hasCycleUseReferencing(static_cast<SVGUseElement*>(target), targetInstance, newTarget);
if (foundProblem)
return;
+
+ // We only need to track fist degree <use> dependencies. Indirect references are handled
+ // as the invalidation bubbles up the dependency chain.
+ if (!foundUse) {
+ ASSERT(document());
+ document()->accessSVGExtensions()->addElementReferencingTarget(this, target);
+ foundUse = true;
+ }
} else if (isDisallowedElement(target)) {
foundProblem = true;
return;
@@ -626,7 +643,7 @@
targetInstance->appendChild(instance.release());
// Enter recursion, appending new instance tree nodes to the "instance" object.
- buildInstanceTree(element, instancePtr, foundProblem);
+ buildInstanceTree(element, instancePtr, foundProblem, foundUse);
if (foundProblem)
return;
}
@@ -637,7 +654,7 @@
RefPtr<SVGElementInstance> newInstance = SVGElementInstance::create(this, static_cast<SVGUseElement*>(target), newTarget);
SVGElementInstance* newInstancePtr = newInstance.get();
targetInstance->appendChild(newInstance.release());
- buildInstanceTree(newTarget, newInstancePtr, foundProblem);
+ buildInstanceTree(newTarget, newInstancePtr, foundProblem, foundUse);
}
bool SVGUseElement::hasCycleUseReferencing(SVGUseElement* use, SVGElementInstance* targetInstance, SVGElement*& newTarget)
Modified: trunk/Source/WebCore/svg/SVGUseElement.h (122448 => 122449)
--- trunk/Source/WebCore/svg/SVGUseElement.h 2012-07-12 12:05:28 UTC (rev 122448)
+++ trunk/Source/WebCore/svg/SVGUseElement.h 2012-07-12 12:26:03 UTC (rev 122449)
@@ -84,7 +84,7 @@
virtual bool selfHasRelativeLengths() const;
// Instance tree handling
- void buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundCycle);
+ void buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundCycle, bool foundUse);
bool hasCycleUseReferencing(SVGUseElement*, SVGElementInstance* targetInstance, SVGElement*& newTarget);
// Shadow tree handling
