[Webkit-unassigned] [Bug 25703] Stack overflow crash rendering element with mega-huge number of background layers

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25703


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57333|review?                     |review-
               Flag|                            |




--- Comment #7 from David Levin <levin at chromium.org>  2010-06-01 10:22:16 PST ---
(From update of attachment 57333)
WebKit uses 4 space indent.

Note when I made comments below, I often only pointed out one instance of the issue but it may occur in several places, so please check your code throughout for the same issue.


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +        Test: css3/many-background-layers.html
> +
> +        * rendering/RenderBox.cpp:
> +        (WebCore::RenderBox::paintFillLayers):

Note that this is where per function comments go.

> +        * rendering/style/FillLayer.h:
> +        (WebCore::FillLayer::hasImage):
> +        (WebCore::FillLayer::hasFixedImage):
> +
>  2010-05-27  Kwang Yul Seo  <skyul at company100.net>
>  
>          Reviewed by Darin Adler.

> diff --git a/WebCore/rendering/RenderBox.cpp b/WebCore/rendering/RenderBox.cpp
>  void RenderBox::paintFillLayers(const PaintInfo& paintInfo, const Color& c, const FillLayer* fillLayer, int tx, int ty, int width, int height, CompositeOperator op, RenderObject* backgroundObject)
>  {
> -    if (!fillLayer)
> -        return;
> +    WTF::Vector<const FillLayer*, 8> layers;

Why 8? (Ideally the ChangeLog would have a comment about this function and the magic number "8".)


> +    for (size_t i = layers.size(); i > 0; --i)
> +      paintFillLayer(paintInfo, c, layers[i-1], tx, ty, width, height, op, backgroundObject);

There should be spaces around operators: "layers[i - 1]"



> diff --git a/WebCore/rendering/style/FillLayer.h b/WebCore/rendering/style/FillLayer.h
>      bool hasImage() const
>      {
> -        if (m_image)
> +        for (const FillLayer *layer = this; layer; layer = layer->m_next)

This should have {} around the body (see http://webkit.org/coding/coding-style.html).

The * is in the wrong place (should be "FillLayer* layer").



Doesn't ~FillLayer have the same problem?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.