Closed Bug 1160908 Opened 4 years ago Closed 4 years ago

[EME] Delete GMPRecords that are 0 bytes in size

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: cpearce, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Our GMPStorage API says "delete a record by overwriting it with 0 bytes". This is basically a copy of the Chromium API.

Our records are backed by a file on disk. We don't actually delete the record's file when overwriting it with 0 bytes. Adobe EME typically deletes two records every time it finishes playback of a movie. This means we'll eventually end up with lots and lots of 0-byte files lying around in the storage dir. We won't actually report those records as existing, but it will add latency to our record enumeration API, and is messy.

We shouldn't leave 0-byte files lying around. This isn't urgent, but we should fix it.
Assignee: nobody → gsquelart
When overwriting a file with 0 bytes, try and delete the file instead.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0990f8b636b6
Attachment #8601956 - Flags: review?(cpearce)
(Trying again, without temporary debugging code.)

When overwriting a file with 0 bytes, try and delete the file instead.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0990f8b636b6
Attachment #8601956 - Attachment is obsolete: true
Attachment #8601956 - Flags: review?(cpearce)
Attachment #8602056 - Flags: review?(cpearce)
Attachment #8602056 - Flags: review?(cpearce) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/450f694752e7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8602056 [details] [diff] [review]
1160908-delete-0-byte-records.patch

Approval Request Comment
[Feature/regressing bug #]: EME

[User impact if declined]: If uplift is declined, the Adobe EME plugin will leave behind several 0-byte-length files in the user profile dir on every playback. With this patch, we'll clean up the files. If we don't take this patch, we'll leave these 0-length files lying around forever, until the user reset their profile at least.

[Describe test coverage new/current, TreeHerder]: We have gtests for EME plugins storage functionality that this covers. This patch has also been in Aurora/Ngithly for a month, and has been tested there.

[Risks and why]: Pretty low; the fix is well contained to EME plugin storage, and that's tested pretty well.

[String/UUID change made/needed]: None.
Attachment #8602056 - Flags: approval-mozilla-beta?
Flags: needinfo?(cpearce)
OK, from discussion with Chris, sounds like this problem has been around a while but may become more of an issue during 39 or 40. We'll test this in beta to make sure it's still working as intended.
Comment on attachment 8602056 [details] [diff] [review]
1160908-delete-0-byte-records.patch

Approved for uplift to beta. This should be in beta 5 later in the week.
Attachment #8602056 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.