1250745 - Two GMP processes created per video element using unencrypted GMP decoding

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

If playback in <video> using GMPs for decoding is enabled, Firefox will spin up two plugin-container processes for every <video> element in the browser. This potentially could be very expensive, and it increases playback start latency.

This is because the GMPAudioDecoder and GMPVideoDecoders are using an empty string as their NodeId, causing us to assume we shouldn't allow sharing of GMP processes servicing that API actor. 

I did this initially because I was concerned that if we shared a GMP decode process instance across multiple tabs, the GMP could maybe track what videos a user was watching and report that to some third party.

I've realized however that in order to do this, the GMP would need to be able to store data about what the user is watching, and would need to be able to report that stored data.

GMP storage is isolated by NodeId. The only way a GMP can communicate to the outside world is by sending messages through the EME API. So provided we ensure we don't use EME with the same NodeId as we've using for GMP unencrypted decoding, then we can be sure that the GMP can't report whatever data it collates to a third party. We can also prevent GMPs from permanently storing data while doing unencrypted decoding.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: MozReview Request: Bug 1250745 - Share GMP processes when doing unencrypted decoding. r?jwwang

But make sure that there's no way a GMP can track and report what a user watches using unencrypted decoding.

Review commit: https://reviewboard.mozilla.org/r/36239/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36239/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8722786 [details]
MozReview Request: Bug 1250745 - Share GMP processes when doing unencrypted decoding. r?jwwang

https://reviewboard.mozilla.org/r/36239/#review32821

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Btw, is it possible for audio/video samples or metadata to contain site information so GMP{Audio,Video}Decoder knows which site the user is browsing when they decode audio/video samples or metadata?

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to JW Wang [:jwwang] from comment #3)
> Btw, is it possible for audio/video samples or metadata to contain site
> information so GMP{Audio,Video}Decoder knows which site the user is browsing
> when they decode audio/video samples or metadata?

I think it's unlikely, but who knows? Maybe one way to do it would be to hash the frame data and report that?

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Will need uplift.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e66d7c87d51a

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e66d7c87d51a

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8722786 [details]
MozReview Request: Bug 1250745 - Share GMP processes when doing unencrypted decoding. r?jwwang

Requesting uplift to Firefox 46, in support of Bug 1250766.

Approval Request Comment
[Feature/regressing bug #]: Playback of MP4/H.264/AAC video files in Firefox on Windows XP.
[User impact if declined]: Increased performance and memory overhead. Users on Windows XP who get MP4 playback via the Adobe GMP unencrypted decoding path added in Bug 1250766 will use more child processes than necessary to decode audio/video.
[Describe test coverage new/current, TreeHerder]: We have mochitests covering this feature.
[Risks and why]: Low; this directs us to follow an existing code path which is well tested.
[String/UUID change made/needed]: None.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking and marking affected for 46 since we are re-enabling GMP.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8722786 [details]
MozReview Request: Bug 1250745 - Share GMP processes when doing unencrypted decoding. r?jwwang

GMP fixes, please uplift to aurora.

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d7268ad99151