Bug 166328 - MediaStream: ASSERTION FAILED: m_ids.size() == m_handles.size() in MediaDeviceSandboxExtensions
Summary: MediaStream: ASSERTION FAILED: m_ids.size() == m_handles.size() in MediaDevic...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nael Ouedraogo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-21 08:39 PST by Nael Ouedraogo
Modified: 2016-12-22 08:41 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.58 KB, patch)
2016-12-21 08:48 PST, Nael Ouedraogo
no flags Details | Formatted Diff | Diff
Patch (6.29 KB, patch)
2016-12-22 05:13 PST, Nael Ouedraogo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nael Ouedraogo 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.
Comment 1 Nael Ouedraogo 2016-12-21 08:48:04 PST
Created attachment 297590 [details]
Patch
Comment 2 Eric Carlson 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?
Comment 3 Nael Ouedraogo 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.
Comment 4 Eric Carlson 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?
Comment 5 Nael Ouedraogo 2016-12-22 05:13:43 PST
Created attachment 297652 [details]
Patch
Comment 6 Nael Ouedraogo 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?
Comment 7 Eric Carlson 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!
Comment 8 Nael Ouedraogo 2016-12-22 08:17:07 PST
Thanks for the review !
Comment 9 WebKit Commit Bot 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>
Comment 10 WebKit Commit Bot 2016-12-22 08:41:58 PST
All reviewed patches have been landed.  Closing bug.