Skip to content

[video_player] Ignore late position updates after disposal - #12971

Merged
auto-submit[bot] merged 5 commits into
flutter:mainfrom
Neelansh-ns:codex/video-player-dispose-seek-upstream
Oct 2, 2026
Merged

auto-submit[bot] merged 5 commits into
flutter:mainfrom
Neelansh-ns:codex/video-player-dispose-seek-upstream

Conversation

@Neelansh-ns

@Neelansh-ns Neelansh-ns commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

A platform seek can finish after VideoPlayerController.dispose(), causing _updatePosition to write to the disposed notifier and throw. This also happens in the controller's own VideoEventType.completed pause/seek chain, without the caller invoking a method after disposal.

Checks _isDisposed at 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 existing NEXT changelog entry as required by the release instructions.

Fixes flutter/flutter#193147

Validation with Flutter 3.44.9 / Dart 3.12.2:

  • Full Dart package suite: 118 passed, one existing skipped test.
  • dart analyze --fatal-infos for the package and example: no issues.
  • Repository Dart formatter run; unrelated pre-existing caption-test reformatting excluded from the diff.
  • Repository publish-check: passed after committing the four-file patch.
  • git diff --check: passed.

Repository-wide validate could 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

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

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.
@google-cla

google-cla Bot commented Sep 22, 2026

Copy link
Copy Markdown

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.

@Neelansh-ns
Neelansh-ns marked this pull request as ready for review September 22, 2026 12:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/video_player/video_player/test/video_player_test.dart
Comment thread packages/video_player/video_player/test/video_player_test.dart
@stuartmorgan-g

stuartmorgan-g commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

This draft is awaiting the contributor's review of the AI-assisted changes and completion of the CLA. Supersedes the closed #12968 with a branch based on current main.

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.)

@stuartmorgan-g
stuartmorgan-g marked this pull request as draft September 22, 2026 15:42
@Neelansh-ns

Copy link
Copy Markdown
Contributor Author

This draft is awaiting the contributor's review of the AI-assisted changes and completion of the CLA. Supersedes the closed #12968 with a branch based on current main.

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.

@Neelansh-ns
Neelansh-ns marked this pull request as ready for review September 23, 2026 11:41

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/video_player/video_player/test/video_player_test.dart
Comment thread packages/video_player/video_player/test/video_player_test.dart
@Neelansh-ns

Copy link
Copy Markdown
Contributor Author

@stuartmorgan-g please get this re-reviewed.

@stuartmorgan-g

Copy link
Copy Markdown
Collaborator

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.

@Piinks
Piinks requested a review from tarrinneal September 29, 2026 21:22

@Piinks Piinks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 2, 2026
@tarrinneal tarrinneal added the CICD Run CI/CD label Oct 2, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 2, 2026
@auto-submit

auto-submit Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

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.

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 2, 2026
@auto-submit
auto-submit Bot merged commit e18a541 into flutter:main Oct 2, 2026
14 checks passed
ZhuJHua pushed a commit to ZhuJHua/flutter that referenced this pull request Oct 6, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: video_player

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[video_player] Pending seek updates controller after disposal

4 participants