Changeset 195411 in webkit
- Timestamp:
- Jan 21, 2016 9:50:26 AM (8 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 13 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r195405 r195411 1 2016-01-21 Said Abou-Hallawa <sabouhallawa@apple.com> 2 3 A crash reproducible in Path::isEmpty() under RenderSVGShape::paint() 4 https://bugs.webkit.org/show_bug.cgi?id=149613 5 6 Reviewed by Darin Adler. 7 8 When running the layout of an SVG root and it has resources which are 9 referenced by clients in other SVG roots, make sure we run the layout 10 for these resources before running the layout for their clients. 11 12 * svg/custom/filter-update-different-root-expected.html: Added. 13 * svg/custom/filter-update-different-root.html: Added. 14 Without this patch this test crashes because we paint a dirty RenderSVGShape. 15 16 * svg/custom/pattern-update-different-root-expected.html: Added. 17 * svg/custom/pattern-update-different-root.html: Added. 18 Without this patch this test works fine but it is good to have it to catch 19 cases where the SVG root needs to run re-layout for its children resources 20 and hence their clients if its size has changed. 21 22 * svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt: 23 * svg/custom/unicode-in-tspan-multi-svg-crash.html: 24 This test was ported from Blink in http://trac.webkit.org/changeset/166420. 25 The expectation of this test was changed in Blink: 26 https://src.chromium.org/viewvc/blink?revision=158480&view=revision. 27 28 1 29 2016-01-21 Nan Wang <n_wang@apple.com> 2 30 -
trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash-expected.txt
r168350 r195411 1 1 2 2 Test Passes if there is no crash in Debug or Asan builds. There should be no characters preceding "Test".3 Test Passes if there is no crash in Debug or Asan builds. -
trunk/LayoutTests/svg/custom/unicode-in-tspan-multi-svg-crash.html
r166420 r195411 34 34 </svg> 35 35 36 <p>Test Passes if there is no crash in Debug or Asan builds. There should be no characters preceding "Test".</p>36 <p>Test Passes if there is no crash in Debug or Asan builds.</p> 37 37 </body> 38 38 </html> -
trunk/Source/WebCore/ChangeLog
r195410 r195411 1 2016-01-21 Said Abou-Hallawa <sabouhallawa@apple.com> 2 3 A crash reproducible in Path::isEmpty() under RenderSVGShape::paint() 4 https://bugs.webkit.org/show_bug.cgi?id=149613 5 6 Reviewed by Darin Adler. 7 8 When RenderSVGRoot::layout() realizes its layout size has changed and 9 it has resources which have relative sizes, it marks all the clients of 10 the resources for invalidates regardless whether they belong to the 11 same RenderSVGRoot or not. But it reruns the layout only for its children. 12 If one of these clients comes before the current RenderSVGRoot in the render 13 tree, ee end up having renderer marked for invalidation at rendering time. 14 This also prevents scheduling the layout if the same renderer is marked 15 for another invalidation later. We prevent this because we do not want 16 to schedule another layout for a renderer which is already marked for 17 invalidation. This can cause crash if the renderer is an RenderSVGPath. 18 19 The fix is to mark "only" the clients of a resource which belong to the 20 same RenderSVGRoot of the resource. Also we need to run the layout for 21 all the resources which belong to different RenderSVGRoots before running 22 the layout for an SVG renderer. 23 24 Tests: svg/custom/filter-update-different-root.html 25 svg/custom/pattern-update-different-root.html 26 27 * rendering/svg/RenderSVGResourceContainer.cpp: 28 (WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation): 29 We should not mark any client outside the current root for invalidation 30 31 * rendering/svg/RenderSVGResourceContainer.h: Remove unneeded private keyword. 32 33 * rendering/svg/RenderSVGRoot.cpp: 34 (WebCore::RenderSVGRoot::addResourceForClientInvalidation): 35 Code clean up; use findTreeRootObject() instead of repeating the same code. 36 37 * rendering/svg/RenderSVGShape.cpp: 38 (WebCore::RenderSVGShape::isEmpty): Avoid crashing if RenderSVGShape::isEmpty() 39 is called before calling RenderSVGShape::layout(). 40 41 * rendering/svg/RenderSVGText.cpp: 42 (WebCore::RenderSVGText::layout): findTreeRootObject() now returns a pointer. 43 44 * rendering/svg/SVGRenderSupport.cpp: 45 (WebCore::SVGRenderSupport::findTreeRootObject): I do think nothing 46 guarantees that an SVG renderer has to have an RenderSVGRoot in its 47 ancestors. So change this function to return a pointer. Also Provide 48 the non-const version of this function. 49 50 (WebCore::SVGRenderSupport::layoutDifferentRootIfNeeded): Runs the layout 51 if needed for all the resources which belong to different RenderSVGRoots. 52 53 (WebCore::SVGRenderSupport::layoutChildren): Make sure all the renderer's 54 resources which belong to different RenderSVGRoots are laid out before 55 running the layout for this renderer. 56 57 * rendering/svg/SVGRenderSupport.h: Remove a mysterious comment. 58 59 * rendering/svg/SVGResources.cpp: 60 (WebCore::SVGResources::layoutDifferentRootIfNeeded): Run the layout for 61 all the resources which belong to different RenderSVGRoots outside the 62 context of their RenderSVGRoots. 63 64 * rendering/svg/SVGResources.h: 65 (WebCore::SVGResources::clipper): 66 (WebCore::SVGResources::markerStart): 67 (WebCore::SVGResources::markerMid): 68 (WebCore::SVGResources::markerEnd): 69 (WebCore::SVGResources::masker): 70 (WebCore::SVGResources::filter): 71 (WebCore::SVGResources::fill): 72 (WebCore::SVGResources::stroke): 73 Code clean up; use nullptr instead of 0. 74 1 75 2016-01-21 Jer Noble <jer.noble@apple.com> 2 76 -
trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
r194496 r195411 95 95 bool needsLayout = mode == LayoutAndBoundariesInvalidation; 96 96 bool markForInvalidation = mode != ParentOnlyInvalidation; 97 auto* root = SVGRenderSupport::findTreeRootObject(*this); 97 98 98 99 for (auto* client : m_clients) { 100 // We should not mark any client outside the current root for invalidation 101 if (root != SVGRenderSupport::findTreeRootObject(*client)) 102 continue; 103 99 104 if (is<RenderSVGResourceContainer>(*client)) { 100 105 downcast<RenderSVGResourceContainer>(*client).removeAllClientsFromCache(markForInvalidation); -
trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h
r180643 r195411 67 67 void removeClient(RenderElement&); 68 68 69 private:70 69 virtual void willBeDestroyed() override final; 71 70 void registerResource(); -
trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp
r194496 r195411 456 456 void RenderSVGRoot::addResourceForClientInvalidation(RenderSVGResourceContainer* resource) 457 457 { 458 RenderElement* svgRoot = resource->parent(); 459 while (svgRoot && !is<RenderSVGRoot>(*svgRoot)) 460 svgRoot = svgRoot->parent(); 458 RenderSVGRoot* svgRoot = SVGRenderSupport::findTreeRootObject(*resource); 461 459 if (!svgRoot) 462 460 return; 463 downcast<RenderSVGRoot>(*svgRoot).m_resourcesNeedingToInvalidateClients.add(resource);464 } 465 466 } 461 svgRoot->m_resourcesNeedingToInvalidateClients.add(resource); 462 } 463 464 } -
trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp
r194819 r195411 90 90 bool RenderSVGShape::isEmpty() const 91 91 { 92 return path().isEmpty(); 92 // This function should never be called before assigning a new Path to m_path. 93 // But this bug can happen if this renderer was created and its layout was not 94 // done before painting. Assert this did not happen but do not crash. 95 ASSERT(hasPath()); 96 return !hasPath() || path().isEmpty(); 93 97 } 94 98 -
trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp
r194496 r195411 370 370 m_needsPositioningValuesUpdate = false; 371 371 updateCachedBoundariesInParents = true; 372 } else if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(*this) .isLayoutSizeChanged()) {372 } else if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(*this)->isLayoutSizeChanged()) { 373 373 // If the root layout size changed (eg. window size changes) or the transform to the root 374 374 // context has changed then recompute the on-screen font size. -
trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp
r192900 r195411 185 185 } 186 186 187 const RenderSVGRoot& SVGRenderSupport::findTreeRootObject(const RenderElement& start) 188 { 189 return *lineageOfType<RenderSVGRoot>(start).first(); 187 RenderSVGRoot* SVGRenderSupport::findTreeRootObject(RenderElement& start) 188 { 189 return lineageOfType<RenderSVGRoot>(start).first(); 190 } 191 192 const RenderSVGRoot* SVGRenderSupport::findTreeRootObject(const RenderElement& start) 193 { 194 return lineageOfType<RenderSVGRoot>(start).first(); 190 195 } 191 196 … … 224 229 225 230 return false; 231 } 232 233 void SVGRenderSupport::layoutDifferentRootIfNeeded(const RenderElement& renderer) 234 { 235 if (auto* resources = SVGResourcesCache::cachedResourcesForRenderer(renderer)) { 236 auto* svgRoot = SVGRenderSupport::findTreeRootObject(renderer); 237 ASSERT(svgRoot); 238 resources->layoutDifferentRootIfNeeded(svgRoot); 239 } 226 240 } 227 241 … … 274 288 275 289 if (child->needsLayout()) { 290 layoutDifferentRootIfNeeded(downcast<RenderElement>(*child)); 276 291 downcast<RenderElement>(*child).layout(); 277 292 // Renderers are responsible for repainting themselves when changing, except -
trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h
r190685 r195411 45 45 class SVGRenderSupport { 46 46 public: 47 static void layoutDifferentRootIfNeeded(const RenderElement&); 48 47 49 // Shares child layouting code between RenderSVGRoot/RenderSVG(Hidden)Container 48 50 static void layoutChildren(RenderElement&, bool selfNeedsLayout); … … 92 94 #endif 93 95 94 // FIXME: These methods do not belong here.95 static const RenderSVGRoot &findTreeRootObject(const RenderElement&);96 static RenderSVGRoot* findTreeRootObject(RenderElement&); 97 static const RenderSVGRoot* findTreeRootObject(const RenderElement&); 96 98 97 99 private: -
trunk/Source/WebCore/rendering/svg/SVGResources.cpp
r194819 r195411 26 26 #include "RenderSVGResourceMarker.h" 27 27 #include "RenderSVGResourceMasker.h" 28 #include "RenderSVGRoot.h" 28 29 #include "SVGGradientElement.h" 29 30 #include "SVGNames.h" … … 284 285 } 285 286 287 void SVGResources::layoutDifferentRootIfNeeded(const RenderSVGRoot* svgRoot) 288 { 289 if (clipper() && svgRoot != SVGRenderSupport::findTreeRootObject(*clipper())) 290 clipper()->layoutIfNeeded(); 291 292 if (masker() && svgRoot != SVGRenderSupport::findTreeRootObject(*masker())) 293 masker()->layoutIfNeeded(); 294 295 if (filter() && svgRoot != SVGRenderSupport::findTreeRootObject(*filter())) 296 filter()->layoutIfNeeded(); 297 298 if (markerStart() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerStart())) 299 markerStart()->layoutIfNeeded(); 300 301 if (markerMid() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerMid())) 302 markerMid()->layoutIfNeeded(); 303 304 if (markerEnd() && svgRoot != SVGRenderSupport::findTreeRootObject(*markerEnd())) 305 markerEnd()->layoutIfNeeded(); 306 } 307 286 308 void SVGResources::removeClientFromCache(RenderElement& renderer, bool markForInvalidation) const 287 309 { -
trunk/Source/WebCore/rendering/svg/SVGResources.h
r188647 r195411 36 36 class RenderSVGResourceMarker; 37 37 class RenderSVGResourceMasker; 38 class RenderSVGRoot; 38 39 class SVGRenderStyle; 39 40 … … 45 46 46 47 bool buildCachedResources(const RenderElement&, const RenderStyle&); 48 void layoutDifferentRootIfNeeded(const RenderSVGRoot*); 47 49 48 50 // Ordinary resources 49 RenderSVGResourceClipper* clipper() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->clipper : 0; } 50 RenderSVGResourceMarker* markerStart() const { return m_markerData ? m_markerData->markerStart : 0; } 51 RenderSVGResourceMarker* markerMid() const { return m_markerData ? m_markerData->markerMid : 0; } 52 RenderSVGResourceMarker* markerEnd() const { return m_markerData ? m_markerData->markerEnd : 0; } 53 RenderSVGResourceMasker* masker() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->masker : 0; } 54 55 RenderSVGResourceFilter* filter() const 56 { 57 if (m_clipperFilterMaskerData) 58 return m_clipperFilterMaskerData->filter; 59 return 0; 60 } 51 RenderSVGResourceClipper* clipper() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->clipper : nullptr; } 52 RenderSVGResourceMarker* markerStart() const { return m_markerData ? m_markerData->markerStart : nullptr; } 53 RenderSVGResourceMarker* markerMid() const { return m_markerData ? m_markerData->markerMid : nullptr; } 54 RenderSVGResourceMarker* markerEnd() const { return m_markerData ? m_markerData->markerEnd : nullptr; } 55 RenderSVGResourceMasker* masker() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->masker : nullptr; } 56 RenderSVGResourceFilter* filter() const { return m_clipperFilterMaskerData ? m_clipperFilterMaskerData->filter : nullptr; } 61 57 62 58 // Paint servers 63 RenderSVGResourceContainer* fill() const { return m_fillStrokeData ? m_fillStrokeData->fill : 0; }64 RenderSVGResourceContainer* stroke() const { return m_fillStrokeData ? m_fillStrokeData->stroke : 0; }59 RenderSVGResourceContainer* fill() const { return m_fillStrokeData ? m_fillStrokeData->fill : nullptr; } 60 RenderSVGResourceContainer* stroke() const { return m_fillStrokeData ? m_fillStrokeData->stroke : nullptr; } 65 61 66 62 // Chainable resources - linked through xlink:href
Note: See TracChangeset
for help on using the changeset viewer.