Changeset 285785 in webkit

Timestamp:

Nov 13, 2021, 4:19:27 PM (5 years ago)

Author:

Said Abou-Hallawa

Message:

REGRESSION (r285618): [mac-wk1] ASSERTION FAILED: cgContext == [currentContext CGContext]
​https://bugs.webkit.org/show_bug.cgi?id=233008
rdar://85311948

Reviewed by Wenson Hsieh.

Source/WebCore:

The assertion fails when loading the expected html page because one of
the elements has a CSS filter named "(#noop)" but the filter "noop" is
not defined.

The reason for the assertion to fail is we switch the PaintInfo to the
context of the CSSFilter::sourceImage() and we do not restore it back.
In fact CSSFilter::buildFilterFunctions() should fail since the filter
has only a reference filter and this reference filter does not exit. The
bug is CSSFilter::buildFilterFunctions() does not fail in this case.

Before r285618, CSSFilter::buildFilterFunctions() was not adding
SourceGraphic to m_functions. It was added as the input of the first
FilterEffect. This was fine since we were applying the lastEffect which
goes backward till it reaches the SourceGraphic.

But the plan is to apply the FilterFunctions from the first to the last
without having to go backward, so we need to add the SourceGraphic to
m_functions explicitly. The bug happens when no FilterFunction is built
successfully and we return 'true' because m_functions is not empty. It
has the SourceGraphic.

The fix is to add the SourceGraphic only when there is at least another
FilterFunction will be added to m_functions.

• rendering/CSSFilter.cpp:

(WebCore::CSSFilter::buildFilterFunctions):

LayoutTests:

Unskip the test http/tests/css/filters-on-iframes-transform.html which
was skipped in r285656.

• platform/mac-wk1/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/CSSFilter.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-13  Said Abou-Hallawa  <said@apple.com>
+
+        REGRESSION (r285618): [mac-wk1] ASSERTION FAILED: cgContext == [currentContext CGContext]
+        https://bugs.webkit.org/show_bug.cgi?id=233008
+        rdar://85311948
+
+        Reviewed by Wenson Hsieh.
+
+        Unskip the test http/tests/css/filters-on-iframes-transform.html which 
+        was skipped in r285656.
+
+        * platform/mac-wk1/TestExpectations:
+
 2021-11-13  Simon Fraser  <simon.fraser@apple.com>
 

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -1517 +1517 @@
 webkit.org/b/232585 [ Catalina Debug ] media/track/track-element-load-event.html [ Pass Crash ]
 
-webkit.org/b/233008 [ Debug ] http/tests/css/filters-on-iframes-transform.html [ Skip ]
-
 # DumpRenderTree doesn't invoke inspector instrumentation when repainting.
 webkit.org/b/232852 inspector/page/setShowPaintRects.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-13  Said Abou-Hallawa  <said@apple.com>
+
+        REGRESSION (r285618): [mac-wk1] ASSERTION FAILED: cgContext == [currentContext CGContext]
+        https://bugs.webkit.org/show_bug.cgi?id=233008
+        rdar://85311948
+
+        Reviewed by Wenson Hsieh.
+
+        The assertion fails when loading the expected html page because one of
+        the elements has a CSS filter named "(#noop)" but the filter "noop" is
+        not defined.
+
+        The reason for the assertion to fail is we switch the PaintInfo to the
+        context of the CSSFilter::sourceImage() and we do not restore it back.
+        In fact CSSFilter::buildFilterFunctions() should fail since the filter
+        has only a reference filter and this reference filter does not exit. The
+        bug is CSSFilter::buildFilterFunctions() does not fail in this case.
+
+        Before r285618, CSSFilter::buildFilterFunctions() was not adding 
+        SourceGraphic to m_functions. It was added as the input of the first
+        FilterEffect. This was fine since we were applying the lastEffect which
+        goes backward till it reaches the SourceGraphic.
+
+        But the plan is to apply the FilterFunctions from the first to the last
+        without having to go backward, so we need to add the SourceGraphic to
+        m_functions explicitly. The bug happens when no FilterFunction is built
+        successfully and we return 'true' because m_functions is not empty. It
+        has the SourceGraphic.
+
+        The fix is to add the SourceGraphic only when there is at least another 
+        FilterFunction will be added to m_functions.
+
+        * rendering/CSSFilter.cpp:
+        (WebCore::CSSFilter::buildFilterFunctions):
+
 2021-11-13  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/CSSFilter.cpp

@@ -243 +243 @@
 
     RefPtr<FilterEffect> previousEffect = SourceGraphic::create(*this);
-    m_functions.append({ *previousEffect });
-
     RefPtr<SVGFilter> filter;
     
@@ -304 +302 @@
         }
 
+        if ((filter || effect) && m_functions.isEmpty()) {
+            ASSERT(previousEffect->filterType() == FilterEffect::Type::SourceGraphic);
+            m_functions.append({ *previousEffect });
+        }
+        
         if (filter) {
             effect = filter->lastEffect();