Bug 115766 - really fixed the memory leak
Summary: really fixed the memory leak
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.8
: P4 Trivial
Assignee: Nobody
Depends on: 115760
  Show dependency treegraph
Reported: 2013-05-07 15:41 PDT by Alex Christensen
Modified: 2013-05-07 23:15 PDT (History)
5 users (show)

See Also:

Patch (1.54 KB, patch)
2013-05-07 15:46 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (1.83 KB, patch)
2013-05-07 16:15 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2013-05-07 15:41:59 PDT
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.
Comment 1 Alex Christensen 2013-05-07 15:46:37 PDT
Created attachment 200988 [details]
Comment 2 Mark Rowe (bdash) 2013-05-07 15:58:57 PDT
Comment on attachment 200988 [details]

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."
Comment 3 Alex Christensen 2013-05-07 16:15:30 PDT
Created attachment 200989 [details]
Comment 4 WebKit Commit Bot 2013-05-07 18:35:07 PDT
Comment on attachment 200989 [details]

Clearing flags on attachment: 200989

Committed r149704: <http://trac.webkit.org/changeset/149704>
Comment 5 WebKit Commit Bot 2013-05-07 18:35:09 PDT
All reviewed patches have been landed.  Closing bug.