Title: [258278] trunk/Source/WebCore
- Revision
- 258278
- Author
- za...@apple.com
- Date
- 2020-03-11 13:22:25 -0700 (Wed, 11 Mar 2020)
Log Message
SVG filter triggers unstable layout.
https://bugs.webkit.org/show_bug.cgi?id=207444
rdar://problem/59297004
Reviewed by Simon Fraser.
SVG filter code marks DOM nodes dirty and schedules style recalc outside of the SVG root
while in layout. This could lead to unstable layout and cause battery drain.
(See webkit.org/b/208903)
* rendering/RenderLayer.cpp: Remove filterNeedsRepaint(). It's a dangerously misleading name and should
not be part of RenderLayer.
(WebCore::RenderLayer::calculateClipRects const):
* rendering/RenderLayer.h:
* rendering/RenderLayerFilters.cpp:
(WebCore::RenderLayerFilters::notifyFinished):
* rendering/svg/RenderSVGResourceContainer.cpp:
(WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation):
(WebCore::RenderSVGResourceContainer::markAllClientLayersForInvalidation):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (258277 => 258278)
--- trunk/Source/WebCore/ChangeLog 2020-03-11 20:19:07 UTC (rev 258277)
+++ trunk/Source/WebCore/ChangeLog 2020-03-11 20:22:25 UTC (rev 258278)
@@ -1,3 +1,25 @@
+2020-03-11 Zalan Bujtas <za...@apple.com>
+
+ SVG filter triggers unstable layout.
+ https://bugs.webkit.org/show_bug.cgi?id=207444
+ rdar://problem/59297004
+
+ Reviewed by Simon Fraser.
+
+ SVG filter code marks DOM nodes dirty and schedules style recalc outside of the SVG root
+ while in layout. This could lead to unstable layout and cause battery drain.
+ (See webkit.org/b/208903)
+
+ * rendering/RenderLayer.cpp: Remove filterNeedsRepaint(). It's a dangerously misleading name and should
+ not be part of RenderLayer.
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/RenderLayer.h:
+ * rendering/RenderLayerFilters.cpp:
+ (WebCore::RenderLayerFilters::notifyFinished):
+ * rendering/svg/RenderSVGResourceContainer.cpp:
+ (WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation):
+ (WebCore::RenderSVGResourceContainer::markAllClientLayersForInvalidation):
+
2020-03-11 Antoine Quint <grao...@webkit.org>
[Mac wk2 Release] imported/w3c/web-platform-tests/web-animations/timing-model/animations/updating-the-finished-state.html flaky fail
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (258277 => 258278)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2020-03-11 20:19:07 UTC (rev 258277)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2020-03-11 20:22:25 UTC (rev 258278)
@@ -6965,16 +6965,6 @@
m_filters->buildFilter(renderer(), page().deviceScaleFactor(), renderer().settings().acceleratedFiltersEnabled() ? RenderingMode::Accelerated : RenderingMode::Unaccelerated);
}
-void RenderLayer::filterNeedsRepaint()
-{
- // We use the enclosing element so that we recalculate style for the ancestor of an anonymous object.
- if (Element* element = enclosingElement()) {
- // FIXME: This really shouldn't have to invalidate layer composition, but tests like css3/filters/effect-reference-delete.html fail if that doesn't happen.
- element->invalidateStyleAndLayerComposition();
- }
- renderer().repaint();
-}
-
IntOutsets RenderLayer::filterOutsets() const
{
if (m_filters)
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (258277 => 258278)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2020-03-11 20:19:07 UTC (rev 258277)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2020-03-11 20:22:25 UTC (rev 258278)
@@ -803,7 +803,6 @@
bool has3DTransform() const { return m_transform && !m_transform->isAffine(); }
bool hasTransformedAncestor() const { return m_hasTransformedAncestor; }
- void filterNeedsRepaint();
bool hasFilter() const { return renderer().hasFilter(); }
bool hasFilterOutsets() const { return !filterOutsets().isZero(); }
IntOutsets filterOutsets() const;
Modified: trunk/Source/WebCore/rendering/RenderLayerFilters.cpp (258277 => 258278)
--- trunk/Source/WebCore/rendering/RenderLayerFilters.cpp 2020-03-11 20:19:07 UTC (rev 258277)
+++ trunk/Source/WebCore/rendering/RenderLayerFilters.cpp 2020-03-11 20:22:25 UTC (rev 258278)
@@ -67,7 +67,11 @@
void RenderLayerFilters::notifyFinished(CachedResource&)
{
- m_layer.filterNeedsRepaint();
+ // FIXME: This really shouldn't have to invalidate layer composition,
+ // but tests like css3/filters/effect-reference-delete.html fail if that doesn't happen.
+ if (auto* enclosingElement = m_layer.enclosingElement())
+ enclosingElement->invalidateStyleAndLayerComposition();
+ m_layer.renderer().repaint();
}
void RenderLayerFilters::updateReferenceFilterClients(const FilterOperations& operations)
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp (258277 => 258278)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp 2020-03-11 20:19:07 UTC (rev 258277)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp 2020-03-11 20:22:25 UTC (rev 258278)
@@ -26,6 +26,7 @@
#include "SVGRenderingContext.h"
#include "SVGResourcesCache.h"
#include <wtf/IsoMallocInlines.h>
+#include <wtf/SetForScope.h>
#include <wtf/StackStats.h>
namespace WebCore {
@@ -91,10 +92,13 @@
void RenderSVGResourceContainer::markAllClientsForInvalidation(InvalidationMode mode)
{
+ // FIXME: Style invalidation should either be a pre-layout task or this function
+ // should never get called while in layout. See webkit.org/b/208903.
if ((m_clients.isEmpty() && m_clientLayers.isEmpty()) || m_isInvalidating)
return;
- m_isInvalidating = true;
+ SetForScope<bool> isInvalidating(m_isInvalidating, true);
+
bool needsLayout = mode == LayoutAndBoundariesInvalidation;
bool markForInvalidation = mode != ParentOnlyInvalidation;
auto* root = SVGRenderSupport::findTreeRootObject(*this);
@@ -116,8 +120,6 @@
}
markAllClientLayersForInvalidation();
-
- m_isInvalidating = false;
}
void RenderSVGResourceContainer::markAllClientLayersForInvalidation()
@@ -124,10 +126,23 @@
{
if (m_clientLayers.isEmpty())
return;
- if ((*m_clientLayers.begin())->renderer().renderTreeBeingDestroyed())
+
+ auto& document = (*m_clientLayers.begin())->renderer().document();
+ if (!document.view() || document.renderTreeBeingDestroyed())
return;
- for (auto* clientLayer : m_clientLayers)
- clientLayer->filterNeedsRepaint();
+
+ auto inLayout = document.view()->layoutContext().isInLayout();
+ for (auto* clientLayer : m_clientLayers) {
+ // FIXME: We should not get here while in layout. See webkit.org/b/208903.
+ // Repaint should also be triggered through some other means.
+ if (inLayout) {
+ clientLayer->renderer().repaint();
+ continue;
+ }
+ if (auto* enclosingElement = clientLayer->enclosingElement())
+ enclosingElement->invalidateStyleAndLayerComposition();
+ clientLayer->renderer().repaint();
+ }
}
void RenderSVGResourceContainer::markClientForInvalidation(RenderObject& client, InvalidationMode mode)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
