Repository navigation
[video_player] Ignore late position updates after disposal - #12971
auto-submit[bot] merged 5 commits into
Conversation
A platform seek can finish after the controller has been disposed, including the seek triggered by a playback-completed event. Guard the shared position update before writing the notifier, and cover both pending seek paths with regression tests.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the video_player package to version 2.14.1, introducing a fix that ignores late position updates after a VideoPlayerController is disposed. It includes new tests to verify this behavior using a simulated pending seek on a fake platform. The review feedback suggests resetting the shared seekCompleter to null using addTearDown in the new tests to prevent state leakage between tests.
Marking as a draft since the PR still includes agent instructions. Please don't open non-draft PRs that haven't been adequately reviewed (including the PR description). (If the checkbox for reading the AI contribution guidelines was checked for you by the agent that opened this PR, make sure that you have read and agree to them. Those instructions are for humans, not agents.) |
Thanks @stuartmorgan-g for your comment, I have updated the PR description. Marking the PR ready for review again. |
There was a problem hiding this comment.
Code Review
This pull request updates the video_player package to version 2.14.1, introducing a check in _updatePosition to return early if the controller is disposed, which prevents late position updates. New tests are added to verify that pending seek and completion seek results are ignored after disposal. The review feedback suggests resetting the shared fakeVideoPlayerPlatform.seekCompleter to null using addTearDown in the new tests to prevent potential side effects on subsequent tests.
|
@stuartmorgan-g please get this re-reviewed. |
|
Reviewers are assigned in a weekly triage process, as it says in the contributing docs that you indicated that you have read. Please don't ping people to attempt to bypass the normal process. |
|
autosubmit label was removed for flutter/packages/12971, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…er#193840) flutter/packages@5620e65...951f2f7 2026-10-05 Hamidrezash1384@gmail.com [material_ui] Add scrollPadding property to DropdownMenuFormField (flutter/packages#12735) 2026-10-02 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 Motion template to use new gen_defaults (flutter/packages#13058) 2026-10-02 44747303+theprantadutta@users.noreply.github.com [cupertino_ui] Forward showDragHandle from showCupertinoSheet to CupertinoSheetRoute (flutter/packages#13011) 2026-10-02 105214765+HibaChamkhi@users.noreply.github.com [cupertino_ui] Fix CupertinoMagnifier focal point with custom size (flutter/packages#13029) 2026-10-02 katelovett@google.com [ci] Add material_ui and cupertino_ui to customer testing (flutter/packages#13114) 2026-10-02 36861262+QuncCccccc@users.noreply.github.com [material_ui] Add Material 3 Expressive migration skill (flutter/packages#12818) 2026-10-02 sethineelansh@gmail.com [video_player] Ignore late position updates after disposal (flutter/packages#12971) 2026-10-02 46639812+Gyeony95@users.noreply.github.com [material_ui] Remove unconditional dart:io import from about.dart (flutter/packages#12774) 2026-10-02 50643541+Mairramer@users.noreply.github.com [material_ui] Fix DropdownButtonFormField underline alignment at bottom (flutter/packages#12606) 2026-10-02 engine-flutter-autoroll@skia.org Roll Flutter from e89fd0a to d03768e (23 revisions) (flutter/packages#13111) 2026-10-02 dkwingsmt@users.noreply.github.com [material_ui] Migrate `Switch` API doc snippets to {@example} (batch 8) (flutter/packages#13052) 2026-10-02 45616602+NikhilKukreja26@users.noreply.github.com [url_launcher] Fix supportsCloseForLaunchMode to query close support (flutter/packages#12926) 2026-10-02 15619084+vashworth@users.noreply.github.com Redistribute iOS/macOS Suggested Reviewers (flutter/packages#13076) 2026-10-02 anilcan.cakir@gmail.com [image_picker] Subsample large images when resizing on Android (flutter/packages#13038) 2026-10-02 kevmoo@users.noreply.github.com [cupertino_ui] Remove redundant null arguments in Completer.complete (flutter/packages#13008) 2026-10-02 269567208+reidbaker-agent@users.noreply.github.com [repo] Update AGENTS.md with dependency allowance and versioning guidance (flutter/packages#12579) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
A platform seek can finish after
VideoPlayerController.dispose(), causing_updatePositionto write to the disposed notifier and throw. This also happens in the controller's ownVideoEventType.completedpause/seek chain, without the caller invoking a method after disposal.Checks
_isDisposedat the shared position-update boundary. Adds regressions for an explicit pending seek and the pending seek from a playback-completed event. Both tests fail on the unmodified controller and pass with the guard. The patch updates the package to 2.14.1 and incorporates the existingNEXTchangelog entry as required by the release instructions.Fixes flutter/flutter#193147
Validation with Flutter 3.44.9 / Dart 3.12.2:
dart analyze --fatal-infosfor the package and example: no issues.publish-check: passed after committing the four-file patch.git diff --check: passed.Repository-wide
validatecould not complete in the sparse checkout because the root README references packages outside the checkout (animations). Native/integration tests were not run; this change is limited to the Dart controller.No public API or dartdoc changes are needed.
Pre-Review Checklist
[shared_preferences]///).