| Summary: | AVVideoCaptureSource::cameraCaptureDeviceTypes() should return a RetainPtr | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
| Component: | WebRTC | Assignee: | youenn fablet <youennf> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bfulgham, darin, webkit-bug-importer, youennf |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
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. 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. Pull request: https://github.com/WebKit/WebKit/pull/19702 Committed 270423@main (a3bf74c86750): <https://commits.webkit.org/270423@main> Reviewed commits have been landed. Closing PR #19702 and removing active labels. |
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.