Bug 166328 – MediaStream: ASSERTION FAILED: m_ids.size() == m_handles.size() in MediaDeviceSandboxExtensions

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Nael Ouedraogo]

This assertion failure happens for port with SANDBOX_EXTENSION disabled. SandboxExtension::HandleArray is empty and HandleArray:size() always returns 0.

[Nael Ouedraogo]

Created attachment 297590 [details]
Patch

[Eric Carlson]

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]

(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]

(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]

Created attachment 297652 [details]
Patch

[Nael Ouedraogo]

> 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]

(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]

Thanks for the review !

[WebKit Commit Bot]

Comment on attachment 297652 [details]
Patch

Clearing flags on attachment: 297652

Committed r210097: <http://trac.webkit.org/changeset/210097>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.