Bug 146748 - Make sure MediaSource engine isn't loaded for interpreting MediaStreams in getUserMedia()
Summary: Make sure MediaSource engine isn't loaded for interpreting MediaStreams in ge...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matthew Daiter
URL:
Keywords: HTML5, InRadar
Depends on:
Blocks: 146746
  Show dependency treegraph
 
Reported: 2015-07-08 16:19 PDT by Matthew Daiter
Modified: 2015-07-08 18:23 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.72 KB, patch)
2015-07-08 16:28 PDT, Matthew Daiter
no flags Details | Formatted Diff | Diff
Patch (4.05 KB, patch)
2015-07-08 16:39 PDT, Matthew Daiter
no flags Details | Formatted Diff | Diff
Patch (4.17 KB, patch)
2015-07-08 16:41 PDT, Matthew Daiter
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.