1156560 - [EME] Persistent data is not shared between EME CDM updates

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Reported by Adobe.

Need to remove the version string from the node ID calculation from bug 1148071 and keep using the old CDM until all instances of it have died.

Comment 1

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: use-old-gmps.patch

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: use-old-gmps-aurora.patch

Comment 4

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

Comment on attachment 8595044 [details] [diff] [review]
use-old-gmps.patch

Review of attachment 8595044 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp/GMPServiceParent.cpp
@@ +583,5 @@
>          gmp->SetNodeId(aNodeId);
>          return gmp;
>        }
>  
> +      if (!gmpToClone || gmpToClone->State() < gmp->State() ||

I think this will cause us to prefer to re-use plugins that are already in-use, meaning we'll have the effect of prolonging the lifetime of out-of-date plugins. I think we should just remove the (gmpToClone->State() < gmp->State()) condition here.

::: dom/media/gmp/PGMPService.ipdl
@@ +18,5 @@
>    sync LoadGMP(nsCString nodeId, nsCString api, nsCString[] tags,
>                 ProcessId[] alreadyBridgedTo)
>      returns (ProcessId id, nsCString displayName, nsCString pluginId);
>    sync GetGMPNodeId(nsString origin, nsString topLevelOrigin,
> +                    bool inPrivateBrowsing)

You need to rev the uuid here.

::: dom/media/gmp/mozIGeckoMediaPluginService.idl
@@ +144,5 @@
>     */
>    [noscript]
>    void getNodeId(in AString origin,
>                   in AString topLevelOrigin,
>                   in bool inPrivateBrowsingMode,

You need to rev the uuid here.

Comment 5

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

Comment on attachment 8595047 [details] [diff] [review]
use-old-gmps-aurora.patch

Review of attachment 8595047 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp/GMPService.cpp
@@ +907,5 @@
>          gmp->SetNodeId(aNodeId);
>          return gmp;
>        }
>  
> +      if (!gmpToClone || gmpToClone->State() < gmp->State() ||

Same as the other patch, namely:

I think this will cause us to prefer to re-use plugins that are already in-use, meaning we'll have the effect of prolonging the lifetime of out-of-date plugins. I think we should just remove the (gmpToClone->State() < gmp->State()) condition here.

::: dom/media/gmp/mozIGeckoMediaPluginService.idl
@@ +109,5 @@
>     * Gets the NodeId for a (origin, urlbarOrigin, isInprivateBrowsing) tuple.
>     */
>    ACString getNodeId(in AString origin,
>                       in AString topLevelOrigin,
> +                     in bool inPrivateBrowsingMode);

You need to rev the uuid here.

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: use-old-gmps.patch

Carry r+.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbff0b332ce5

Comment 7

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: use-old-gmps-aurora.patch

Carry r+.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03b132a49631

Comment 8

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

Will need uplift.

Comment 9

[Pulsebot]

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

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 11

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

Comment on attachment 8595116 [details] [diff] [review]
use-old-gmps.patch

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this patch, when the EME Plugin updates, it will effectively orphan all storage from the old version. This will mean any DRM licenses and individualization that have been cached will not be present, and so startup of EME sessions will be slower. Also any unfinished playback sessions remembered by the site (say in IndexedDB) will not be present in the EME Plugin's storage, so the site will see inconsistent state and could be confused. (Adobe's QE picked this up)
[Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests
[Risks and why]: Low; most of this patch is removing code, and there's only a small logic change in there to make us keep using something that already works for a little longer.
[String/UUID change made/needed]: UUID change of mozIGeckoMediaPluginService. Previously gfritzsche was OK with changing this.

(We need this to 38... I'm not actually sure what's going on with us having uplifted 38 to Release early, so I requested Release approval... we *don't* need to uplift this to 37).

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8595116 [details] [diff] [review]
use-old-gmps.patch

Should be in 38 beta 7.
(no need to uplift to beta for now)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/43532ecc2aa3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/7c66212e4c09

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7c66212e4c09