[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


Adam Barth <abarth at webkit.org> changed:

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




--- Comment #13 from Adam Barth <abarth at webkit.org>  2010-10-10 11:30:16 PST ---
(From update of attachment 59116)
View in context: https://bugs.webkit.org/attachment.cgi?id=59116&action=review

R- for readability.

> WebCore/rendering/RenderBox.cpp:761
> +    Vector<const FillLayer*, 8> layers; // Situations with more than 8 layers are extremely rare.

Do we have data to support this claim?

> WebCore/rendering/style/FillLayer.cpp:53
> +FillLayer::FillLayer(const FillLayer& o, bool shallowCopy)

Please don't use "o" as a variable name.  It looks too much like 0.

> WebCore/rendering/style/FillLayer.cpp:81
> +    const FillLayer* otherLayer = &o;

Perhaps "otherLayer" is a better name for o.

> WebCore/rendering/style/FillLayer.cpp:84
> +        thisLayer->m_next = new FillLayer(*otherLayer->m_next, true);

passing explicit true/false is hard to read.  :(

> WebCore/rendering/style/FillLayer.cpp:96
> +        delete layer;

Perhaps this should be an OwnPtr ?  Manual new/delete is bad times.

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