Diff
Modified: trunk/LayoutTests/ChangeLog (195410 => 195411)
--- trunk/LayoutTests/ChangeLog 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/LayoutTests/ChangeLog 2016-01-21 17:50:26 UTC (rev 195411)
@@ -1,3 +1,31 @@
+2016-01-21 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ A crash reproducible in Path::isEmpty() under RenderSVGShape::paint()
+ https://bugs.webkit.org/show_bug.cgi?id=149613
+
+ Reviewed by Darin Adler.
+
+ When running the layout of an SVG root and it has resources which are
+ referenced by clients in other SVG roots, make sure we run the layout
+ for these resources before running the layout for their clients.
+
+ * svg/custom/filter-update-different-root-expected.html: Added.
+ * svg/custom/filter-update-different-root.html: Added.
+ Without this patch this test crashes because we paint a dirty RenderSVGShape.
+
+ * svg/custom/pattern-update-different-root-expected.html: Added.
+ * svg/custom/pattern-update-different-root.html: Added.
+ Without this patch this test works fine but it is good to have it to catch
+ cases where the SVG root needs to run re-layout for its children resources
+ and hence their clients if its size has changed.
+
+ * svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt:
+ * svg/custom/unicode-in-tspan-multi-svg-crash.html:
+ This test was ported from Blink in http://trac.webkit.org/changeset/166420.
+ The expectation of this test was changed in Blink:
+ https://src.chromium.org/viewvc/blink?revision=158480&view=revision.
+
+
2016-01-21 Nan Wang <n_w...@apple.com>
AX: [IOS] Implement next/previous text marker functions using TextIterator
Added: trunk/LayoutTests/svg/custom/filter-update-different-root-expected.html (0 => 195411)
--- trunk/LayoutTests/svg/custom/filter-update-different-root-expected.html (rev 0)
+++ trunk/LayoutTests/svg/custom/filter-update-different-root-expected.html 2016-01-21 17:50:26 UTC (rev 195411)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+ <svg>
+ <rect width="200" height="100" fill="green"/>
+ </svg>
+</body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/svg/custom/filter-update-different-root.html (0 => 195411)
--- trunk/LayoutTests/svg/custom/filter-update-different-root.html (rev 0)
+++ trunk/LayoutTests/svg/custom/filter-update-different-root.html 2016-01-21 17:50:26 UTC (rev 195411)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .dummy { }
+ .hide-second-path > .second-path { display: none; }
+ </style>
+</head>
+<body>
+ <svg style="background-color: #fff">
+ <g id="parent-group" class="hide-second-path">
+ <path id="id1" d="m 0,0 H 100 V 100 H 0 z" fill="red"/>
+ <path id="id2" d="m 0,0 H 100 V 100 H 0 z" fill="green" class="second-path"/>
+ <g id="child-group" filter="url(#nop-filter)">
+ <path id="id3" d="m 100,0 H 200 V 100 H 100 z" fill="green"/>
+ <path id="id4" d="m 100,0 H 200 V 100 H 100 z" fill="red" class="second-path"/>
+ </g>
+ </g>
+ </svg>
+ <svg>
+ <filter id="nop-filter">
+ <feOffset dx="0" dy="0"/>
+ </filter>
+ </svg>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+ setTimeout(function() {
+ document.styleSheets[0].deleteRule(0);
+ setTimeout(function() {
+ document.getElementById('parent-group').removeAttribute('class');
+ document.getElementById('child-group').setAttribute('class', 'hide-second-path');
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 50);
+ }, 50);
+ </script>
+</body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/svg/custom/pattern-update-different-root-expected.html (0 => 195411)
--- trunk/LayoutTests/svg/custom/pattern-update-different-root-expected.html (rev 0)
+++ trunk/LayoutTests/svg/custom/pattern-update-different-root-expected.html 2016-01-21 17:50:26 UTC (rev 195411)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ svg {
+ width: 100px;
+ height: 100px;
+ }
+</style>
+</head>
+<body>
+ <svg>
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+</body>
+</html>
Added: trunk/LayoutTests/svg/custom/pattern-update-different-root.html (0 => 195411)
--- trunk/LayoutTests/svg/custom/pattern-update-different-root.html (rev 0)
+++ trunk/LayoutTests/svg/custom/pattern-update-different-root.html 2016-01-21 17:50:26 UTC (rev 195411)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ svg {
+ width: 100px;
+ height: 100px;
+ }
+</style>
+</head>
+<body>
+ <svg>
+ <pattern id="r1" width="100%" height="100%" xlink:href="" patternUnits="userSpaceOnUse"/>
+ <rect width="100%" height="100%" fill="url(#r1)"/>
+ </svg>
+ <svg>
+ <pattern id="r2">
+ <rect width="100%" height="100%" fill="green"/>
+ </pattern>
+ </svg>
+</body>
+</html>
Modified: trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt (195410 => 195411)
--- trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt 2016-01-21 17:50:26 UTC (rev 195411)
@@ -1,2 +1,3 @@
+
-Test Passes if there is no crash in Debug or Asan builds. There should be no characters preceding "Test".
+Test Passes if there is no crash in Debug or Asan builds.
Modified: trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html (195410 => 195411)
--- trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html 2016-01-21 17:50:26 UTC (rev 195411)
@@ -33,6 +33,6 @@
<filter id="filterInSecondRoot"/>
</svg>
- <p>Test Passes if there is no crash in Debug or Asan builds. There should be no characters preceding "Test".</p>
+ <p>Test Passes if there is no crash in Debug or Asan builds.</p>
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (195410 => 195411)
--- trunk/Source/WebCore/ChangeLog 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/ChangeLog 2016-01-21 17:50:26 UTC (rev 195411)
@@ -1,3 +1,77 @@
+2016-01-21 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ A crash reproducible in Path::isEmpty() under RenderSVGShape::paint()
+ https://bugs.webkit.org/show_bug.cgi?id=149613
+
+ Reviewed by Darin Adler.
+
+ When RenderSVGRoot::layout() realizes its layout size has changed and
+ it has resources which have relative sizes, it marks all the clients of
+ the resources for invalidates regardless whether they belong to the
+ same RenderSVGRoot or not. But it reruns the layout only for its children.
+ If one of these clients comes before the current RenderSVGRoot in the render
+ tree, ee end up having renderer marked for invalidation at rendering time.
+ This also prevents scheduling the layout if the same renderer is marked
+ for another invalidation later. We prevent this because we do not want
+ to schedule another layout for a renderer which is already marked for
+ invalidation. This can cause crash if the renderer is an RenderSVGPath.
+
+ The fix is to mark "only" the clients of a resource which belong to the
+ same RenderSVGRoot of the resource. Also we need to run the layout for
+ all the resources which belong to different RenderSVGRoots before running
+ the layout for an SVG renderer.
+
+ Tests: svg/custom/filter-update-different-root.html
+ svg/custom/pattern-update-different-root.html
+
+ * rendering/svg/RenderSVGResourceContainer.cpp:
+ (WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation):
+ We should not mark any client outside the current root for invalidation
+
+ * rendering/svg/RenderSVGResourceContainer.h: Remove unneeded private keyword.
+
+ * rendering/svg/RenderSVGRoot.cpp:
+ (WebCore::RenderSVGRoot::addResourceForClientInvalidation):
+ Code clean up; use findTreeRootObject() instead of repeating the same code.
+
+ * rendering/svg/RenderSVGShape.cpp:
+ (WebCore::RenderSVGShape::isEmpty): Avoid crashing if RenderSVGShape::isEmpty()
+ is called before calling RenderSVGShape::layout().
+
+ * rendering/svg/RenderSVGText.cpp:
+ (WebCore::RenderSVGText::layout): findTreeRootObject() now returns a pointer.
+
+ * rendering/svg/SVGRenderSupport.cpp:
+ (WebCore::SVGRenderSupport::findTreeRootObject): I do think nothing
+ guarantees that an SVG renderer has to have an RenderSVGRoot in its
+ ancestors. So change this function to return a pointer. Also Provide
+ the non-const version of this function.
+
+ (WebCore::SVGRenderSupport::layoutDifferentRootIfNeeded): Runs the layout
+ if needed for all the resources which belong to different RenderSVGRoots.
+
+ (WebCore::SVGRenderSupport::layoutChildren): Make sure all the renderer's
+ resources which belong to different RenderSVGRoots are laid out before
+ running the layout for this renderer.
+
+ * rendering/svg/SVGRenderSupport.h: Remove a mysterious comment.
+
+ * rendering/svg/SVGResources.cpp:
+ (WebCore::SVGResources::layoutDifferentRootIfNeeded): Run the layout for
+ all the resources which belong to different RenderSVGRoots outside the
+ context of their RenderSVGRoots.
+
+ * rendering/svg/SVGResources.h:
+ (WebCore::SVGResources::clipper):
+ (WebCore::SVGResources::markerStart):
+ (WebCore::SVGResources::markerMid):
+ (WebCore::SVGResources::markerEnd):
+ (WebCore::SVGResources::masker):
+ (WebCore::SVGResources::filter):
+ (WebCore::SVGResources::fill):
+ (WebCore::SVGResources::stroke):
+ Code clean up; use nullptr instead of 0.
+
2016-01-21 Jer Noble <jer.no...@apple.com>
[EME] Correctly report errors when generating key requests from AVContentKeySession.
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -94,8 +94,13 @@
m_isInvalidating = true;
bool needsLayout = mode == LayoutAndBoundariesInvalidation;
bool markForInvalidation = mode != ParentOnlyInvalidation;
+ auto* root = SVGRenderSupport::findTreeRootObject(*this);
for (auto* client : m_clients) {
+ // We should not mark any client outside the current root for invalidation
+ if (root != SVGRenderSupport::findTreeRootObject(*client))
+ continue;
+
if (is<RenderSVGResourceContainer>(*client)) {
downcast<RenderSVGResourceContainer>(*client).removeAllClientsFromCache(markForInvalidation);
continue;
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h 2016-01-21 17:50:26 UTC (rev 195411)
@@ -66,7 +66,6 @@
void addClient(RenderElement&);
void removeClient(RenderElement&);
-private:
virtual void willBeDestroyed() override final;
void registerResource();
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -455,12 +455,10 @@
void RenderSVGRoot::addResourceForClientInvalidation(RenderSVGResourceContainer* resource)
{
- RenderElement* svgRoot = resource->parent();
- while (svgRoot && !is<RenderSVGRoot>(*svgRoot))
- svgRoot = svgRoot->parent();
+ RenderSVGRoot* svgRoot = SVGRenderSupport::findTreeRootObject(*resource);
if (!svgRoot)
return;
- downcast<RenderSVGRoot>(*svgRoot).m_resourcesNeedingToInvalidateClients.add(resource);
+ svgRoot->m_resourcesNeedingToInvalidateClients.add(resource);
}
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -89,7 +89,11 @@
bool RenderSVGShape::isEmpty() const
{
- return path().isEmpty();
+ // This function should never be called before assigning a new Path to m_path.
+ // But this bug can happen if this renderer was created and its layout was not
+ // done before painting. Assert this did not happen but do not crash.
+ ASSERT(hasPath());
+ return !hasPath() || path().isEmpty();
}
void RenderSVGShape::fillShape(GraphicsContext& context) const
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -369,7 +369,7 @@
m_needsReordering = true;
m_needsPositioningValuesUpdate = false;
updateCachedBoundariesInParents = true;
- } else if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(*this).isLayoutSizeChanged()) {
+ } else if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(*this)->isLayoutSizeChanged()) {
// If the root layout size changed (eg. window size changes) or the transform to the root
// context has changed then recompute the on-screen font size.
updateFontInAllDescendants(this, &m_layoutAttributesBuilder);
Modified: trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -184,11 +184,16 @@
return localTransform.mapRect(localRepaintRect).intersects(paintInfo.rect);
}
-const RenderSVGRoot& SVGRenderSupport::findTreeRootObject(const RenderElement& start)
+RenderSVGRoot* SVGRenderSupport::findTreeRootObject(RenderElement& start)
{
- return *lineageOfType<RenderSVGRoot>(start).first();
+ return lineageOfType<RenderSVGRoot>(start).first();
}
+const RenderSVGRoot* SVGRenderSupport::findTreeRootObject(const RenderElement& start)
+{
+ return lineageOfType<RenderSVGRoot>(start).first();
+}
+
static inline void invalidateResourcesOfChildren(RenderElement& renderer)
{
ASSERT(!renderer.needsLayout());
@@ -225,6 +230,15 @@
return false;
}
+void SVGRenderSupport::layoutDifferentRootIfNeeded(const RenderElement& renderer)
+{
+ if (auto* resources = SVGResourcesCache::cachedResourcesForRenderer(renderer)) {
+ auto* svgRoot = SVGRenderSupport::findTreeRootObject(renderer);
+ ASSERT(svgRoot);
+ resources->layoutDifferentRootIfNeeded(svgRoot);
+ }
+}
+
void SVGRenderSupport::layoutChildren(RenderElement& start, bool selfNeedsLayout)
{
bool layoutSizeChanged = layoutSizeOfNearestViewportChanged(start);
@@ -273,6 +287,7 @@
child->setNeedsLayout(MarkOnlyThis);
if (child->needsLayout()) {
+ layoutDifferentRootIfNeeded(downcast<RenderElement>(*child));
downcast<RenderElement>(*child).layout();
// Renderers are responsible for repainting themselves when changing, except
// for the initial paint to avoid potential double-painting caused by non-sensical "old" bounds.
Modified: trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h 2016-01-21 17:50:26 UTC (rev 195411)
@@ -44,6 +44,8 @@
// SVGRendererSupport is a helper class sharing code between all SVG renderers.
class SVGRenderSupport {
public:
+ static void layoutDifferentRootIfNeeded(const RenderElement&);
+
// Shares child layouting code between RenderSVGRoot/RenderSVG(Hidden)Container
static void layoutChildren(RenderElement&, bool selfNeedsLayout);
@@ -91,8 +93,8 @@
static void updateMaskedAncestorShouldIsolateBlending(const RenderElement&);
#endif
- // FIXME: These methods do not belong here.
- static const RenderSVGRoot& findTreeRootObject(const RenderElement&);
+ static RenderSVGRoot* findTreeRootObject(RenderElement&);
+ static const RenderSVGRoot* findTreeRootObject(const RenderElement&);
private:
// This class is not constructable.
Modified: trunk/Source/WebCore/rendering/svg/SVGResources.cpp (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/SVGResources.cpp 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/SVGResources.cpp 2016-01-21 17:50:26 UTC (rev 195411)
@@ -25,6 +25,7 @@
#include "RenderSVGResourceFilter.h"
#include "RenderSVGResourceMarker.h"
#include "RenderSVGResourceMasker.h"
+#include "RenderSVGRoot.h"
#include "SVGGradientElement.h"
#include "SVGNames.h"
#include "SVGPaint.h"
@@ -283,6 +284,27 @@
return foundResources;
}
+void SVGResources::layoutDifferentRootIfNeeded(const RenderSVGRoot* svgRoot)
+{
+ if (clipper() && svgRoot != SVGRenderSupport::findTreeRootObject(*clipper()))
+ clipper()->layoutIfNeeded();
+
+ if (masker() && svgRoot != SVGRenderSupport::findTreeRootObject(*masker()))
+ masker()->layoutIfNeeded();
+
+ if (filter() && svgRoot != SVGRenderSupport::findTreeRootObject(*filter()))
+ filter()->layoutIfNeeded();
+
+ if (markerStart() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerStart()))
+ markerStart()->layoutIfNeeded();
+
+ if (markerMid() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerMid()))
+ markerMid()->layoutIfNeeded();
+
+ if (markerEnd() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerEnd()))
+ markerEnd()->layoutIfNeeded();
+}
+
void SVGResources::removeClientFromCache(RenderElement& renderer, bool markForInvalidation) const
{
if (!m_clipperFilterMaskerData && !m_markerData && !m_fillStrokeData && !m_linkedResource)
Modified: trunk/Source/WebCore/rendering/svg/SVGResources.h (195410 => 195411)
--- trunk/Source/WebCore/rendering/svg/SVGResources.h 2016-01-21 17:15:16 UTC (rev 195410)
+++ trunk/Source/WebCore/rendering/svg/SVGResources.h 2016-01-21 17:50:26 UTC (rev 195411)
@@ -35,6 +35,7 @@
class RenderSVGResourceFilter;
class RenderSVGResourceMarker;
class RenderSVGResourceMasker;
+class RenderSVGRoot;
class SVGRenderStyle;
// Holds a set of resources associated with a RenderObject
@@ -44,24 +45,19 @@
SVGResources();
bool buildCachedResources(const RenderElement&, const RenderStyle&);
+ void layoutDifferentRootIfNeeded(const RenderSVGRoot*);
// Ordinary resources
- RenderSVGResourceClipper* clipper() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->clipper : 0; }
- RenderSVGResourceMarker* markerStart() const { return m_markerData ? m_markerData->markerStart : 0; }
- RenderSVGResourceMarker* markerMid() const { return m_markerData ? m_markerData->markerMid : 0; }
- RenderSVGResourceMarker* markerEnd() const { return m_markerData ? m_markerData->markerEnd : 0; }
- RenderSVGResourceMasker* masker() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->masker : 0; }
+ RenderSVGResourceClipper* clipper() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->clipper : nullptr; }
+ RenderSVGResourceMarker* markerStart() const { return m_markerData ? m_markerData->markerStart : nullptr; }
+ RenderSVGResourceMarker* markerMid() const { return m_markerData ? m_markerData->markerMid : nullptr; }
+ RenderSVGResourceMarker* markerEnd() const { return m_markerData ? m_markerData->markerEnd : nullptr; }
+ RenderSVGResourceMasker* masker() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->masker : nullptr; }
+ RenderSVGResourceFilter* filter() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->filter : nullptr; }
- RenderSVGResourceFilter* filter() const
- {
- if (m_clipperFilterMaskerData)
- return m_clipperFilterMaskerData->filter;
- return 0;
- }
-
// Paint servers
- RenderSVGResourceContainer* fill() const { return m_fillStrokeData ? m_fillStrokeData->fill : 0; }
- RenderSVGResourceContainer* stroke() const { return m_fillStrokeData ? m_fillStrokeData->stroke : 0; }
+ RenderSVGResourceContainer* fill() const { return m_fillStrokeData ? m_fillStrokeData->fill : nullptr; }
+ RenderSVGResourceContainer* stroke() const { return m_fillStrokeData ? m_fillStrokeData->stroke : nullptr; }
// Chainable resources - linked through xlink:href
RenderSVGResourceContainer* linkedResource() const { return m_linkedResource; }
