1160908 - [EME] Delete GMPRecords that are 0 bytes in size

Status: RESOLVED FIXED

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

Description

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

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.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1160908-delete-0-byte-records.patch

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

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

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1160908-delete-0-byte-records.patch

(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

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/450f694752e7

Comment 4

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

https://hg.mozilla.org/mozilla-central/rev/450f694752e7

Comment 5

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

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.

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

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 7

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d787fb3b86be