A crash reproducible in Path::isEmpty() under RenderSVGShape::paint()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Jan 2016 17:50:26 +0000 (17:50 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Jan 2016 17:50:26 +0000 (17:50 +0000)
commit3652c2e2a38d19b0707ad9a8a6bc5911625a3788
tree747011defbcd598c5deff6a7d34b143253ae031d
parent6cb2b04ba8bf0308e3dd13255d4e97e5d77147bb
A crash reproducible in Path::isEmpty() under RenderSVGShape::paint()
https://bugs.webkit.org/show_bug.cgi?id=149613

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2016-01-21
Reviewed by Darin Adler.
Source/WebCore:

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.

LayoutTests:

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@195411 268f45cc-cd09-0410-ab3c-d52691b4dbfc
17 files changed:
LayoutTests/ChangeLog
LayoutTests/svg/custom/filter-update-different-root-expected.html [new file with mode: 0644]
LayoutTests/svg/custom/filter-update-different-root.html [new file with mode: 0644]
LayoutTests/svg/custom/pattern-update-different-root-expected.html [new file with mode: 0644]
LayoutTests/svg/custom/pattern-update-different-root.html [new file with mode: 0644]
LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt
LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
Source/WebCore/rendering/svg/RenderSVGResourceContainer.h
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/RenderSVGShape.cpp
Source/WebCore/rendering/svg/RenderSVGText.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.h
Source/WebCore/rendering/svg/SVGResources.cpp
Source/WebCore/rendering/svg/SVGResources.h