Bug 115766 – really fixed the memory leak

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Alex Christensen]

I just uploaded a patch that "fixed" a memory leak in my WebKitTestRunner using webarchives, but I forgot to adopt the pointer.  This fixes that.

[Alex Christensen]

Created attachment 200988 [details]
Patch

[Mark Rowe (bdash)]

Comment on attachment 200988 [details]
Patch

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

r=me, but I think the ChangeLog should be improved before landing.

> Tools/ChangeLog:6
> +        I needed to adopt the pointer to fix the memory leak.
> +        https://bugs.webkit.org/show_bug.cgi?id=115766
> +
> +        Reviewed by NOBODY (OOPS!).

This would be better if the title stated what it's doing, and then a descriptive sentence later on mentioned the SVN revision in which the leak was introduced, the SVN revision in which you attempted to fix it earlier, and why that fix was incorrect. Something like:

<http://webkit.org/b/115766> Fix a memory leak introduced in r149692

Reviewed by NOBODY (OOPS!).

In r149692, the fix for <http://webkit.org/b/42324>, a call to WKBundleFrameCopyWebArchive was added without any matching call to WKRelease. An earlier attempted fix in r149697 introduced a RetainPtr but failed to adopt the object.

> Tools/ChangeLog:10
> +        really fixed memory leak

This should be a complete sentence. It wouldn't hurt to describe the fix either. "Fix the memory leak by switching to WKRetainPtr and adopting the returned object."

[Alex Christensen]

Created attachment 200989 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 200989 [details]
Patch

Clearing flags on attachment: 200989

Committed r149704: <http://trac.webkit.org/changeset/149704>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.