1264037 - Add testing for CRX Extraction

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Robin Steuber (they/them) [:bytesized]]

CRX extractor (See Bug 1260836) needs testing.

Comment 1

[Robin Steuber (they/them) [:bytesized]]

Attachment: MozReview Request: Bug 1264037 - Add testing for CRX reading

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

Comment 2

[Robin Steuber (they/them) [:bytesized]]

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

Comment 3

[Stephen A Pohl [:spohl]]

Comment on attachment 8740645 [details]
MozReview Request: Bug 1264037 - Add testing for CRX reading

https://reviewboard.mozilla.org/r/45845/#review42507

r+, assuming try comes back green and the minor issues below are addressed. Thanks!

::: modules/libjar/test/unit/test_crx.js:9
(Diff revision 1)
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +function wrapInputStream(input)
> +{
> +  var nsIScriptableInputStream = Components.interfaces.nsIScriptableInputStream;

new code should use |let| instead of |var|, here and everywhere else in this file.

::: modules/libjar/test/unit/xpcshell.ini:20
(Diff revision 1)
>    data/test_corrupt.zip
>    data/test_corrupt2.zip
>    data/test_corrupt3.zip
>    data/test_umlaute.zip
>    data/uncompressed.zip
> +  data/test_crx_dummy.crx

This needs to move to the proper alphabetical order

::: modules/libjar/test/unit/xpcshell.ini:45
(Diff revision 1)
>  [test_dirjar_bug525755.js]
>  [test_jarinput_stream_zipreader_reference.js]
>  [test_not_found.js]
>  [test_uncompressed.js]
>  [test_umlaute.js]
> -
> +[test_crx.js]

alphabetical order

Comment 4

[Robin Steuber (they/them) [:bytesized]]

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

Comment 5

[Robin Steuber (they/them) [:bytesized]]

Comment on attachment 8740645 [details]
MozReview Request: Bug 1264037 - Add testing for CRX reading

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45845/diff/1-2/

Comment 6

[Stephen A Pohl [:spohl]]

Comment on attachment 8740645 [details]
MozReview Request: Bug 1264037 - Add testing for CRX reading

https://reviewboard.mozilla.org/r/45845/#review43023

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b3d8b016792

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3b3d8b016792

Comment 9

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

Kirk/spohl: Should we include this in the widevine uplift?

Comment 10

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

spohl says yes, we should uplift on IRC.

Comment 11

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

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

Comment 12

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/26b73eb3a491

Comment 13

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

Comment on attachment 8740645 [details]
MozReview Request: Bug 1264037 - Add testing for CRX reading

Requesting uplift for Firefox 47.

Approval Request Comment
[Feature/regressing bug #]: Widevine EME support
[User impact if declined]: This adds a test for some of the earlier Widevine CDM install code.
[Describe test coverage new/current, TreeHerder]: This *is* a test. ;)
[Risks and why]: Low.
[String/UUID change made/needed]: None.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8740645 [details]
MozReview Request: Bug 1264037 - Add testing for CRX reading

Widevine related uplifts were pre-approved, automated test, yay!!!