Bug 263577
| 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 | ||
Simon Fraser (smfr)
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
<rdar://problem/117388247>
Darin Adler
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
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
Pull request: https://github.com/WebKit/WebKit/pull/19702
EWS
Committed 270423@main (a3bf74c86750): <https://commits.webkit.org/270423@main>
Reviewed commits have been landed. Closing PR #19702 and removing active labels.