RESOLVED FIXED263577
AVVideoCaptureSource::cameraCaptureDeviceTypes() should return a RetainPtr 
https://bugs.webkit.org/show_bug.cgi?id=263577
Summary AVVideoCaptureSource::cameraCaptureDeviceTypes() should return a RetainPtr
Simon Fraser (smfr)
Reported 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.
Attachments
Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-10-23 20:20:07 PDT
<rdar://problem/117388247>
Darin Adler
Comment 2 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.
Darin Adler
Comment 3 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.
youenn fablet
Comment 4 2023-10-30 00:54:40 PDT
Pull request: https://github.com/WebKit/WebKit/pull/19702
EWS
Comment 5 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.
Note You need to log in before you can comment on or make changes to this bug.