From f3ec12e3c3baaf0ead4841501b2bdce44ef99a64 Mon Sep 17 00:00:00 2001 From: Zhou Weidong <502333560@qq.com> Date: Wed, 21 Oct 2026 20:00:15 +0800 Subject: [PATCH] =?UTF-8?q?fix(player):=20=E4=BF=AE=E5=A4=8D=E5=88=87?= =?UTF-8?q?=E6=8D=A2=E9=A2=91=E9=81=93=E6=97=B6=E5=A4=8D=E7=94=A8=E5=B7=B2?= =?UTF-8?q?=E9=87=8A=E6=94=BE=E7=9A=84=E8=A7=86=E9=A2=91=E6=8E=A7=E5=88=B6?= =?UTF-8?q?=E5=99=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/PROGRESS.md | 28 +++++ lib/core/data/services/playback_session.dart | 28 ++++- lib/pages/player/player_logic.dart | 8 +- test/playback/playback_session_test.dart | 55 ++++++++- test/playback/progress_throttle_test.dart | 1 + test/player/player_logic_test.dart | 115 +++++++++++++++++++ 6 files changed, 229 insertions(+), 6 deletions(-) diff --git a/docs/plans/PROGRESS.md b/docs/plans/PROGRESS.md index 9a8bd65..2122f93 100644 --- a/docs/plans/PROGRESS.md +++ b/docs/plans/PROGRESS.md @@ -175,3 +175,31 @@ Running Gradle task 'assembleDebug'... 26.4s 解读：静态分析零告警，563 项全量测试通过，debug APK 构建成功； `pubspec.lock` 已还原，无依赖变更，未安装或操作真机。 + +## 2026-10-21 K98.3 修复频道切换复用已释放视频控制器 + +- 已从最新 `origin/develop` 创建 `fix/k98.3-disposed-video-controller`。 +- `PlaybackSession.active` 与 `preparedPlayerAt` 不再暴露 closed/retiring slot； + `_retire` 先清空活动引用并通知，再等待界面帧边界后暂停和释放播放器。 +- `PlayerLogic._replaceSession` 切换 session 后等待 `endOfFrame`，并再次校验 + revision 后才关闭旧 session；其它 session 回收路径同样使用该帧边界。 +- 已补 retiring player 脱钩单测，以及真实 `PlayerVideoSurface` 经 feed 切换、 + 连续 pump 后不出现 disposed controller 异常的回归测试。 + +门禁原始输出： + +```text +$ dart analyze lib test +Analyzing lib, test... +No issues found! + +$ flutter test --no-pub +00:58 +565: All tests passed! + +$ flutter build apk --debug +Running Gradle task 'assembleDebug'... 35.7s +✓ Built build/app/outputs/flutter-apk/app-debug.apk +``` + +解读：静态分析零告警，565 项全量测试通过，debug APK 构建成功； +`pubspec.lock` 已还原，无依赖变更，未安装或操作真机。 diff --git a/lib/core/data/services/playback_session.dart b/lib/core/data/services/playback_session.dart index 6127fa9..d0c58ea 100644 --- a/lib/core/data/services/playback_session.dart +++ b/lib/core/data/services/playback_session.dart @@ -49,6 +49,7 @@ class PlaybackSession extends ChangeNotifier { this.prepare, this.activate, PlaybackCoordinator? coordinator, + this.waitForFrame, }) : _coordinator = coordinator ?? PlaybackCoordinator(); final List episodes; @@ -60,6 +61,7 @@ class PlaybackSession extends ChangeNotifier { /// Runs only when a slot becomes current (e.g. progress listeners). final PlayerPreparation? activate; + final Future Function()? waitForFrame; final PlaybackCoordinator _coordinator; final Set<_PlayerSlot> _live = {}; _PlayerSlot? _active; @@ -80,8 +82,16 @@ class PlaybackSession extends ChangeNotifier { int get currentIndex => _currentIndex; Episode get currentEpisode => episodes[_currentIndex]; - PlayerPort? get active => - _active?.index == _currentIndex ? _active?.player : null; + PlayerPort? get active { + final slot = _active; + if (_closed || + slot == null || + slot.retiring || + slot.index != _currentIndex) { + return null; + } + return slot.player; + } bool get loading => _loading; bool get userWantsPlay => _userWantsPlay; bool get canAutoAdvance => _mayPlay && !_loading && active != null; @@ -99,8 +109,15 @@ class PlaybackSession extends ChangeNotifier { /// Exposes only prepared slots so the adjacent page can mount its video texture /// during a swipe without starting audio or registering history listeners. PlayerPort? preparedPlayerAt(int index) { + if (_closed) return null; for (final slot in [_active, _next]) { - if (slot?.index == index && slot!.prepared) return slot.player; + if (slot == null || + slot.retiring || + slot.index != index || + !slot.prepared) { + continue; + } + return slot.player; } return null; } @@ -162,6 +179,11 @@ class PlaybackSession extends ChangeNotifier { Future _retire(_PlayerSlot? slot) async { if (slot == null || !_live.contains(slot)) return; slot.retiring = true; + final cleared = _active == slot || _next == slot; + if (_active == slot) _active = null; + if (_next == slot) _next = null; + if (cleared) _notify(); + await waitForFrame?.call(); await slot.ready; try { await slot.player.pause(); diff --git a/lib/pages/player/player_logic.dart b/lib/pages/player/player_logic.dart index debe265..288023f 100644 --- a/lib/pages/player/player_logic.dart +++ b/lib/pages/player/player_logic.dart @@ -40,13 +40,16 @@ class PlayerLogic extends ChangeNotifier { required this.pictureInPicture, this.wallet, this.downloads, - }); + Future Function()? waitForFrame, + }) : _waitForFrame = + waitForFrame ?? (() => WidgetsBinding.instance.endOfFrame); final WatchHistoryRepository history; final PlaybackRepository playback; final AndroidPictureInPicture pictureInPicture; final WalletController? wallet; final DownloadController? downloads; + final Future Function() _waitForFrame; late PageController pages; late PlaybackQueue queue; @@ -220,6 +223,7 @@ class PlayerLogic extends ChangeNotifier { prepare: preparePlayer, activate: activatePlayer, coordinator: coordinator, + waitForFrame: _waitForFrame, ); } @@ -307,6 +311,8 @@ class PlayerLogic extends ChangeNotifier { session = nextSession; nextSession.setRouteVisible(active && visibleIndices.isNotEmpty); _notifyAfterFrameIfNeeded(); + await _waitForFrame(); + if (revision != feedRevision || !identical(session, nextSession)) return; await previous.close(); if (revision != feedRevision || !identical(session, nextSession)) return; _jumpToFeedPage(targetIndex); diff --git a/test/playback/playback_session_test.dart b/test/playback/playback_session_test.dart index 336d3b6..d9905c2 100644 --- a/test/playback/playback_session_test.dart +++ b/test/playback/playback_session_test.dart @@ -123,6 +123,38 @@ void main() { await session.close(); }); + test( + 'should_detach_retiring_player_before_disposal_finishes', + () async { + final disposeStarted = Completer(); + final disposeGate = Completer(); + final factory = FakeFactory( + firstDisposeStarted: disposeStarted, + firstDisposeGate: disposeGate, + ); + final session = PlaybackSession( + episodes: episodes, + factory: factory.create, + canPlay: (episode) => episode.isFree, + ); + await session.select(0); + final retiring = factory.created.single; + + final switching = session.select(1); + await disposeStarted.future; + + expect(retiring.disposed, isFalse); + expect(session.active, isNull); + expect(session.preparedPlayerAt(0), isNull); + + disposeGate.complete(); + await switching; + expect(retiring.disposed, isTrue); + expect(session.active, isA()); + await session.close(); + }, + ); + test('should_keep_user_pause_when_reselecting_the_same_episode', () async { final factory = FakeFactory(); final session = PlaybackSession( @@ -164,8 +196,14 @@ void main() { } class FakeFactory { - FakeFactory({this.holdFirstInitialize = false}); + FakeFactory({ + this.holdFirstInitialize = false, + this.firstDisposeStarted, + this.firstDisposeGate, + }); final bool holdFirstInitialize; + final Completer? firstDisposeStarted; + final Completer? firstDisposeGate; final List created = []; final Completer firstCreated = Completer(); int live = 0; @@ -176,6 +214,8 @@ class FakeFactory { if (live > peakLive) peakLive = live; final player = FakePlayer( holdInitialize: holdFirstInitialize && created.isEmpty, + disposeStarted: created.isEmpty ? firstDisposeStarted : null, + disposeGate: created.isEmpty ? firstDisposeGate : null, onDispose: () => live--, ); created.add(player); @@ -185,9 +225,16 @@ class FakeFactory { } class FakePlayer implements PlayerPort { - FakePlayer({required this.holdInitialize, required this.onDispose}); + FakePlayer({ + required this.holdInitialize, + required this.onDispose, + this.disposeStarted, + this.disposeGate, + }); final bool holdInitialize; final void Function() onDispose; + final Completer? disposeStarted; + final Completer? disposeGate; final Completer _initializeGate = Completer(); bool initialized = false; bool disposed = false; @@ -211,6 +258,10 @@ class FakePlayer implements PlayerPort { @override Future dispose() async { if (disposed) return; + if (disposeStarted != null && !disposeStarted!.isCompleted) { + disposeStarted!.complete(); + } + await disposeGate?.future; disposed = true; onDispose(); } diff --git a/test/playback/progress_throttle_test.dart b/test/playback/progress_throttle_test.dart index 90b3697..7bfb62a 100644 --- a/test/playback/progress_throttle_test.dart +++ b/test/playback/progress_throttle_test.dart @@ -37,6 +37,7 @@ PlayerLogic _logic(FakeWatchHistory history, {Drama? drama}) { pictureInPicture: AndroidPictureInPicture( channel: const MethodChannel('test/progress/pip'), ), + waitForFrame: () async {}, ); if (drama != null) { logic.attach( diff --git a/test/player/player_logic_test.dart b/test/player/player_logic_test.dart index dea2fcf..c04bc59 100644 --- a/test/player/player_logic_test.dart +++ b/test/player/player_logic_test.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:video_player/video_player.dart'; import 'package:fruitshorts/core/data/models/drama.dart'; import 'package:fruitshorts/core/data/models/play_ticket.dart'; import 'package:fruitshorts/core/data/models/player_port.dart'; @@ -11,8 +12,10 @@ import 'package:fruitshorts/core/data/services/android_picture_in_picture.dart'; import 'package:fruitshorts/core/data/services/download_controller.dart'; import 'package:fruitshorts/core/data/services/download_storage.dart'; import 'package:fruitshorts/core/data/services/playback_session.dart'; +import 'package:fruitshorts/core/data/services/video_player_port.dart'; import 'package:fruitshorts/pages/player/player_logic.dart'; import 'package:fruitshorts/pages/player/player_view.dart'; +import 'package:fruitshorts/pages/player/widgets/player_video_surface.dart'; import '../support/fake_watch_history.dart'; import 'package:fruitshorts/core/data/repositories/playback_repository.dart'; @@ -45,10 +48,12 @@ AndroidPictureInPicture _pip() => PlayerLogic _logic( FakeWatchHistory history, { PlaybackRepository? playback, + Future Function()? waitForFrame, }) => PlayerLogic( history: history, playback: playback ?? FakePlaybackRepository(), pictureInPicture: _pip(), + waitForFrame: waitForFrame ?? () async {}, ); /// Drains the session's serialized work so a test never ends mid-disposal. @@ -681,6 +686,49 @@ void main() { logic.dispose(); }, ); + + testWidgets( + 'should_unmount_old_controller_before_disposing_it_when_feed_changes', + (tester) async { + final factory = _VideoRecordingFactory(); + final initial = [_drama('old')]; + final logic = _logic(FakeWatchHistory()); + logic.attach( + PlayerPage( + drama: initial.first, + initialIndex: 0, + homeMode: true, + feedDramas: initial, + ), + isMounted: () => true, + ); + await tester.runAsync(() async { + logic.start( + factory: factory.create, + coordinator: PlaybackCoordinator(), + ); + await _drain(logic); + }); + final oldPlayer = factory.created.single; + await tester.pumpWidget( + MaterialApp(home: _SessionVideoSurface(logic: logic)), + ); + await tester.pump(); + expect(find.byType(VideoPlayer), findsOneWidget); + + logic.updateFeed([_drama('new')], resetToFirst: true); + for (var frame = 0; frame < 5; frame++) { + await tester.pump(); + await tester.runAsync(pumpEventQueue); + } + + expect(tester.takeException(), isNull); + expect(oldPlayer.disposed, isTrue); + expect(factory.created.length, greaterThanOrEqualTo(2)); + logic.dispose(); + await tester.pump(); + }, + ); }); group('lifecycle', () { @@ -889,3 +937,70 @@ class _RecordingPlayer implements PlayerPort { onDispose(); } } + +class _SessionVideoSurface extends StatelessWidget { + const _SessionVideoSurface({required this.logic}); + + final PlayerLogic logic; + + @override + Widget build(BuildContext context) { + return ListenableBuilder( + listenable: logic, + builder: (context, _) => AnimatedBuilder( + animation: logic.session, + builder: (context, _) { + final port = logic.session.active; + final video = port is VideoPlayerPort ? port : null; + return Scaffold( + body: PlayerVideoSurface( + controller: video?.controller, + videoUrl: video?.controller.dataSource ?? '', + theater: false, + onTap: () {}, + ), + ); + }, + ), + ); + } +} + +class _VideoRecordingFactory { + final List<_VideoRecordingPlayer> created = []; + + PlayerPort create(Episode episode) { + final player = _VideoRecordingPlayer(episode); + created.add(player); + return player; + } +} + +class _VideoRecordingPlayer extends VideoPlayerPort { + _VideoRecordingPlayer(super.episode); + + bool disposed = false; + + @override + Future initialize() async { + controller.value = const VideoPlayerValue( + duration: Duration(minutes: 1), + size: Size(1280, 720), + position: Duration(seconds: 1), + isInitialized: true, + ); + } + + @override + Future play() async {} + + @override + Future pause() async {} + + @override + Future dispose() async { + if (disposed) return; + disposed = true; + await controller.dispose(); + } +}