Bug 232628 - Leak of UUID in WebKit::ModelElementController::modelElementDidCreatePreview()
Summary: Leak of UUID in WebKit::ModelElementController::modelElementDidCreatePreview()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-02 09:59 PDT by David Kilzer (:ddkilzer)
Modified: 2021-11-03 14:33 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (1.78 KB, patch)
2021-11-02 10:03 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2021-11-02 09:59:52 PDT
Leak of UUID in WebKit::ModelElementController::modelElementDidCreatePreview().

This leaks a UUID object under MRR:

    auto preview = adoptNS([allocASVInlinePreviewInstance() initWithFrame:CGRectMake(0, 0, size.width(), size.height()) UUID:[[NSUUID alloc] initWithUUIDString:uuid]]);
Comment 1 Radar WebKit Bug Importer 2021-11-02 10:00:54 PDT
<rdar://problem/84935290>
Comment 2 David Kilzer (:ddkilzer) 2021-11-02 10:03:16 PDT
Created attachment 443101 [details]
Patch v1
Comment 3 Brent Fulgham 2021-11-03 11:17:39 PDT
Comment on attachment 443101 [details]
Patch v1

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

r=me

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:133
> +    auto preview = adoptNS([allocASVInlinePreviewInstance() initWithFrame:CGRectMake(0, 0, size.width(), size.height()) UUID:uuid.get()]);

Could also be:

auto preview = adoptNS([allocASVInlinePreviewInstance() initWithFrame:CGRectMake(0, 0, size.width(), size.height()) UUID:adoptNS([[NSUUID alloc] initWithUUIDString:uuid]).get()]);
Comment 4 David Kilzer (:ddkilzer) 2021-11-03 11:21:39 PDT
Comment on attachment 443101 [details]
Patch v1

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

>> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:133
>> +    auto preview = adoptNS([allocASVInlinePreviewInstance() initWithFrame:CGRectMake(0, 0, size.width(), size.height()) UUID:uuid.get()]);
> 
> Could also be:
> 
> auto preview = adoptNS([allocASVInlinePreviewInstance() initWithFrame:CGRectMake(0, 0, size.width(), size.height()) UUID:adoptNS([[NSUUID alloc] initWithUUIDString:uuid]).get()]);

Yep, I found the inline version much harder to parse as a human, which is why I created a separate variable.

Thanks for the review!
Comment 5 EWS 2021-11-03 11:26:18 PDT
Committed r285217 (243842@main): <https://commits.webkit.org/243842@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443101 [details].
Comment 6 David Kilzer (:ddkilzer) 2021-11-03 14:33:11 PDT
(In reply to EWS from comment #5)
> Committed r285217 (243842@main): <https://commits.webkit.org/243842@main>
> 
> All reviewed patches have been landed. Closing bug and clearing flags on
> attachment 443101 [details].

Follow-up build fix for internal bots (with newer clang):

https://trac.webkit.org/changeset/285229/webkit