2064564 - Wallpapers: Orphaned wallpaper cleanup

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[dgrant]

Uploading a custom wallpaper leaves the previous file in <profile>/wallpaper/ forever, un-referenced and unreachable and nothing in Firefox ever removes it. This deletes the replaced file when a new one takes its place and sweeps leftovers on startup so profiles that already accumulated them get cleaned up. It also writes the identifier pref only after the file lands and clears it when the file it names is gone.

Comment 1

[Hackbot]

Triage notes (no STR in the bug, so this reconstructs the problem from the code — please correct if HNT-2982 means something else).

Root cause. An "orphaned wallpaper" is a file in <profileDir>/wallpaper/ whose name is not the current value of browser.newtabpage.activity-stream.newtabWallpapers.customWallpaper.uuid. That pref is the only pointer to a stored wallpaper — the image is read back via moz-newtab-wallpaper://<uuid>, resolved in netwerk/protocol/res/MozNewTabWallpaperProtocolHandler.cpp — but wallpaperUpload() mints a fresh UUID and clobbers the pref on every upload without touching the previous file: browser/extensions/newtab/lib/Wallpapers/WallpaperFeed.sys.mjs#296-305.

The only deletion in the tree is removeCustomWallpaper() at WallpaperFeed.sys.mjs#327-341, which returns early when the pref is empty and removes only the currently-referenced UUID — so it can never reach an already-orphaned file. Nothing enumerates the directory (no IOUtils.getChildren anywhere under browser/extensions/newtab), and at.INIT does no reconciliation (#358).

Aggravating factors:

• Its sole dispatcher is handleReset() (WallpaperCategories.jsx#268), wired to a reset button rendered only when !novaEnabled (#594). Once Nova ships there is no user-reachable path that deletes a wallpaper file at all.
• PictureOfTheDayFeed.setWallpaper() (#238) dispatches WALLPAPER_UPLOAD with a freshly fetched high-res image, so orphans can now accumulate daily with no manual upload.
• The pref is set before the write (#297 vs #305); a throw leaves the pref pointing at a nonexistent file with the old one already orphaned, and an interrupted write can leave a stale <uuid>.tmp.
• Size is effectively uncapped: customWallpaper.fileSize.enabled defaults false (browser/app/profile/firefox.js#2003-2004) and the check is content-side only.

Proposed fix, all in WallpaperFeed.sys.mjs:

1. In wallpaperUpload(), read the existing UUID before generating the new one; write the new file first, set the pref only on success, then IOUtils.remove the previous file. That fixes the common case and the set-pref-before-write ordering together.
2. Add a reconciliation sweep — IOUtils.getChildren(<profileDir>/wallpaper), delete every entry whose name !== the current customWallpaper.uuid (and any *.tmp) — and run it once from the at.INIT path, off the critical path (idle dispatch). This is what actually clears profiles that have already accumulated orphans; a fix to (1) alone leaves existing junk forever.
3. Separately worth deciding: under Nova nothing dispatches WALLPAPER_REMOVE_UPLOAD, so the toggle should either fire it or rely on the sweep.

Tests. test_WallpaperFeed.js covers the upload round-trip (test_Wallpaper_Upload) but has no coverage of removeCustomWallpaper, WALLPAPER_REMOVE_UPLOAD, or re-upload/replacement — that's the natural home for regression tests here; also test_PictureOfTheDayFeed.js.

Severity: S3 suggested. No crash or loss of user data, and the growth is bounded by how often the user changes wallpaper — but there is no in-product way to see or remove the files, so the only workaround is deleting them by hand in a file manager.

Bug 2041892 looks like the user-facing report of exactly this ("previously added custom images are never removed") — worth linking as see-also or duping. Note the directory is singular wallpaper, not wallpapers as that report says.

(Analysis from reading the code only — not built or run.)

---

If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-wrong-file, ai-triage-wrong-cause, ai-triage-hallucination, ai-triage-out-of-scope.

Comment 2

[dgrant]

Attachment: Bug 2064564 - Remove orphaned custom wallpaper files. r?#home-newtab-reviewers

Uploading a custom wallpaper leaves the previous file in <profile>/wallpaper/
forever, unreferenced and unreachable, and nothing in Firefox ever removes it.
This deletes the replaced file when a new one takes its place, and sweeps
leftovers on startup so profiles that already accumulated them get cleaned up. It
also writes the identifier pref only after the file lands, and clears it when the
file it names is gone.

Two known cases stay open on purpose. Two uploads racing can still orphan a file,
since both read the same previous identifier; the startup sweep clears it on the
next launch, and serializing the writes belongs with the wallpaper library work in
HNT-2835. And with Nova on there is still no way for someone to delete a stored
wallpaper, which needs a real per-wallpaper delete rather than making the
wallpapers toggle destroy files.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1009fe49afe4
https://hg.mozilla.org/integration/autoland/rev/a25bb0567c76
Remove orphaned custom wallpaper files. r=mconley

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/a25bb0567c76