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 [email protected] [material_ui] Add scrollPadding property to DropdownMenuFormField (flutter/packages#12735) 2026-10-02 [email protected] [material_ui] Migrate M3 Motion template to use new gen_defaults (flutter/packages#13058) 2026-10-02 [email protected] [cupertino_ui] Forward showDragHandle from showCupertinoSheet to CupertinoSheetRoute (flutter/packages#13011) 2026-10-02 [email protected] [cupertino_ui] Fix CupertinoMagnifier focal point with custom size (flutter/packages#13029) 2026-10-02 [email protected] [ci] Add material_ui and cupertino_ui to customer testing (flutter/packages#13114) 2026-10-02 [email protected] [material_ui] Add Material 3 Expressive migration skill (flutter/packages#12818) 2026-10-02 [email protected] [video_player] Ignore late position updates after disposal (flutter/packages#12971) 2026-10-02 [email protected] [material_ui] Remove unconditional dart:io import from about.dart (flutter/packages#12774) 2026-10-02 [email protected] [material_ui] Fix DropdownButtonFormField underline alignment at bottom (flutter/packages#12606) 2026-10-02 [email protected] Roll Flutter from e89fd0a to d03768e (23 revisions) (flutter/packages#13111) 2026-10-02 [email protected] [material_ui] Migrate `Switch` API doc snippets to {@example} (batch 8) (flutter/packages#13052) 2026-10-02 [email protected] [url_launcher] Fix supportsCloseForLaunchMode to query close support (flutter/packages#12926) 2026-10-02 [email protected] Redistribute iOS/macOS Suggested Reviewers (flutter/packages#13076) 2026-10-02 [email protected] [image_picker] Subsample large images when resizing on Android (flutter/packages#13038) 2026-10-02 [email protected] [cupertino_ui] Remove redundant null arguments in Completer.complete (flutter/packages#13008) 2026-10-02 [email protected] [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 [email protected] 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]///).