fix(player): 修复切换频道时复用已释放的视频控制器
This commit is contained in:
1 parent
45a53180d9
commit
f3ec12e3c3
6 files changed
+229
-6
No files matched your search
@@ -175,3 +175,31 @@ Running Gradle task 'assembleDebug'... 26.4s
|
|||||||
|
|
||||||
解读:静态分析零告警,563 项全量测试通过,debug APK 构建成功;
|
解读:静态分析零告警,563 项全量测试通过,debug APK 构建成功;
|
||||||
`pubspec.lock` 已还原,无依赖变更,未安装或操作真机。
|
`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` 已还原,无依赖变更,未安装或操作真机。
|
||||||
@@ -49,6 +49,7 @@ class PlaybackSession extends ChangeNotifier {
|
|||||||
this.prepare,
|
this.prepare,
|
||||||
this.activate,
|
this.activate,
|
||||||
PlaybackCoordinator? coordinator,
|
PlaybackCoordinator? coordinator,
|
||||||
|
this.waitForFrame,
|
||||||
}) : _coordinator = coordinator ?? PlaybackCoordinator();
|
}) : _coordinator = coordinator ?? PlaybackCoordinator();
|
||||||
|
|
||||||
final List<Episode> episodes;
|
final List<Episode> episodes;
|
||||||
@@ -60,6 +61,7 @@ class PlaybackSession extends ChangeNotifier {
|
|||||||
|
|
||||||
/// Runs only when a slot becomes current (e.g. progress listeners).
|
/// Runs only when a slot becomes current (e.g. progress listeners).
|
||||||
final PlayerPreparation? activate;
|
final PlayerPreparation? activate;
|
||||||
|
final Future<void> Function()? waitForFrame;
|
||||||
final PlaybackCoordinator _coordinator;
|
final PlaybackCoordinator _coordinator;
|
||||||
final Set<_PlayerSlot> _live = {};
|
final Set<_PlayerSlot> _live = {};
|
||||||
_PlayerSlot? _active;
|
_PlayerSlot? _active;
|
||||||
@@ -80,8 +82,16 @@ class PlaybackSession extends ChangeNotifier {
|
|||||||
|
|
||||||
int get currentIndex => _currentIndex;
|
int get currentIndex => _currentIndex;
|
||||||
Episode get currentEpisode => episodes[_currentIndex];
|
Episode get currentEpisode => episodes[_currentIndex];
|
||||||
PlayerPort? get active =>
|
PlayerPort? get active {
|
||||||
_active?.index == _currentIndex ? _active?.player : null;
|
final slot = _active;
|
||||||
|
if (_closed ||
|
||||||
|
slot == null ||
|
||||||
|
slot.retiring ||
|
||||||
|
slot.index != _currentIndex) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return slot.player;
|
||||||
|
}
|
||||||
bool get loading => _loading;
|
bool get loading => _loading;
|
||||||
bool get userWantsPlay => _userWantsPlay;
|
bool get userWantsPlay => _userWantsPlay;
|
||||||
bool get canAutoAdvance => _mayPlay && !_loading && active != null;
|
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
|
/// Exposes only prepared slots so the adjacent page can mount its video texture
|
||||||
/// during a swipe without starting audio or registering history listeners.
|
/// during a swipe without starting audio or registering history listeners.
|
||||||
PlayerPort? preparedPlayerAt(int index) {
|
PlayerPort? preparedPlayerAt(int index) {
|
||||||
|
if (_closed) return null;
|
||||||
for (final slot in [_active, _next]) {
|
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;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -162,6 +179,11 @@ class PlaybackSession extends ChangeNotifier {
|
|||||||
Future<void> _retire(_PlayerSlot? slot) async {
|
Future<void> _retire(_PlayerSlot? slot) async {
|
||||||
if (slot == null || !_live.contains(slot)) return;
|
if (slot == null || !_live.contains(slot)) return;
|
||||||
slot.retiring = true;
|
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;
|
await slot.ready;
|
||||||
try {
|
try {
|
||||||
await slot.player.pause();
|
await slot.player.pause();
|
||||||
|
|||||||
@@ -40,13 +40,16 @@ class PlayerLogic extends ChangeNotifier {
|
|||||||
required this.pictureInPicture,
|
required this.pictureInPicture,
|
||||||
this.wallet,
|
this.wallet,
|
||||||
this.downloads,
|
this.downloads,
|
||||||
});
|
Future<void> Function()? waitForFrame,
|
||||||
|
}) : _waitForFrame =
|
||||||
|
waitForFrame ?? (() => WidgetsBinding.instance.endOfFrame);
|
||||||
|
|
||||||
final WatchHistoryRepository history;
|
final WatchHistoryRepository history;
|
||||||
final PlaybackRepository playback;
|
final PlaybackRepository playback;
|
||||||
final AndroidPictureInPicture pictureInPicture;
|
final AndroidPictureInPicture pictureInPicture;
|
||||||
final WalletController? wallet;
|
final WalletController? wallet;
|
||||||
final DownloadController? downloads;
|
final DownloadController? downloads;
|
||||||
|
final Future<void> Function() _waitForFrame;
|
||||||
|
|
||||||
late PageController pages;
|
late PageController pages;
|
||||||
late PlaybackQueue queue;
|
late PlaybackQueue queue;
|
||||||
@@ -220,6 +223,7 @@ class PlayerLogic extends ChangeNotifier {
|
|||||||
prepare: preparePlayer,
|
prepare: preparePlayer,
|
||||||
activate: activatePlayer,
|
activate: activatePlayer,
|
||||||
coordinator: coordinator,
|
coordinator: coordinator,
|
||||||
|
waitForFrame: _waitForFrame,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -307,6 +311,8 @@ class PlayerLogic extends ChangeNotifier {
|
|||||||
session = nextSession;
|
session = nextSession;
|
||||||
nextSession.setRouteVisible(active && visibleIndices.isNotEmpty);
|
nextSession.setRouteVisible(active && visibleIndices.isNotEmpty);
|
||||||
_notifyAfterFrameIfNeeded();
|
_notifyAfterFrameIfNeeded();
|
||||||
|
await _waitForFrame();
|
||||||
|
if (revision != feedRevision || !identical(session, nextSession)) return;
|
||||||
await previous.close();
|
await previous.close();
|
||||||
if (revision != feedRevision || !identical(session, nextSession)) return;
|
if (revision != feedRevision || !identical(session, nextSession)) return;
|
||||||
_jumpToFeedPage(targetIndex);
|
_jumpToFeedPage(targetIndex);
|
||||||
|
|||||||
@@ -123,6 +123,38 @@ void main() {
|
|||||||
await session.close();
|
await session.close();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test(
|
||||||
|
'should_detach_retiring_player_before_disposal_finishes',
|
||||||
|
() async {
|
||||||
|
final disposeStarted = Completer<void>();
|
||||||
|
final disposeGate = Completer<void>();
|
||||||
|
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<FakePlayer>());
|
||||||
|
await session.close();
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
test('should_keep_user_pause_when_reselecting_the_same_episode', () async {
|
test('should_keep_user_pause_when_reselecting_the_same_episode', () async {
|
||||||
final factory = FakeFactory();
|
final factory = FakeFactory();
|
||||||
final session = PlaybackSession(
|
final session = PlaybackSession(
|
||||||
@@ -164,8 +196,14 @@ void main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
class FakeFactory {
|
class FakeFactory {
|
||||||
FakeFactory({this.holdFirstInitialize = false});
|
FakeFactory({
|
||||||
|
this.holdFirstInitialize = false,
|
||||||
|
this.firstDisposeStarted,
|
||||||
|
this.firstDisposeGate,
|
||||||
|
});
|
||||||
final bool holdFirstInitialize;
|
final bool holdFirstInitialize;
|
||||||
|
final Completer<void>? firstDisposeStarted;
|
||||||
|
final Completer<void>? firstDisposeGate;
|
||||||
final List<FakePlayer> created = [];
|
final List<FakePlayer> created = [];
|
||||||
final Completer<void> firstCreated = Completer<void>();
|
final Completer<void> firstCreated = Completer<void>();
|
||||||
int live = 0;
|
int live = 0;
|
||||||
@@ -176,6 +214,8 @@ class FakeFactory {
|
|||||||
if (live > peakLive) peakLive = live;
|
if (live > peakLive) peakLive = live;
|
||||||
final player = FakePlayer(
|
final player = FakePlayer(
|
||||||
holdInitialize: holdFirstInitialize && created.isEmpty,
|
holdInitialize: holdFirstInitialize && created.isEmpty,
|
||||||
|
disposeStarted: created.isEmpty ? firstDisposeStarted : null,
|
||||||
|
disposeGate: created.isEmpty ? firstDisposeGate : null,
|
||||||
onDispose: () => live--,
|
onDispose: () => live--,
|
||||||
);
|
);
|
||||||
created.add(player);
|
created.add(player);
|
||||||
@@ -185,9 +225,16 @@ class FakeFactory {
|
|||||||
}
|
}
|
||||||
|
|
||||||
class FakePlayer implements PlayerPort {
|
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 bool holdInitialize;
|
||||||
final void Function() onDispose;
|
final void Function() onDispose;
|
||||||
|
final Completer<void>? disposeStarted;
|
||||||
|
final Completer<void>? disposeGate;
|
||||||
final Completer<void> _initializeGate = Completer<void>();
|
final Completer<void> _initializeGate = Completer<void>();
|
||||||
bool initialized = false;
|
bool initialized = false;
|
||||||
bool disposed = false;
|
bool disposed = false;
|
||||||
@@ -211,6 +258,10 @@ class FakePlayer implements PlayerPort {
|
|||||||
@override
|
@override
|
||||||
Future<void> dispose() async {
|
Future<void> dispose() async {
|
||||||
if (disposed) return;
|
if (disposed) return;
|
||||||
|
if (disposeStarted != null && !disposeStarted!.isCompleted) {
|
||||||
|
disposeStarted!.complete();
|
||||||
|
}
|
||||||
|
await disposeGate?.future;
|
||||||
disposed = true;
|
disposed = true;
|
||||||
onDispose();
|
onDispose();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ PlayerLogic _logic(FakeWatchHistory history, {Drama? drama}) {
|
|||||||
pictureInPicture: AndroidPictureInPicture(
|
pictureInPicture: AndroidPictureInPicture(
|
||||||
channel: const MethodChannel('test/progress/pip'),
|
channel: const MethodChannel('test/progress/pip'),
|
||||||
),
|
),
|
||||||
|
waitForFrame: () async {},
|
||||||
);
|
);
|
||||||
if (drama != null) {
|
if (drama != null) {
|
||||||
logic.attach(
|
logic.attach(
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import 'package:flutter/material.dart';
|
import 'package:flutter/material.dart';
|
||||||
import 'package:flutter/services.dart';
|
import 'package:flutter/services.dart';
|
||||||
import 'package:flutter_test/flutter_test.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/drama.dart';
|
||||||
import 'package:fruitshorts/core/data/models/play_ticket.dart';
|
import 'package:fruitshorts/core/data/models/play_ticket.dart';
|
||||||
import 'package:fruitshorts/core/data/models/player_port.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_controller.dart';
|
||||||
import 'package:fruitshorts/core/data/services/download_storage.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/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_logic.dart';
|
||||||
import 'package:fruitshorts/pages/player/player_view.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 '../support/fake_watch_history.dart';
|
||||||
import 'package:fruitshorts/core/data/repositories/playback_repository.dart';
|
import 'package:fruitshorts/core/data/repositories/playback_repository.dart';
|
||||||
@@ -45,10 +48,12 @@ AndroidPictureInPicture _pip() =>
|
|||||||
PlayerLogic _logic(
|
PlayerLogic _logic(
|
||||||
FakeWatchHistory history, {
|
FakeWatchHistory history, {
|
||||||
PlaybackRepository? playback,
|
PlaybackRepository? playback,
|
||||||
|
Future<void> Function()? waitForFrame,
|
||||||
}) => PlayerLogic(
|
}) => PlayerLogic(
|
||||||
history: history,
|
history: history,
|
||||||
playback: playback ?? FakePlaybackRepository(),
|
playback: playback ?? FakePlaybackRepository(),
|
||||||
pictureInPicture: _pip(),
|
pictureInPicture: _pip(),
|
||||||
|
waitForFrame: waitForFrame ?? () async {},
|
||||||
);
|
);
|
||||||
|
|
||||||
/// Drains the session's serialized work so a test never ends mid-disposal.
|
/// Drains the session's serialized work so a test never ends mid-disposal.
|
||||||
@@ -681,6 +686,49 @@ void main() {
|
|||||||
logic.dispose();
|
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', () {
|
group('lifecycle', () {
|
||||||
@@ -889,3 +937,70 @@ class _RecordingPlayer implements PlayerPort {
|
|||||||
onDispose();
|
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<void> initialize() async {
|
||||||
|
controller.value = const VideoPlayerValue(
|
||||||
|
duration: Duration(minutes: 1),
|
||||||
|
size: Size(1280, 720),
|
||||||
|
position: Duration(seconds: 1),
|
||||||
|
isInitialized: true,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
Future<void> play() async {}
|
||||||
|
|
||||||
|
@override
|
||||||
|
Future<void> pause() async {}
|
||||||
|
|
||||||
|
@override
|
||||||
|
Future<void> dispose() async {
|
||||||
|
if (disposed) return;
|
||||||
|
disposed = true;
|
||||||
|
await controller.dispose();
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user