1140797 - [EME] Make gmp-clearkey buildable outside of mozilla-central

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

I've overwritten my non-spec-compliant ClearKey GMP with the one we have in mozilla-central. I required a few small changes in order to make the code build outside of m-c.

Comment 1

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

Attachment: Patch: Basic compile fixes

Basic include/char encoding fixes to make building gmp-clearkey outside of m-c possible.

Comment 2

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

Attachment: Patch: prevent assert in base64 decode of keyIds

The bsae64decode of keyIds and keys in the gmp still isn't quite right. When build outside of m-c, DecodeBase64() asserts when we do the "*(++out) = aEncoded[i] << (shift + 2);" line on the last 6bit-octet, as the iterator is pointing to aOutDecoded[aEncoded.length() * 6 / 8], whereas the size is one less than that. Note that we call:
reserve aOutDecoded.reserve(aEncoded.length() * 6 / 8 + 1)

so we won't actually be writing into unallocated memory, so what we're doing now isn't fatal. But when we try to build and run this code out side of m-c, the STL asserts that we're writing outside of the bounds of the size of the vector (which we are).

We only use DecodeBase64() for decoding 128 bit keyIds and keys, so 128/6=21.3~, so this last byte we're writing is padding. We don't need it. So we can just check for this last bit and not write it, since it's padding.

Comment 3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/58d2dbe695c4
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd14abe25846

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/58d2dbe695c4
https://hg.mozilla.org/mozilla-central/rev/cd14abe25846

Comment 5

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

Will need this so that we can take Bug 1141386 cleanly.

Comment 6

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

Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this, it's harder to uplift Bug 1141386, which is required for EME to work on some EME sites
[Describe test coverage new/current, TreeHerder]: The changes here only affect the build. The build works. ;)
[Risks and why]: Low, the changes here only affect the build.
[String/UUID change made/needed]: None

Comment 7

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

Comment on attachment 8574356 [details] [diff] [review]
Patch: prevent assert in base64 decode of keyIds

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this, it's harder to uplift Bug 1141386, which is required for EME to work on some EME sites
[Describe test coverage new/current, TreeHerder]: Local testing, and there's a test in Bug 1141386 to cover the code changes made here, which I shall also request approval for uplift on.
[Risks and why]: Low, isolated to EME.
[String/UUID change made/needed]: None

Comment 8

[[e-mail] (:bhearsum)]

This needs to get backported to mozilla-beta (Firefox 37) pretty soon (the final beta goes to build next Thursday), otherwise we won't be able to provide the XULRunner SDK to add-on developers.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5abad9f5fc15
https://hg.mozilla.org/releases/mozilla-aurora/rev/95d7fea08a00

Comment 10

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

Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Nil. This code is not enabled on Firefox 38. Some developers will not be able to build the xulrunner SDK without these patches however.
[Describe test coverage new/current, TreeHerder]: Mochitests
[Risks and why]: Low
[String/UUID change made/needed]: None

Comment 11

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

Comment on attachment 8574356 [details] [diff] [review]
Patch: prevent assert in base64 decode of keyIds

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Nil. This code is not enabled on Firefox 38. Some developers will not be able to build the xulrunner SDK without these patches however.
[Describe test coverage new/current, TreeHerder]: Mochitests
[Risks and why]: Low
[String/UUID change made/needed]: None

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

As stated in the approval request, this code will not be executed in the release but is required for the xulrunner SDK build. Beta+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a49b40d229df
https://hg.mozilla.org/releases/mozilla-beta/rev/29333933d6d6