Bug 57478 – REGRESSION (r82400): Leaks seen beneath CSSParser::createFloatingVectorSelector when parsing UA stylesheet

• Component: CSS
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

To see the leaks:

1. Go to <http://build.webkit.org/LeaksViewer/?url=http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r82400%20(15899)/>
2. Dig into malloc_zone_malloc > malloc -> WTF::fastMalloc

The first leak listed here is an allocation inside Vector. If you look a little lower down you'll see the Vector itself is leaking (as indicated by the Vector<OwnPtr<CSSParserSelector> >::operator new leak).

Other evidence:

r82398, 2439 leaks:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Leaks/builds/15898

r82400, 3186 leaks:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Leaks/builds/15899

[Adam Roben (:aroben)]

r82400 added uses of :-webkit-any to the UA stylesheet, so my guess is that the :-webkit-any implementation is leaky.

[Adam Roben (:aroben)]

<rdar://problem/9209334>

[Yuta Kitamura]

The same leaks are observed in Chromium, too.
<http://code.google.com/p/chromium/issues/detail?id=77885>

[Ojan Vafai]

Created attachment 87665 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 87665 [details]
Patch

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

> Source/WebCore/css/CSSGrammar.y:1152
> +            OwnPtr<Vector<OwnPtr<CSSParserSelector> > > selectorVector = p->sinkFloatingSelectorVector($4);
> +            $$->adoptSelectorVector(*selectorVector.get());

No need for the local variable, or the .get().

> Source/WebCore/css/CSSParser.h:188
>          Vector<OwnPtr<CSSParserSelector> >* createFloatingSelectorVector();

Seems like we should change createFloatingSelectorVector to return a PassOwnPtr at some point.

[Ojan Vafai]

(In reply to comment #5)
> (From update of attachment 87665 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=87665&action=review
> > Source/WebCore/css/CSSParser.h:188
> >          Vector<OwnPtr<CSSParserSelector> >* createFloatingSelectorVector();
> 
> Seems like we should change createFloatingSelectorVector to return a PassOwnPtr at some point.

Maybe. I was just cargo culting createFloatingSelector. :)

[Ojan Vafai]

Committed r82545: <http://trac.webkit.org/changeset/82545>

[WebKit Review Bot]

http://trac.webkit.org/changeset/82545 might have broken GTK Linux 32-bit Debug

[Ojan Vafai]

(In reply to comment #8)
> http://trac.webkit.org/changeset/82545 might have broken GTK Linux 32-bit Debug

Looks like a false positive. This bot hasn't successfully run the tests in hours.

[Adam Roben (:aroben)]

Looks like this worked:

r82544, 3282 leaks:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Leaks/builds/15924

r82547, 2463 leaks:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Leaks/builds/15925

[Adam Roben (:aroben)]

And Leaks Viewer says the leaks are gone, too.