Bug 106796 - [TexMap] Composited CSS shaders crash when using non-GL TextureMapper
Summary: [TexMap] Composited CSS shaders crash when using non-GL TextureMapper
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-14 07:43 PST by Allan Sandfeld Jensen
Modified: 2013-01-16 03:01 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.03 KB, patch)
2013-01-15 04:47 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (2.41 KB, patch)
2013-01-15 08:45 PST, Allan Sandfeld Jensen
noam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2013-01-14 07:43:46 PST
If custom CSS filter aka CSS shaders are used together with animations, they will fail to work or crash.

The problem appears to be caused by BitmapTextureImageBuffer trying to build a FilterEffectRenderer and passing a null pointer RenderObject. This will work for all other filters than custom ones. In custom ones it will fail to build a valid filter, which then causes FilterEffectRenderer::apply() to crash.
Comment 1 Allan Sandfeld Jensen 2013-01-15 04:47:50 PST
Created attachment 182739 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-01-15 07:56:47 PST
Comment on attachment 182739 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182739&action=review

> Source/WebCore/rendering/FilterEffectRenderer.cpp:424
> +    if (!effect)
> +        return;

By looking at the rest of FilterEffectRenderer, it looks like lastEffect is expected not to be null, so I'm not sure if this is the best fix longer term.
It feels like none of this code makes sense if the list of effect is empty. Is there a way you could fix it at the caller?
Comment 3 Allan Sandfeld Jensen 2013-01-15 08:02:44 PST
(In reply to comment #2)
> (From update of attachment 182739 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182739&action=review
> 
> > Source/WebCore/rendering/FilterEffectRenderer.cpp:424
> > +    if (!effect)
> > +        return;
> 
> By looking at the rest of FilterEffectRenderer, it looks like lastEffect is expected not to be null, so I'm not sure if this is the best fix longer term.
> It feels like none of this code makes sense if the list of effect is empty. Is there a way you could fix it at the caller?

The rest of the patch is fixing it at the caller, so this change on it is own is not needed to fix the crash (though it was here it crashed), but since a FilterEffectRenderer can be left "invalid" after a failed build() call, I felt it would be a good idea not to crash on a state that FilterEffectRenderer has left itself in.
Comment 4 Allan Sandfeld Jensen 2013-01-15 08:45:49 PST
Created attachment 182782 [details]
Patch
Comment 5 Allan Sandfeld Jensen 2013-01-16 03:01:37 PST
Committed r139864: <http://trac.webkit.org/changeset/139864>