Bug 166328

Summary: MediaStream: ASSERTION FAILED: m_ids.size() == m_handles.size() in MediaDeviceSandboxExtensions
Product: WebKit Reporter: Nael Ouedraogo <nael.ouedp>
Component: MediaAssignee: Nael Ouedraogo <nael.ouedp>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, youennf
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Nael Ouedraogo
Reported 2016-12-21 08:39:04 PST
This assertion failure happens for port with SANDBOX_EXTENSION disabled. SandboxExtension::HandleArray is empty and HandleArray:size() always returns 0.
Attachments
Patch (2.58 KB, patch)
2016-12-21 08:48 PST, Nael Ouedraogo 		no flags
DetailsFormatted DiffDiff
Patch (6.29 KB, patch)
2016-12-22 05:13 PST, Nael Ouedraogo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Nael Ouedraogo
Comment 1 2016-12-21 08:48:04 PST
Created attachment 297590 [details] Patch
Eric Carlson
Comment 2 2016-12-21 09:18:55 PST
This will break the macOS/iOS builds because you haven't defined ENABLE_SANDBOX_EXTENSIONS for those builds. Instead of doing that, could you add an early return to the UserMediaPermissionRequestManager destructor when m_userMediaDeviceSandboxExtensions is empty?
Nael Ouedraogo
Comment 3 2016-12-21 10:18:06 PST
(In reply to comment #2) > This will break the macOS/iOS builds because you haven't defined > ENABLE_SANDBOX_EXTENSIONS for those builds. Ok it seems that the patch passed for mac and ios build bots. Maybe ENABLE_SANDBOX_EXTENSIONS is defined for those build ? > Instead of doing that, could you > add an early return to the UserMediaPermissionRequestManager destructor when > m_userMediaDeviceSandboxExtensions is empty? I tried but the same assertions fail. First ASSERTION FAILURE happens in MediaDeviceSandboxExtensions constructor when called in UserMediaProcessManager::willCreateMediaStream.
Eric Carlson
Comment 4 2016-12-21 11:12:01 PST
(In reply to comment #3) > (In reply to comment #2) > > This will break the macOS/iOS builds because you haven't defined > > ENABLE_SANDBOX_EXTENSIONS for those builds. > Ok it seems that the patch passed for mac and ios build bots. Maybe > ENABLE_SANDBOX_EXTENSIONS is defined for those build ? > You are correct, silly me! > > Instead of doing that, could you > > add an early return to the UserMediaPermissionRequestManager destructor when > > m_userMediaDeviceSandboxExtensions is empty? > > I tried but the same assertions fail. > > First ASSERTION FAILURE happens in MediaDeviceSandboxExtensions constructor > when called in UserMediaProcessManager::willCreateMediaStream. While this patch gets rid of the assertions, wouldn't it be more efficient to change UserMediaProcessManager to not create/delete the extensions and avoid the extra IPC unless ENABLE_SANDBOX_EXTENSIONS is defined?
Nael Ouedraogo
Comment 5 2016-12-22 05:13:43 PST
Created attachment 297652 [details] Patch
Nael Ouedraogo
Comment 6 2016-12-22 05:23:54 PST
> While this patch gets rid of the assertions, wouldn't it be more efficient > to change UserMediaProcessManager to not create/delete the extensions and > avoid the extra IPC unless ENABLE_SANDBOX_EXTENSIONS is defined? Yes ! Uploaded patch avoids to create and delete the extensions in UserMediaProcessManager. I think that GrantUserMediaDeviceSandboxExtensions and RevokeUserMediaDeviceSandboxExtensions messages should be also disabled. Is it correct?
Eric Carlson
Comment 7 2016-12-22 06:32:08 PST
(In reply to comment #6) > Uploaded patch avoids to create and delete the extensions in > UserMediaProcessManager. I think that GrantUserMediaDeviceSandboxExtensions > and RevokeUserMediaDeviceSandboxExtensions messages should be also disabled. > > Is it correct? That makes sense to me!
Nael Ouedraogo
Comment 8 2016-12-22 08:17:07 PST
Thanks for the review !
WebKit Commit Bot
Comment 9 2016-12-22 08:41:54 PST
Comment on attachment 297652 [details] Patch Clearing flags on attachment: 297652 Committed r210097: <http://trac.webkit.org/changeset/210097>
WebKit Commit Bot
Comment 10 2016-12-22 08:41:58 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.