Bug 72723 - [CSS Filters] WebKit crashes when changing the filter
Summary: [CSS Filters] WebKit crashes when changing the filter
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: Alexandru Chiculita
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-11-18 06:39 PST by Alexandru Chiculita
Modified: 2011-11-23 06:15 PST (History)
4 users (show)

See Also:


Attachments
Test case (296 bytes, text/html)
2011-11-18 06:39 PST, Alexandru Chiculita
no flags Details
Crashlog (36.24 KB, text/plain)
2011-11-18 06:40 PST, Alexandru Chiculita
no flags Details
Patch V1 (5.79 KB, patch)
2011-11-18 07:07 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch for landing (34.45 KB, patch)
2011-11-23 02:56 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2011-11-18 06:39:03 PST
Created attachment 115797 [details]
Test case

When the -webkit-filter changes, WebKit crashes. See the attached test.
Comment 1 Alexandru Chiculita 2011-11-18 06:40:05 PST
Created attachment 115798 [details]
Crashlog
Comment 2 Alexandru Chiculita 2011-11-18 06:40:30 PST
I have a fix already. I will post a patch soon.
Comment 3 Alexandru Chiculita 2011-11-18 07:07:02 PST
Created attachment 115802 [details]
Patch V1
Comment 4 Simon Fraser (smfr) 2011-11-18 08:54:36 PST
Comment on attachment 115802 [details]
Patch V1

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

> LayoutTests/css3/filters/crash-filter-change.html:7
> +<img style="-webkit-filter: hue-rotate(90deg)" src="resources/reference.png">
> +<script>
> +    // force a layout
> +    document.body.offsetTop;
> +    var img = document.getElementsByTagName('img')[0];
> +    img.style['-webkit-filter'] = 'hue-rotate(10deg)';

Why not use CSS classes and change the className?

> Source/WebCore/rendering/FilterEffectRenderer.cpp:238
> +    setMaxEffectRects(m_sourceDrawingRegion);

Could this be tested separately? Does the right thing happen if the size of the filtered element changes?
Comment 5 Alexandru Chiculita 2011-11-18 09:00:13 PST
(In reply to comment #4)
> (From update of attachment 115802 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=115802&action=review
> 
> > LayoutTests/css3/filters/crash-filter-change.html:7
> > +<img style="-webkit-filter: hue-rotate(90deg)" src="resources/reference.png">
> > +<script>
> > +    // force a layout
> > +    document.body.offsetTop;
> > +    var img = document.getElementsByTagName('img')[0];
> > +    img.style['-webkit-filter'] = 'hue-rotate(10deg)';
> 
> Why not use CSS classes and change the className?

I can use classes. I will change before committing. 

> 
> > Source/WebCore/rendering/FilterEffectRenderer.cpp:238
> > +    setMaxEffectRects(m_sourceDrawingRegion);
> 
> Could this be tested separately? Does the right thing happen if the size of the filtered element changes?

Yes the problem was that the max size was zero after a rebuild of the filter pipeline and  the result would be transparent white. I think this test can also be an image test, like the other ones in the css3/filters, so that we can actually assert that the result is correct. I will convert it to pixel test.
Comment 6 Dean Jackson 2011-11-18 12:23:13 PST
Thanks Alex!
Comment 7 Radar WebKit Bug Importer 2011-11-18 12:24:19 PST
<rdar://problem/10471745>
Comment 8 Alexandru Chiculita 2011-11-23 02:56:30 PST
Created attachment 116336 [details]
Patch for landing
Comment 9 WebKit Review Bot 2011-11-23 06:15:04 PST
Comment on attachment 116336 [details]
Patch for landing

Clearing flags on attachment: 116336

Committed r101077: <http://trac.webkit.org/changeset/101077>
Comment 10 WebKit Review Bot 2011-11-23 06:15:09 PST
All reviewed patches have been landed.  Closing bug.