Don't treat shorthand filters as errors on SVG content

We don't support filter shorthands yet, so we shouldn't treat them as errors. Fix up the hasFilter() condition to also check if it's a filter that we pretend we can handle. Also straighten out the code-flow in applyFilterIfNecessary. BUG=645995 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2349743003 Cr-Commit-Position: refs/heads/master@{#419267}

Don't treat shorthand filters as errors on SVG content
We don't support filter shorthands yet, so we shouldn't treat them as errors. Fix up the hasFilter() condition to also check if it's a filter that we pretend we can handle. Also straighten out the code-flow in applyFilterIfNecessary. BUG=645995 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2349743003 Cr-Commit-Position: refs/heads/master@{#419267}
05c8461d · fs · Commit bot · a2525d33 · 05c8461d · 05c8461d
Commit 05c8461d authored Sep 16, 2016 by fs Committed by Commit bot Sep 16, 2016
5 changed files
--- a/third_party/WebKit/LayoutTests/animations/animated-filter-svg-element-expected.html
+++ b/third_party/WebKit/LayoutTests/animations/animated-filter-svg-element-expected.html
 <!doctype html>
 <div style="width: 100px; height: 100px; background-color: blue; backface-visibility: hidden; filter: blur(10px)"></div>
+<div style="width: 100px; height: 100px; background-color: blue"></div>
--- a/third_party/WebKit/LayoutTests/animations/animated-filter-svg-element.html
+++ b/third_party/WebKit/LayoutTests/animations/animated-filter-svg-element.html
@@ -10,11 +10,12 @@
  }

  #root, #child {
+    display: block;
    animation: blur-animation 1s infinite;
  }
 </style>

-<svg id="root">
+<svg id="root" height="100">
  <rect fill="blue" x="0" y="0" height="100" width="100" />
 </svg>


--- a/third_party/WebKit/LayoutTests/svg/filters/svg-element-invalid-filter-expected.html
+++ b/third_party/WebKit/LayoutTests/svg/filters/svg-element-invalid-filter-expected.html
@@ -57,6 +57,14 @@

 <!-- SVG child element: the first filter is valid, the rest are not. -->

+<svg viewBox="0 0 50 50">
+  <rect fill="lime" x="0" y="0" height="50" width="50"/>
+</svg>
+
+<svg viewBox="0 0 50 50">
+  <rect fill="lime" x="0" y="0" height="50" width="50"/>
+</svg>
+
 <svg viewBox="0 0 50 50">
  <rect fill="lime" x="0" y="0" height="50" width="50"/>
 </svg>

--- a/third_party/WebKit/LayoutTests/svg/filters/svg-element-invalid-filter.html
+++ b/third_party/WebKit/LayoutTests/svg/filters/svg-element-invalid-filter.html
@@ -66,12 +66,12 @@
 </svg>

 <svg viewBox="0 0 50 50">
-  <rect fill="red" x="0" y="0" height="50" width="50"
+  <rect fill="lime" x="0" y="0" height="50" width="50"
       style="filter: url(#filter) blur(0px)"/>
 </svg>

 <svg viewBox="0 0 50 50">
-  <rect fill="red" x="0" y="0" height="50" width="50"
+  <rect fill="lime" x="0" y="0" height="50" width="50"
       style="filter: blur(0px)"/>
 </svg>


--- a/third_party/WebKit/Source/core/paint/SVGPaintContext.cpp
+++ b/third_party/WebKit/Source/core/paint/SVGPaintContext.cpp
@@ -134,27 +134,38 @@ bool SVGPaintContext::applyMaskIfNecessary(SVGResources* resources)
    return true;
 }

+static bool hasReferenceFilterOnly(const ComputedStyle& style)
+{
+    if (!style.hasFilter())
+        return false;
+    const FilterOperations& operations = style.filter();
+    if (operations.size() != 1)
+        return false;
+    return operations.at(0)->type() == FilterOperation::REFERENCE;
+}
+
 bool SVGPaintContext::applyFilterIfNecessary(SVGResources* resources)
 {
-    if (!resources) {
-        if (m_object.style()->hasFilter())
-            return false;
-    } else if (LayoutSVGResourceFilter* filter = resources->filter()) {
-        m_filterRecordingContext = wrapUnique(new SVGFilterRecordingContext(paintInfo().context));
-        m_filter = filter;
-        GraphicsContext* filterContext = SVGFilterPainter(*filter).prepareEffect(m_object, *m_filterRecordingContext);
-        if (!filterContext)
-            return false;
+    if (!resources)
+        return !hasReferenceFilterOnly(m_object.styleRef());

-        // Because the filter needs to cache its contents we replace the context
-        // during filtering with the filter's context.
-        m_filterPaintInfo = wrapUnique(new PaintInfo(*filterContext, m_paintInfo));
+    LayoutSVGResourceFilter* filter = resources->filter();
+    if (!filter)
+        return true;
+    m_filterRecordingContext = wrapUnique(new SVGFilterRecordingContext(paintInfo().context));
+    m_filter = filter;
+    GraphicsContext* filterContext = SVGFilterPainter(*filter).prepareEffect(m_object, *m_filterRecordingContext);
+    if (!filterContext)
+        return false;

-        // Because we cache the filter contents and do not invalidate on paint
-        // invalidation rect changes, we need to paint the entire filter region
-        // so elements outside the initial paint (due to scrolling, etc) paint.
-        m_filterPaintInfo->m_cullRect.m_rect = LayoutRect::infiniteIntRect();
-    }
+    // Because the filter needs to cache its contents we replace the context
+    // during filtering with the filter's context.
+    m_filterPaintInfo = wrapUnique(new PaintInfo(*filterContext, m_paintInfo));
+
+    // Because we cache the filter contents and do not invalidate on paint
+    // invalidation rect changes, we need to paint the entire filter region
+    // so elements outside the initial paint (due to scrolling, etc) paint.
+    m_filterPaintInfo->m_cullRect.m_rect = LayoutRect::infiniteIntRect();
    return true;
 }