fix: stop the playlist cache serving stale watched state on re-entry - #686
Merged
Conversation
Reported: after watching videos and re-entering a playlist, the watched ones came back looking unwatched. That is a regression from the full-list cache, and the diagnosis offered with the report was right — re-entry was being served from cache. getWatchPercent() reads watched state primarily from data embedded in the renderer itself (thumbnailOverlayResumePlaybackRenderer, played/status overlays, watched badges), falling back to the live _ttVideoProgressCache only when none of those are present. The cache holds raw renderers captured at collection time, so their overlays are frozen at that moment. Injecting them on a later visit replays whatever progress they had then, and overwrites the fresh response that did carry the updated state. So the cache is now strictly a hand-off from the pre-reload pass to the reloaded page, consumed on injection. Every visit runs its own collect+reload against fresh data. That costs a cycle per visit (~1.7s) instead of a 0.38s cache hit, which is the right trade for showing correct watched state. Two guards keep that from looping, since consuming the cache mid-visit would otherwise let the reloaded page collect and reload again: - __ttServedFromCache marks the page that was served from cache, and the scheduler stands down for it — it already holds the full list and has no continuation token. - The reload guard is cleared in _clearState (real navigation) rather than at injection, so it persists across SOFT_RELOAD_PAGE, which does not navigate, while still letting the next genuine visit run its own cycle. Not addressed, as agreed: stopping playback returns to the playlist without reprocessing it, so videos watched in that session stay visible until the page is re-entered. With this fix that re-entry now filters them correctly, which was the practical problem.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Valid finding — remote property injection on the cache write. Both the full-playlist cache and the one-shot reload guard were plain objects keyed by playlistKeyFromHash(), i.e. window.location.hash. That is attacker-influenceable through a crafted URL, so cache[key] = ... with a hash of #__proto__ would write through to Object.prototype and pollute every object in the page. Converted both to the collection types that have no prototype chain to reach: - __ttPlaylistFullCache: object -> Map (get/set/has/delete, plus size and iteration for the eviction pass, which previously used Object.keys and a reduce over cache[a].ts). - __ttFullReloadDone: object -> Set, since it only ever stored true as a membership marker. Both are guarded with an instanceof check on read, so a stale plain object left on window by an earlier build is replaced rather than misused. Set is already used in four places in adblock.js, so these types are known to work on the target devices. No behaviour change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Your diagnosis was right — re-entry was being served from cache.
getWatchPercent()reads watched state primarily from data embedded in the renderer itself (thumbnailOverlayResumePlaybackRenderer, played/status overlays, watched badges), and only falls back to the live_ttVideoProgressCachewhen none are present. The cache holds raw renderers captured at collection time, so their overlays are frozen at that moment. Injecting them on a later visit replays the old progress and discards the fresh response that did carry the update.So the cache is now strictly a hand-off from the pre-reload pass to the reloaded page, and is consumed on injection. Every visit runs its own collect+reload against fresh data — ~1.7s per visit instead of a 0.38s cache hit, which is the right trade for correct watched state.
Two guards prevent a reload loop, since consuming the cache mid-visit would otherwise let the reloaded page collect and reload again:
__ttServedFromCachemarks the cache-served page and the scheduler stands down for it (it already holds the full list and has no continuation token).SOFT_RELOAD_PAGE(which doesn't navigate) while still letting the next genuine visit run.Point 1 not addressed, as you said was fine: stopping playback returns to the playlist without reprocessing, so videos watched in that session stay visible until you re-enter. With this fix, that re-entry now filters them correctly — which was the practical problem.