Bug 263577 - AVVideoCaptureSource::cameraCaptureDeviceTypes() should return a RetainPtr
Summary: AVVideoCaptureSource::cameraCaptureDeviceTypes() should return a RetainPtr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-10-23 20:19 PDT by Simon Fraser (smfr)
Modified: 2023-11-08 23:14 PST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2023-10-23 20:19:59 PDT
AVVideoCaptureSource::cameraCaptureDeviceTypes() return a pointer to a retained singleton NSMutableArray. Code like AVVideoCaptureSource::facingModeFitnessScoreAdjustment() uses the raw return value.

However, AVCaptureDeviceManager adopts the return value into a RetainPtr here:
    , m_avCaptureDeviceTypes(adoptNS(AVVideoCaptureSource::cameraCaptureDeviceTypes()))

so when that RetainPtr gets destroyed, it can release the last reference to the static NSMutableArray.

This would all be cleaner if `cameraCaptureDeviceTypes()` returned a RetainPtr.
Comment 1 Radar WebKit Bug Importer 2023-10-23 20:20:07 PDT
<rdar://problem/117388247>
Comment 2 Darin Adler 2023-10-25 09:40:49 PDT
Iā€™m not sure this is the change we should make. If cameraCaptureDeviceTypes returns a retained singleton pointer, then we could just stop doing adoptNS on the value it returns. RetainPtr is OK, but unnecessary.
Comment 3 Darin Adler 2023-10-25 09:43:01 PDT
I suppose functions returning RetainPtr are unambiguous and harder to use wrong. But also, I would like a discipline where adoptNS is not scattered in code, and kept to the boundaries between our C++ code and the underlying platform code. Maybe we should have a rule that all adoptNS and adoptCF have to be in wrapper files that just wrap the ownership and are part of the platform interface, not in implementation files mixed with other implementation details.
Comment 4 youenn fablet 2023-10-30 00:54:40 PDT
Pull request: https://github.com/WebKit/WebKit/pull/19702
Comment 5 EWS 2023-11-08 23:14:30 PST
Committed 270423@main (a3bf74c86750): <https://commits.webkit.org/270423@main>

Reviewed commits have been landed. Closing PR #19702 and removing active labels.