Bug 121642 - Safari crashes with 0 length repeating-radial-gradient CSS definition.
Summary: Safari crashes with 0 length repeating-radial-gradient CSS definition.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Critical
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-19 15:30 PDT by styu007
Modified: 2014-08-19 09:06 PDT (History)
8 users (show)

See Also:


Attachments
reduction case (270 bytes, text/html)
2013-09-20 10:03 PDT, zalan
no flags Details
Patch (6.43 KB, patch)
2013-09-21 07:04 PDT, zalan
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description styu007 2013-09-19 15:30:04 PDT
The browser tab crashes when using some specific repeating radial gradient on any element.

background-image: repeating-radial-gradient(closest-side circle at 0% 0%, #fff, #000);

I think this is because the closest side is equal to the center of the gradient. Any position above zero seems to work (even 0.1%).

No freeze with non repeating gradients.

You can try with the following fiddle (can safely load it, the CSS is applied on a button's click):
http://jsfiddle.net/styu/apxdz/
Comment 1 zalan 2013-09-20 08:34:37 PDT
1   0x10ed5fdc0 WTFCrash
2   0x10fe6394c WTF::VectorBufferBase<WebCore::GradientStop>::allocateBuffer(unsigned long)
3   0x10fe63884 WTF::Vector<WebCore::GradientStop, 0ul, WTF::CrashOnOverflow>::reserveCapacity(unsigned long)
4   0x10fe6380f WTF::Vector<WebCore::GradientStop, 0ul, WTF::CrashOnOverflow>::expandCapacity(unsigned long)
5   0x10fe635f4 WTF::Vector<WebCore::GradientStop, 0ul, WTF::CrashOnOverflow>::expandCapacity(unsigned long, WebCore::GradientStop*)
6   0x10fe63522 void WTF::Vector<WebCore::GradientStop, 0ul, WTF::CrashOnOverflow>::appendSlowCase<WebCore::GradientStop&>(WebCore::GradientStop&&&)
7   0x10fe62b2e void WTF::Vector<WebCore::GradientStop, 0ul, WTF::CrashOnOverflow>::append<WebCore::GradientStop&>(WebCore::GradientStop&&&)
8   0x10fe5e141 WebCore::CSSGradientValue::addStops(WebCore::Gradient*, WebCore::RenderObject*, WebCore::RenderStyle*, float)
9   0x10fe5cae3 WebCore::CSSRadialGradientValue::createGradient(WebCore::RenderObject*, WebCore::IntSize const&)
10  0x10fe5ac71 WebCore::CSSGradientValue::image(WebCore::RenderObject*, WebCore::IntSize const&)
11  0x10fe790a4 WebCore::CSSImageGeneratorValue::image(WebCore::RenderObject*, WebCore::IntSize const&)
12  0x111228a76 WebCore::StyleGeneratedImage::image(WebCore::RenderObject*, WebCore::IntSize const&) const
13  0x110e5f78c WebCore::RenderBoxModelObject::paintFillLayerExtended(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const*, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::InlineFlowBox*, WebCore::LayoutSize const&, WebCore::CompositeOperator, WebCore::RenderObject*)
14  0x110e3dcae WebCore::RenderBox::paintFillLayer(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const*, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::CompositeOperator, WebCore::RenderObject*)
15  0x110e3c206 WebCore::RenderBox::paintFillLayers(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const*, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::CompositeOperator, WebCore::RenderObject*)
16  0x110e3cc07 WebCore::RenderBox::paintBackground(WebCore::PaintInfo const&, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance)
17  0x110e3c8c9 WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
18  0x110dd8209 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
19  0x110dd5b2b WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
20  0x110dd7ca4 WebCore::RenderBlock::paintChild(WebCore::RenderBox*, WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool)
21  0x110dd78f7 WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool)
22  0x110dd7884 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
23  0x110dd83c8 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
24  0x110dd5b2b WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
25  0x110dd7ca4 WebCore::RenderBlock::paintChild(WebCore::RenderBox*, WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool)
26  0x110dd78f7 WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool)
27  0x110dd7884 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
28  0x110dd83c8 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
29  0x110dd5b2b WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&)
30  0x110efe55a WebCore::RenderLayer::paintForegroundForFragmentsWithPhase(WebCore::PaintPhase, WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow> const&, WebCore::GraphicsContext*, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int, WebCore::RenderObject*)
31  0x110efcfa1 WebCore::RenderLayer::paintForegroundForFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow> const&, WebCore::GraphicsContext*, WebCore::GraphicsContext*, WebCore::LayoutRect const&, bool, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int, WebCore::RenderObject*, bool, bool)
Comment 2 zalan 2013-09-20 10:03:01 PDT
Created attachment 212185 [details]
reduction case
Comment 3 zalan 2013-09-20 10:43:00 PDT
in CSSGradientValue::addStops(), gradientLength == 0 makes maxExtent->inf and we get stuck in a while(true) loop.
Comment 4 zalan 2013-09-21 07:04:48 PDT
Created attachment 212273 [details]
Patch
Comment 5 zalan 2013-09-21 07:08:48 PDT
For some reason, I can't generate pixel test results for this new test case. The generate png comes up blank while when the length is changed to some non-zero value, the repeating radial gradient renders fine. (the same time, Safari renders the zero length value gradient as expected -solid blue color). Any idea?
Comment 6 Simon Fraser (smfr) 2013-09-22 21:29:24 PDT
What does Safari do if you disable accelerated drawing?
Comment 7 zalan 2013-09-24 08:54:50 PDT
(In reply to comment #6)
> What does Safari do if you disable accelerated drawing?
It doesn't render the radial gradients. 
Is it about CGContextDrawRadialGradient not being able to handle zero sized gradients? Out of curiosity, I changed Gradient::paint() so that it called CGContextDrawLinearGradient() instead and the (one-stop solid color)blue boxes showed up fine.
Comment 8 styu007 2013-09-24 10:43:28 PDT
I have already reported this bug to the chromium bug tracker, and they made a patch:
https://code.google.com/p/chromium/issues/detail?id=295126#c7
Comment 9 styu007 2013-09-24 10:45:04 PDT
You may not see the issue, because it is flagged as security issue, so here is the patch they made:
--- trunk/Source/core/css/CSSGradientValue.cpp	2013/09/23 23:48:43	158219
+++ trunk/Source/core/css/CSSGradientValue.cpp	2013/09/23 23:56:31	158220
@@ -276,7 +276,7 @@
                 }
 
                 if (maxLengthForRepeat > gradientLength)
-                    maxExtent = maxLengthForRepeat / gradientLength;
+                    maxExtent = gradientLength > 0 ? maxLengthForRepeat / gradientLength : 0;
             }
 
             size_t originalNumStops = numStops;
Comment 10 zalan 2013-09-24 10:58:16 PDT
(In reply to comment #9)
> You may not see the issue, because it is flagged as security issue, so here is the patch they made:
> --- trunk/Source/core/css/CSSGradientValue.cpp    2013/09/23 23:48:43    158219
> +++ trunk/Source/core/css/CSSGradientValue.cpp    2013/09/23 23:56:31    158220
> @@ -276,7 +276,7 @@
>                  }
> 
>                  if (maxLengthForRepeat > gradientLength)
> -                    maxExtent = maxLengthForRepeat / gradientLength;
> +                    maxExtent = gradientLength > 0 ? maxLengthForRepeat / gradientLength : 0;
>              }
> 
>              size_t originalNumStops = numStops;

I am not so sure if that's the right fix (I was considering something similar though I was going to have maxExtent=1 (default value)). I'd rather have an explicit path for the zero size case (similarly to the zero stop case) as opposed to going through some generic code and bail out randomly just because we set some value to default (or in this case to 0??). Highly error prone.
Comment 11 Darin Adler 2014-08-19 09:06:54 PDT
Comment on attachment 212273 [details]
Patch

I’d like to see a better test. A reference test that compares a gradient with a solid color rather than just a “this didn’t crash” test.