[webkit-reviews] review granted: [Bug 172545] [SVG] Leak in SVGAnimatedListPropertyTearOff : [Attachment 318591] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Aug 20 12:17:19 PDT 2017
Darin Adler <darin at apple.com> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 172545: [SVG] Leak in SVGAnimatedListPropertyTearOff
https://bugs.webkit.org/show_bug.cgi?id=172545
Attachment 318591: Patch
https://bugs.webkit.org/attachment.cgi?id=318591&action=review
--- Comment #38 from Darin Adler <darin at apple.com> ---
Comment on attachment 318591
--> https://bugs.webkit.org/attachment.cgi?id=318591
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=318591&action=review
> Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h:83
> + unsigned size = m_wrappers.size();
> + for (size_t i = 0; i < size; ++i) {
> + if (&property == m_wrappers.at(i)) {
> + m_wrappers.at(i) = nullptr;
> + break;
> + }
> + }
Mix of size_t and unsigned here is unusual. I would avoid that by writing this
as either a modern for loop or using Vector::find.
for (auto& wrapper : m_wrappers) {
if (&property == wrapper) {
wrapper = nullptr;
break;
}
}
Or:
size_t i = m_wrappers.find(&property);
if (i != notFound)
m_wrappers[i] = nullptr;
> LayoutTests/svg/animations/animation-leak-list-property-instances.html:42
> + // One gc() call is not enough and cause flakiness in some platforms.
Confusing and annoying! Also a grammatical error: should be "causes".
More information about the webkit-reviews
mailing list