Bug 10259 – REGRESSION: Leaks reported by buildbot

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Geoffrey Garen]

149 Node
11 RenderObject
1 Frame
39 KJS::Node

Looks like this began with r15791 (???).

Index: WebCore/ChangeLog
===================================================================
--- WebCore/ChangeLog   (revision 15790)
+++ WebCore/ChangeLog   (revision 15792)
@@ -1,3 +1,16 @@
[phone]  David Hyatt  <[e-mail]>
+
+        Fix for bug 10229, don't bother trying to clear when no floats are
+        present.  I suspect there's still a bug in the math that follows, but
+        this fix is safer in that it just does the obvious thing (and doesn't
+        compute any clearance if no floats are even around).
+
+        Reviewed by maciej
+
+        * ChangeLog:
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::getClearDelta):
+
 2006-08-03  Justin Garcia  <[e-mail]>
 
         Reviewed by harrison

[Geoffrey Garen]

Dave says that this change just replaced a function call with its implementation, so it looks like the buildbot is blaming this change for an independent leak.

[Darin Adler]

Here's what I see:

    1) many leaks of the InlineBox created by RenderListMarker::createInlineBox -- this leak *does* seem like it was caused by a recent change

    2) a failure in test dom/html/level2/html/HTMLBaseElement02.html

    3) an entire frame leak as Geoff's reporting here -- not sure what caused this

[Darin Adler]

(In reply to comment #2)
>     1) many leaks of the InlineBox created by RenderListMarker::createInlineBox
> -- this leak *does* seem like it was caused by a recent change

I'm trying and having trouble reproducing this locally.

[Darin Adler]

Now I can reproduce it fine.

[Darin Adler]

Created attachment 9896 [details]
patch that fixes some of the biggest leaks I see

[Geoffrey Garen]

Comment on attachment 9896 [details]
patch that fixes some of the biggest leaks I see

Patch looks good to me, but I'll wait for Hyatt.

What do you think about changing this:

+    if (!handle) {
+        delete this;
+        return false;
+    }
 
      return true;

to this:

if (handle)
    return true;

?

[Darin Adler]

(In reply to comment #6)
> What do you think about changing this:
> 
> +    if (!handle) {
> +        delete this;
> +        return false;
> +    }
> 
>       return true;
> 
> to this:
> 
> if (handle)
>     return true;
> 
> ?

Seems OK. I handled the error case first because I think of it as the exceptional case, but I can see sharing the code instead.

[Dave Hyatt]

Comment on attachment 9896 [details]
patch that fixes some of the biggest leaks I see

r=me

[Darin Adler]

Comment on attachment 9896 [details]
patch that fixes some of the biggest leaks I see

Committed revision 15857.

Clearing review flag so we can use this bug to track fixing the rest of the leaks (including the ones that Geoff was originally talking about, I think).

[Stephanie Lewis]

In Radar
<rdar://problem/4946778>

[Mark Rowe (bdash)]

The leaks reported here are long-gone.  A new bug should be opened for any new leaks that are noticed.