Skip to content

GitLab

T
tangled
Commit dd6dea97 authored by fs@opera.com's avatar fs@opera.com
Browse files
Options

Make SVGResourcesCycleSolver check for cycles beyond one level (and more) 

The SVGResourceCycleSolver checks for cycles from within the resources
a RenderObject references to the resources it references, or any resources
in the ancestor-chain of the starting RenderObject (including itself if
it is a resource container).
This suffers from a few issues. One is that a resource that is referenced
both by the starting RenderObject and another resource referenced from the
starting RenderObject will be flagged as a cycle because all the resources
are added to the resources set.
Another issue is that longer cycles may not be detected because only the
resources referenced by the starting RenderObject is checked, and
depending on how the render tree is built, some cycles could be missed.
Additionally, a resource defined within another resource can cause a
"false cycle" to be flagged when the inner resource references a resource
in the resource set - eventhough the inner resource itself is not reachable
from the starting RenderObject.

To fix the above, make the SVGResourcesCycleSolver traverse the entire
resource "graph", and verify that it is acyclic. The cycle-checking is
achieved by keeping track of the current "path" through the graph, and
checking any discovered references against that set. When traversing
a subtree looking for references, any resources discovered will be skipped
to avoid inject false dependencies that way.
Also keep a simple "cache" of sub-graphs that has been checked already
(and verified cycle-free) to avoid having to revisit them.

BUG=351713

Review URL: https://codereview.chromium.org/303693009

git-svn-id: svn://svn.chromium.org/blink/trunk@175129 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 7bc16110
Hide whitespace changes
Inline Side-by-side
Showing with 328 additions and 82 deletions
third_party/WebKit/LayoutTests/svg/custom/pattern-3-step-cycle-dynamic-1.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
window.onload = function() {
testRunner.displayAsyncThen(function() {
mutateTree();
testRunner.displayAsyncThen(function() {
testRunner.notifyDone();
});
});
};
} else {
window.onload = function() { setTimeout(mutateTree, 100); };
}
function mutateTree() {
// A reference from the 'rect' to the pattern cycle.
document.getElementsByTagName('rect')[0].setAttribute('fill', 'url(#p1)');
}
</script>
<p>PASS if no crash (stack overflow).</p>
<svg width="100" height="100">
<rect width="100" height="100"/>
<pattern id="p3" width="1" height="1">
<rect fill="url(#p1)" width="100" height="100"/>
</pattern>
<pattern id="p2" width="1" height="1">
<rect fill="url(#p3)" width="100" height="100"/>
</pattern>
<pattern id="p1" width="1" height="1">
<rect fill="url(#p2)" width="100" height="100"/>
</pattern>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-3-step-cycle-dynamic-2.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
window.onload = function() {
testRunner.displayAsyncThen(function() {
mutateTree();
testRunner.displayAsyncThen(function() {
testRunner.notifyDone();
});
});
};
} else {
window.onload = function() { setTimeout(mutateTree, 100); };
}
function mutateTree() {
// Add a reference from the rect in pattern#p3 to form a cycle.
document.getElementsByTagName('rect')[1].setAttribute('fill', 'url(#p1)');
}
</script>
<p>PASS if no crash (stack overflow).</p>
<svg width="100" height="100">
<rect width="100" height="100" fill="url(#p1)"/>
<pattern id="p3" width="1" height="1">
<rect width="100" height="100"/>
</pattern>
<pattern id="p2" width="1" height="1">
<rect fill="url(#p3)" width="100" height="100"/>
</pattern>
<pattern id="p1" width="1" height="1">
<rect fill="url(#p2)" width="100" height="100"/>
</pattern>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-3-step-cycle-dynamic-3.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
window.onload = function() {
testRunner.displayAsyncThen(function() {
mutateTree();
testRunner.displayAsyncThen(function() {
testRunner.notifyDone();
});
});
};
} else {
window.onload = function() { setTimeout(mutateTree, 100); };
}
const svgNs = 'http://www.w3.org/2000/svg';
function buildPattern(patternId, refId) {
var pattern = document.createElementNS(svgNs, 'pattern');
var rect = pattern.appendChild(document.createElementNS(svgNs, 'rect'));
pattern.setAttribute('id', patternId);
pattern.setAttribute('width', 1);
pattern.setAttribute('height', 1);
rect.setAttribute('width', 100);
rect.setAttribute('height', 100);
rect.setAttribute('fill', 'url(#' + refId + ')');
return pattern;
}
function mutateTree() {
// Build a three-step pattern cycle in a detached
// subtree and then insert it at load.
var defs = document.createElementNS(svgNs, 'defs');
defs.appendChild(buildPattern('p3', 'p1'));
defs.appendChild(buildPattern('p2', 'p3'));
defs.appendChild(buildPattern('p1', 'p2'));
document.querySelector('svg').appendChild(defs);
}
</script>
<p>PASS if no crash (stack overflow).</p>
<svg width="100" height="100">
<rect width="100" height="100" fill="url(#p1)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-3-step-cycle-dynamic-4.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<script src="../../resources/run-after-display.js"></script>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
window.onload = function() {
testRunner.displayAsyncThen(function() {
mutateTree();
testRunner.displayAsyncThen(function() {
testRunner.notifyDone();
});
});
};
} else {
window.onload = function() { setTimeout(mutateTree, 100); };
}
const svgNs = 'http://www.w3.org/2000/svg';
function buildPattern(patternId, refId) {
var pattern = document.createElementNS(svgNs, 'pattern');
var rect = pattern.appendChild(document.createElementNS(svgNs, 'rect'));
pattern.setAttribute('id', patternId);
pattern.setAttribute('width', 1);
pattern.setAttribute('height', 1);
rect.setAttribute('width', 100);
rect.setAttribute('height', 100);
rect.setAttribute('fill', 'url(#' + refId + ')');
return pattern;
}
function mutateTree() {
// Get reference to rect in pattern#p2 before inserting the pattern.
var p2rect = document.getElementsByTagName('rect')[1];
// Add a pattern#p3 and a reference to it from pattern#p2 to form a cycle.
var defs = document.querySelector('defs');
defs.appendChild(buildPattern('p3', 'p1'));
p2rect.setAttribute('fill', 'url(#p3)');
}
</script>
<p>PASS if no crash (stack overflow).</p>
<svg width="100" height="100">
<rect width="100" height="100" fill="url(#p1)"/>
<defs>
<pattern id="p2" width="1" height="1">
<rect width="100" height="100"/>
</pattern>
<pattern id="p1" width="1" height="1">
<rect fill="url(#p2)" width="100" height="100"/>
</pattern>
</defs>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-3-step-cycle.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
window.onload = function() {
testRunner.displayAsyncThen(function() { testRunner.notifyDone(); });
};
}
</script>
<p>PASS if no crash (stack overflow).</p>
<svg width="100" height="100">
<rect width="100" height="100" fill="url(#p1)"/>
<pattern id="p3" width="1" height="1">
<rect fill="url(#p1)" width="100" height="100"/>
</pattern>
<pattern id="p2" width="1" height="1">
<rect fill="url(#p3)" width="100" height="100"/>
</pattern>
<pattern id="p1" width="1" height="1">
<rect fill="url(#p2)" width="100" height="100"/>
</pattern>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-directly-and-indirectly-referenced-expected.svg 0 → 100644
View file @ dd6dea97
<svg xmlns="http://www.w3.org/2000/svg">
<pattern width="100%" height="100%" id="pattern" viewBox="0 0 100 100">
<rect width="100" height="100" fill="gray"/>
</pattern>
<pattern width="100%" height="100%" id="maskPattern" viewBox="0 0 100 100">
<rect width="100" height="100" fill="gray"/>
</pattern>
<mask id="mask">
<circle fill="url(#maskPattern)" cx="100" cy="100" r="100"/>
</mask>
<rect width="200" height="200" mask="url(#mask)" fill="url(#pattern)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-directly-and-indirectly-referenced.svg 0 → 100644
View file @ dd6dea97
<svg xmlns="http://www.w3.org/2000/svg">
<pattern width="100%" height="100%" id="pattern" viewBox="0 0 100 100">
<rect width="100" height="100" fill="gray"/>
</pattern>
<mask id="mask">
<circle fill="url(#pattern)" cx="100" cy="100" r="100"/>
</mask>
<rect width="200" height="200" mask="url(#mask)" fill="url(#pattern)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-false-resource-cycle-expected.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<svg>
<clipPath id="c1">
<rect width="100" height="100"/>
</clipPath>
<pattern id="p1" width="1" height="1">
<rect width="100" height="100" fill="green"/>
</pattern>
<rect width="100" height="100" fill="url(#p1)" clip-path="url(#c1)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-false-resource-cycle.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<svg>
<clipPath id="c1">
<rect width="100" height="100"/>
</clipPath>
<pattern id="p1" width="1" height="1">
<rect width="100" height="100" fill="green"/>
<clipPath id="c2" clip-path="url(#c1)">
<rect width="100" height="100"/>
</clipPath>
</pattern>
<rect width="100" height="100" fill="url(#p1)" clip-path="url(#c1)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-within-other-pattern-expected.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<svg width="100" height="100">
<pattern id="p" width="1" height="1">
<rect width="100" height="100" fill="green"/>
</pattern>
<rect width="100" height="100" fill="url(#p)"/>
</svg>
third_party/WebKit/LayoutTests/svg/custom/pattern-within-other-pattern.html 0 → 100644
View file @ dd6dea97
<!DOCTYPE html>
<svg width="100" height="100">
<pattern id="outer" width="1" height="1">
<pattern id="inner" width="1" height="1">
<rect width="100" height="100" fill="url(#outer)"/>
</pattern>
<rect width="100" height="100" fill="green"/>
</pattern>
<rect width="100" height="100" fill="url(#inner)"/>
</svg>
third_party/WebKit/Source/core/rendering/svg/SVGResourcesCycleSolver.cpp
View file @ dd6dea97
......@@ -47,101 +47,83 @@ SVGResourcesCycleSolver::~SVGResourcesCycleSolver()
{
}
bool SVGResourcesCycleSolver::resourceContainsCycles(RenderObject* renderer) const
{
ASSERT(renderer);
// First (first loop iteration) operate on the resources of the given
// renderer.
// <marker id="a"> <path marker-start="url(#b)"/> ...
// <marker id="b" marker-start="url(#a)"/>
// Then operate on the child resources of the given renderer.
// <marker id="a"> <path marker-start="url(#b)"/> ...
// <marker id="b"> <path marker-start="url(#a)"/> ...
for (RenderObject* child = renderer; child; child = child->nextInPreOrder(renderer)) {
SVGResources* childResources = SVGResourcesCache::cachedResourcesForRenderObject(child);
if (!childResources)
continue;
struct ActiveFrame {
typedef SVGResourcesCycleSolver::ResourceSet ResourceSet;
// A child of the given 'resource' contains resources.
HashSet<RenderSVGResourceContainer*> childSet;
childResources->buildSetOfResources(childSet);
ActiveFrame(ResourceSet& activeSet, RenderSVGResourceContainer* resource)
: m_activeSet(activeSet)
, m_resource(resource)
{
m_activeSet.add(m_resource);
}
~ActiveFrame()
{
m_activeSet.remove(m_resource);
}
// Walk all child resources and check whether they reference any resource contained in the resources set.
HashSet<RenderSVGResourceContainer*>::iterator end = childSet.end();
for (HashSet<RenderSVGResourceContainer*>::iterator it = childSet.begin(); it != end; ++it) {
if (m_allResources.contains(*it))
return true;
ResourceSet& m_activeSet;
RenderSVGResourceContainer* m_resource;
};
bool SVGResourcesCycleSolver::resourceContainsCycles(RenderSVGResourceContainer* resource)
{
// If we've traversed this sub-graph before and no cycles were observed, then
// reuse that result.
if (m_dagCache.contains(resource))
return false;
ActiveFrame frame(m_activeResources, resource);
RenderObject* node = resource;
while (node) {
// Skip subtrees which are themselves resources. (They will be
// processed - if needed - when they are actually referenced.)
if (node != resource && node->isSVGResourceContainer()) {
node = node->nextInPreOrderAfterChildren(resource);
continue;
}
if (SVGResources* nodeResources = SVGResourcesCache::cachedResourcesForRenderObject(node)) {
// Fetch all the resources referenced by |node|.
ResourceSet nodeSet;
nodeResources->buildSetOfResources(nodeSet);
// Iterate resources referenced by |node|.
ResourceSet::iterator end = nodeSet.end();
for (ResourceSet::iterator it = nodeSet.begin(); it != end; ++it) {
if (m_activeResources.contains(*it) || resourceContainsCycles(*it))
return true;
}
}
node = node->nextInPreOrder(resource);
}
// No cycles found in (or from) this resource. Add it to the "DAG cache".
m_dagCache.add(resource);
return false;
}
void SVGResourcesCycleSolver::resolveCycles()
{
ASSERT(m_allResources.isEmpty());
ASSERT(m_activeResources.isEmpty());
#if DEBUG_CYCLE_DETECTION > 0
fprintf(stderr, "\nBefore cycle detection:\n");
m_resources->dump(m_renderer);
#endif
// Stash all resources into a HashSet for the ease of traversing.
HashSet<RenderSVGResourceContainer*> localResources;
m_resources->buildSetOfResources(localResources);
ASSERT(!localResources.isEmpty());
// Add all parent resource containers to the HashSet.
HashSet<RenderSVGResourceContainer*> parentResources;
RenderObject* parent = m_renderer->parent();
while (parent) {
if (parent->isSVGResourceContainer())
parentResources.add(toRenderSVGResourceContainer(parent));
parent = parent->parent();
}
#if DEBUG_CYCLE_DETECTION > 0
fprintf(stderr, "\nDetecting wheter any resources references any of following objects:\n");
{
fprintf(stderr, "Local resources:\n");
HashSet<RenderSVGResourceContainer*>::iterator end = localResources.end();
for (HashSet<RenderSVGResourceContainer*>::iterator it = localResources.begin(); it != end; ++it)
fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
fprintf(stderr, "Parent resources:\n");
end = parentResources.end();
for (HashSet<RenderSVGResourceContainer*>::iterator it = parentResources.begin(); it != end; ++it)
fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
}
#endif
// Build combined set of local and parent resources.
m_allResources = localResources;
HashSet<RenderSVGResourceContainer*>::iterator end = parentResources.end();
for (HashSet<RenderSVGResourceContainer*>::iterator it = parentResources.begin(); it != end; ++it)
m_allResources.add(*it);
// If we're a resource, add ourselves to the HashSet.
// If the starting RenderObject is a resource container itself, then add it
// to the active set (to break direct self-references.)
if (m_renderer->isSVGResourceContainer())
m_allResources.add(toRenderSVGResourceContainer(m_renderer));
m_activeResources.add(toRenderSVGResourceContainer(m_renderer));
ASSERT(!m_allResources.isEmpty());
ResourceSet localResources;
m_resources->buildSetOfResources(localResources);
// The job of this function is to determine wheter any of the 'resources' associated with the given 'renderer'
// references us (or wheter any of its kids references us) -> that's a cycle, we need to find and break it.
end = localResources.end();
for (HashSet<RenderSVGResourceContainer*>::iterator it = localResources.begin(); it != end; ++it) {
RenderSVGResourceContainer* resource = *it;
if (parentResources.contains(resource) || resourceContainsCycles(resource))
breakCycle(resource);
// This performs a depth-first search for a back-edge in all the
// (potentially disjoint) graphs formed by the resources referenced by
// |m_renderer|.
ResourceSet::iterator end = localResources.end();
for (ResourceSet::iterator it = localResources.begin(); it != end; ++it) {
if (m_activeResources.contains(*it) || resourceContainsCycles(*it))
breakCycle(*it);
}
#if DEBUG_CYCLE_DETECTION > 0
fprintf(stderr, "\nAfter cycle detection:\n");
m_resources->dump(m_renderer);
#endif
m_allResources.clear();
m_activeResources.clear();
}
void SVGResourcesCycleSolver::breakCycle(RenderSVGResourceContainer* resourceLeadingToCycle)
......
third_party/WebKit/Source/core/rendering/svg/SVGResourcesCycleSolver.h
View file @ dd6dea97
......@@ -37,13 +37,17 @@ public:
void resolveCycles();
typedef HashSet<RenderSVGResourceContainer*> ResourceSet;
private:
bool resourceContainsCycles(RenderObject*) const;
bool resourceContainsCycles(RenderSVGResourceContainer*);
void breakCycle(RenderSVGResourceContainer*);
RenderObject* m_renderer;
SVGResources* m_resources;
HashSet<RenderSVGResourceContainer*> m_allResources;
ResourceSet m_activeResources;
ResourceSet m_dagCache;
};
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment