Bug 222236

Summary: UserMediaPermissionRequestManagerProxy may be released while computing capture device list
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebRTCAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: ajuma, darin, ews-watchlist, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch none

Eric Carlson
Reported 2021-02-20 13:41:00 PST
UserMediaPermissionRequestManagerProxy::computeFilteredDeviceList should NULL-check after computing capture device list
Attachments
Patch (2.22 KB, patch)
2021-02-20 13:51 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.18 KB, patch)
2021-02-22 08:49 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (1.68 KB, patch)
2021-02-22 15:09 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Carlson
Comment 1 2021-02-20 13:41:28 PST
<rdar://74480265>
Eric Carlson
Comment 2 2021-02-20 13:51:22 PST
Created attachment 421110 [details] Patch
Peng Liu
Comment 3 2021-02-20 14:20:26 PST
Comment on attachment 421110 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421110&action=review > Source/WebKit/ChangeLog:3 > + UserMediaPermissionRequestManagerProxy::computeFilteredDeviceList may be released Sounds like UserMediaPermissionRequestManagerProxy may be released?
youenn fablet
Comment 4 2021-02-21 00:26:28 PST
Comment on attachment 421110 [details] Patch I guess the issue is log identifier, that is subtle… maybe this is an indication we should not site this in lambdas but just weakptr.
Eric Carlson
Comment 5 2021-02-22 08:49:11 PST
Created attachment 421198 [details] Patch for landing
EWS
Comment 6 2021-02-22 11:44:54 PST
Committed r273265: <https://commits.webkit.org/r273265> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421198 [details].
Darin Adler
Comment 7 2021-02-22 11:47:28 PST
Comment on attachment 421198 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=421198&action=review > Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:741 > - if (weakThis) > - m_hasFilteredDeviceList = !revealIdsAndLabels; > - > + weakThis->m_hasFilteredDeviceList = !revealIdsAndLabels; Why capture "this" but use "weakThis" here?
Eric Carlson
Comment 8 2021-02-22 15:09:10 PST
Reopening to attach new patch.
Eric Carlson
Comment 9 2021-02-22 15:09:11 PST
Created attachment 421244 [details] Patch
Eric Carlson
Comment 10 2021-02-22 15:10:18 PST
Comment on attachment 421198 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=421198&action=review >> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:741 >> + weakThis->m_hasFilteredDeviceList = !revealIdsAndLabels; > > Why capture "this" but use "weakThis" here? Oops, that is left over from a bigger refactoring I (tried to) undo to make this a more targeted change. I'll remove it in a follow.
EWS
Comment 11 2021-02-22 17:11:02 PST
Committed r273289: <https://commits.webkit.org/r273289> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421244 [details].
Eric Carlson
Comment 12 2021-03-19 10:45:34 PDT
*** Bug 223476 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.