Summary: | Leak of NSArray (4.25 Kbytes) in com.apple.WebKit.WebContent running WebKit layout tests on iOS Simulator | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, darin, lforschler, mmaxfield, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=194761 | ||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2019-02-01 16:18:27 PST
Created attachment 360923 [details]
Patch v1
Comment on attachment 360923 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=360923&action=review > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:163 > CFArrayRef errors = nullptr; > CTFontManagerUnregisterFontsForURLs(static_cast<CFArrayRef>(fontsToRemove), kCTFontManagerScopeProcess, &errors); > + if (errors) { > + for (id error in (__bridge NSArray *)errors) > + NSLog(@"%@", (__bridge CFErrorRef)error); > + CFRelease(errors); > + } A better fix is to get rid of the "errors" local variable, and pass "nullptr" instead of "&errors" to CTFontManagerUnregisterFontsForURLs. Then there is no need for CFRelease. CTFontManagerUnregisterFontsForURLs won't generate an array of errors if we don’t pass a pointer to a place to put the CFArrayRef. Also, we should not land that logging code. Comment on attachment 360923 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=360923&action=review >> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:163 >> + } > > A better fix is to get rid of the "errors" local variable, and pass "nullptr" instead of "&errors" to CTFontManagerUnregisterFontsForURLs. Then there is no need for CFRelease. CTFontManagerUnregisterFontsForURLs won't generate an array of errors if we don’t pass a pointer to a place to put the CFArrayRef. > > Also, we should not land that logging code. Yep, I had assumed that since Myles originally added the `errors` variable that we cared about the errors returned from CTFontManagerUnregisterFontsForURLs(). Easy enough to fix. Created attachment 360964 [details]
Patch v2
Comment on attachment 360964 [details] Patch v2 Clearing flags on attachment: 360964 Committed r240900: <https://trac.webkit.org/changeset/240900> All reviewed patches have been landed. Closing bug. |