Bug 146748

Summary: Make sure MediaSource engine isn't loaded for interpreting MediaStreams in getUserMedia()
Product: WebKit Reporter: Matthew Daiter <mdaiter>
Component: WebCore Misc.Assignee: Matthew Daiter <mdaiter>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, eric.carlson, jonlee, mdaiter, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: HTML5, InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 146746    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Matthew Daiter 2015-07-08 16:19:38 PDT
MediaSource's AVFObjC engine was previously being loaded for interpreting MediaStreams. Making sure that it doesn't get loaded.
Comment 1 Radar WebKit Bug Importer 2015-07-08 16:21:34 PDT
<rdar://problem/21735416>
Comment 2 Matthew Daiter 2015-07-08 16:28:16 PDT
Created attachment 256418 [details]
Patch
Comment 3 Brent Fulgham 2015-07-08 16:32:57 PDT
Comment on attachment 256418 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256418&action=review

Looks good. Please address my concerns and we can get this landed!

> Source/WebCore/ChangeLog:8
> +

Add a statement like:

"Prevent the MediaSource engine from being used to process MediaStreams, since they are not compatible"

> Source/WebCore/ChangeLog:10
> +        Changed MediaStream to never be loaded

Move this explanation down to line 11, where you were saying "Ditto".

> Source/WebCore/ChangeLog:11
> +        (WebCore::MediaPlayerPrivateAVFoundationObjC::supportsType): Ditto

Remove Ditto from this line.

> Source/WebCore/ChangeLog:14
> +        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: Ditto

Remove 'Ditto' from this line.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:99
> +    virtual void load(MediaStreamPrivate*) override { setNetworkState(MediaPlayer::FormatError); }

No need for virtual here. "override" takes care of that. I think you should move this implementation inside of MediaPlayerPrivateMediaSourceAVFObjC.mm.
Comment 4 Matthew Daiter 2015-07-08 16:39:48 PDT
Created attachment 256423 [details]
Patch
Comment 5 Matthew Daiter 2015-07-08 16:41:31 PDT
Created attachment 256424 [details]
Patch
Comment 6 Brent Fulgham 2015-07-08 16:46:09 PDT
Comment on attachment 256424 [details]
Patch

r=me. We can cq+ this if the tests all pass.
Comment 7 WebKit Commit Bot 2015-07-08 18:23:33 PDT
Comment on attachment 256424 [details]
Patch

Clearing flags on attachment: 256424

Committed r186563: <http://trac.webkit.org/changeset/186563>
Comment 8 WebKit Commit Bot 2015-07-08 18:23:37 PDT
All reviewed patches have been landed.  Closing bug.