Bug 115766 - really fixed the memory leak
Summary: really fixed the memory leak
Status: RESOLVED FIXED
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
URL:
Keywords:
Depends on: 115760
Blocks:
  Show dependency tree / graph
 
Reported: 2013-05-07 15:41 PDT by Alex Christensen
Modified: 2013-05-07 23:15 PDT (History)
5 users (show)

See Also:

Attachments
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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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]
Patch
Comment 2 Mark Rowe (bdash) 2013-05-07 15:58:57 PDT 
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."
Comment 3 Alex Christensen 2013-05-07 16:15:30 PDT 
Created attachment 200989 [details]
Patch
Comment 4 WebKit Commit Bot 2013-05-07 18:35:07 PDT 
Comment on attachment 200989 [details]
Patch

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.