1104970 - [EME] GMPStorage can't handle long record names

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

Bug 1100499 changed our GMPStorage API to base64encode the record name and uses that as the file name. If a long record name is put through this, it can hit the max path size of a file on Windows, resulting in I/O failure.

Comment 1

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

Attachment: Patch: Detect GMPStorage failure in gtests

We should really detect failures in GMPStorage reads/writes in our gtests, instead of assuming that reads/writes always succeed...

Comment 2

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

Attachment: Patch: Store GMP record name in record file on disk

* Revert to using Mozilla::HashString(recordName) as the filename for records.
* Write the length of the recordName and then the recordName into the start of the record's file, before the actual record data.
* Treat files which aren't in this format as empty records; so they can be overwritten.

Comment 3

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

Attachment: Patch: Store GMP record name in record file on disk

With WAE compile fixes on for GCC, and test tweaked to pass...

Comment 4

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d5124979a26c
https://tbpl.mozilla.org/?tree=Try&rev=d5124979a26c

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8528787 [details] [diff] [review]
Patch: Store GMP record name in record file on disk

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

Looks good... spent a while looking over the size/offset stuff; looks good

Comment 6

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

https://tbpl.mozilla.org/?tree=Try&rev=2c9a925932dd

Comment 7

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

Attachment: Patch: Detect GMPStorage failure in gtests

Commit message updated.

Comment 8

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

Attachment: Patch: Store GMP record name in record file on disk

Commit message updated to reflect r+

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5fc3ea149549
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d18f4965871

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5fc3ea149549
https://hg.mozilla.org/mozilla-central/rev/8d18f4965871

Comment 11

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

Mass update firefox-status to track EME uplift.