Title: [285785] trunk
Revision
285785
Author
s...@apple.com
Date
2021-11-13 16:19:27 -0800 (Sat, 13 Nov 2021)

Log 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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (285784 => 285785)


--- trunk/LayoutTests/ChangeLog	2021-11-13 23:28:17 UTC (rev 285784)
+++ trunk/LayoutTests/ChangeLog	2021-11-14 00:19:27 UTC (rev 285785)
@@ -1,3 +1,16 @@
+2021-11-13  Said Abou-Hallawa  <s...@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.fra...@apple.com>
 
         Implement UIScriptController.sendEventStream() for DumpRenderTree

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (285784 => 285785)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-11-13 23:28:17 UTC (rev 285784)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-11-14 00:19:27 UTC (rev 285785)
@@ -1516,7 +1516,5 @@
 
 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 ]

Modified: trunk/Source/WebCore/ChangeLog (285784 => 285785)


--- trunk/Source/WebCore/ChangeLog	2021-11-13 23:28:17 UTC (rev 285784)
+++ trunk/Source/WebCore/ChangeLog	2021-11-14 00:19:27 UTC (rev 285785)
@@ -1,3 +1,38 @@
+2021-11-13  Said Abou-Hallawa  <s...@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  <za...@apple.com>
 
         [LFC][IFC] Inline box end's padding/border/margin should be taken into account when computing horizontal position for bidi content

Modified: trunk/Source/WebCore/rendering/CSSFilter.cpp (285784 => 285785)


--- trunk/Source/WebCore/rendering/CSSFilter.cpp	2021-11-13 23:28:17 UTC (rev 285784)
+++ trunk/Source/WebCore/rendering/CSSFilter.cpp	2021-11-14 00:19:27 UTC (rev 285785)
@@ -242,8 +242,6 @@
     m_outsets = { };
 
     RefPtr<FilterEffect> previousEffect = SourceGraphic::create(*this);
-    m_functions.append({ *previousEffect });
-
     RefPtr<SVGFilter> filter;
     
     for (auto& operation : operations.operations()) {
@@ -303,6 +301,11 @@
             break;
         }
 
+        if ((filter || effect) && m_functions.isEmpty()) {
+            ASSERT(previousEffect->filterType() == FilterEffect::Type::SourceGraphic);
+            m_functions.append({ *previousEffect });
+        }
+        
         if (filter) {
             effect = filter->lastEffect();
             setupLastEffectProperties(*effect, consumer);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to