feat: serve fully-collected playlists from cache so no helper is ever needed - #680
Merged
Conversation
… needed Removes the helper tiles entirely for playlists the background collector manages to fetch in full, which is the only way left to be rid of the stranded blank slots. Why this shape. A helper only exists because a continuation response must append at least one item or YouTube stops asking for more (proved by the previous PR, which returned [] and stalled a 68-video playlist after two batches). The initial page response is the one place that constraint does not apply: give it the WHOLE playlist and null its continuations, and there is nothing left to load, so no keep-one branch runs and no helper is ever created. Nothing then gets stranded in the virtual list's data model, which is what made every DOM-side attempt futile (654577e). The ordering problem is that on a first visit the full list only exists after collection finishes, long after the initial response rendered. So: collect as now, cache the result keyed by playlist hash, and reload the page once. The reloaded initial response is served from cache, complete, with continuations nulled. Revisiting that playlist later in the session hits the cache immediately and skips both the collection and the reload. Reloading per batch — the obvious variant — cannot work: a reload re-fetches the FIRST page, so it would loop on batch 1 forever. Details: - _collectAll only ever returns the CONTINUATION batches (53 of 68 on the measured playlist), so adblock.js hands the raw initial batch over via noteInitialPlaylistContents to make the cached list complete from item 1. - Only a COMPLETE collection is cached (no continuation token left, not aborted). Caching a partial list would render a truncated playlist with no token to load the rest. - Reload fires at most once per playlist key, and not at all if the user navigated away while collecting. - Cache is bounded to 3 playlists, evicting oldest — these are full renderer objects and this runs on a TV. - resolveCommand is reached through a window global rather than an ES import: playlistBatchCollect.js must evaluate before adblock.js to capture JSON.parse ahead of its patch, and importing resolveCommand (which pulls in settings/UI) would risk reordering that. Cost is one visible reload the first time a playlist is opened. Only active with enablePlaylistBatchCollect on.
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
1 participant
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.
This is the commit that was missing from #679. That PR was merged while it still contained only the revert, so v1.15.570 has the revert but not this — which is why the log showed
prefetch_ready {hasMore:false, aborted:false}firing with nofull_cache.*entries at all. My mistake in how I pushed it.Removes helper tiles entirely for playlists the collector fetches in full.
The initial page response is the one place the "a continuation must append at least one item" rule doesn't apply. Give it the whole playlist with
continuationsnulled and there's nothing left to load — so the keep-one branch never runs, no helper is created, and nothing gets stranded in the virtual list's data model (654577e).Ordering is handled by collecting as now, caching by playlist hash, and reloading once; the reloaded initial response is served complete from cache. Revisiting the playlist later in the session hits the cache with no reload.
Guards: only a complete collection is cached (a partial one would render truncated with no token to load the rest); reload fires at most once per playlist and not if you navigated away; cache bounded to 3 playlists.
Expected log on the next test:
full_cache.stored→full_cache.reloading→full_cache.injected, and nokeep-one.register.