fix: restore per-batch helper, then remove the need for helpers entirely - #679
Merged
Conversation
…ading My last change was wrong and made things worse. Capping the playlist at a single helper and returning [] for later all-watched batches stops the playlist loading entirely: keep-one.register BO7rZs_CYQs keep-one.reused active:["BO7rZs_CYQs"] continuation.detected x1 and nothing further, ever A 68-video playlist got two batches in and stalled showing only the helper. Confirmed deliberately: video 68 was set to 20% watched against an 80% hide threshold so it had to render, and it never appeared, because batch 5 was never fetched. The reasoning that led there was sound as far as it went — YouTube does refill an underfull viewport on its own, which is why disabling our schedulePlaylistAutoLoad cascade changed nothing. What I missed is that its refill loop also needs each continuation response to append at least one item. Hand it an empty batch and it treats the fetch as satisfied and stops asking, so the chain dies at the last non-empty response. The per-batch helper is not redundant: it is the single item that keeps the list starved enough to request more while still receiving something each time. Reverted to one helper per batch, with that failure mode recorded at the call site so it isn't attempted again. The blank slots stay, and remain unfixable from the DOM side while the virtual list's data model is unreachable on Tizen 5.0 (654577e). Keeps the cascade guard from #677, which a prior run verified is safe on its own: with autoload_skipped x3 YouTube still pulled the whole playlist unaided in 780ms.
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
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.
Two commits — merge both together.
1. Revert the helper cap. It stalled loading:
YouTube's refill loop needs each continuation to append at least one item. Hand it an empty batch and it treats the fetch as satisfied and stops asking. The per-batch helper is what keeps that loop alive. Your video-68 test (20% watched vs 80% threshold) proved it — it should have rendered and never did, because batch 5 was never fetched.
2. Serve fully-collected playlists from cache — no helper ever needed.
The initial page response is the one place the append-at-least-one 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 data model (654577e).Ordering is the catch: on a first visit the full list only exists after collection finishes. So we collect as now, cache by playlist hash, and reload once — the reloaded initial response is served complete from cache. Revisiting that playlist later in the session hits the cache with no reload at all.
Reloading per batch can't work, for the record: a reload re-fetches the first page, so it loops on batch 1 forever.
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.
Cost: one visible reload the first time you open a playlist. Only active with
enablePlaylistBatchCollecton.