fix: scope navigation callbacks to active owners
This commit is contained in:
@@ -29,22 +29,25 @@ mixin TabNavigationMixin<T extends StatefulWidget> on State<T>, TickerProviderSt
|
||||
/// Number of tabs — derived from [tabChipFocusNodes].
|
||||
int get tabCount => tabChipFocusNodes.length;
|
||||
|
||||
/// Initialise the [TabController] and register gamepad callbacks.
|
||||
/// Initialise the [TabController] and register owner-scoped gamepad callbacks.
|
||||
/// Call from [initState].
|
||||
void initTabNavigation() {
|
||||
tabController = TabController(length: tabCount, vsync: this);
|
||||
tabController.addListener(onTabChanged);
|
||||
GamepadService.onL1Pressed = goToPreviousTab;
|
||||
GamepadService.onR1Pressed = goToNextTab;
|
||||
GamepadService.registerTabNavigation(
|
||||
this,
|
||||
previous: goToPreviousTab,
|
||||
next: goToNextTab,
|
||||
isActive: () => mounted && TickerMode.getValuesNotifier(context).value.enabled,
|
||||
);
|
||||
}
|
||||
|
||||
/// Dispose the [TabController] and clear gamepad callbacks.
|
||||
/// Dispose the [TabController] and remove only this screen's callbacks.
|
||||
/// Call from [dispose].
|
||||
void disposeTabNavigation() {
|
||||
GamepadService.unregisterTabNavigation(this);
|
||||
tabController.removeListener(onTabChanged);
|
||||
tabController.dispose();
|
||||
GamepadService.onL1Pressed = null;
|
||||
GamepadService.onR1Pressed = null;
|
||||
}
|
||||
|
||||
void goToPreviousTab() {
|
||||
|
||||
@@ -681,6 +681,7 @@ class _MainScreenState extends State<MainScreen>
|
||||
appLogger.d('System shelf tap: $contentId');
|
||||
_handleShelfContentId(contentId);
|
||||
};
|
||||
_systemShelfTapCallback = systemShelf.onShelfItemTap;
|
||||
|
||||
// Check for pending deep link from cold start
|
||||
WidgetsBinding.instance.addPostFrameCallback((_) async {
|
||||
@@ -738,6 +739,7 @@ class _MainScreenState extends State<MainScreen>
|
||||
}
|
||||
|
||||
bool _companionRemoteSetup = false;
|
||||
ValueChanged<String>? _systemShelfTapCallback;
|
||||
|
||||
@override
|
||||
void didChangeDependencies() {
|
||||
@@ -803,7 +805,7 @@ class _MainScreenState extends State<MainScreen>
|
||||
};
|
||||
|
||||
final receiver = CompanionRemoteReceiver.instance;
|
||||
|
||||
receiver.navigationOwner = this;
|
||||
receiver.onTabNext = () {
|
||||
final tabs = _getVisibleTabs(_isOffline);
|
||||
final idx = tabs.indexWhere((t) => t.id == _currentTab);
|
||||
@@ -877,18 +879,27 @@ class _MainScreenState extends State<MainScreen>
|
||||
_contentFocusScope.dispose();
|
||||
_setTvosMenuPassthrough(false);
|
||||
|
||||
// Clean up companion remote callbacks
|
||||
// Clean up only callbacks still owned by this screen. A replacement
|
||||
// MainScreen may already have installed its callbacks this frame.
|
||||
if (_companionRemoteSetup) {
|
||||
final receiver = CompanionRemoteReceiver.instance;
|
||||
receiver.onTabNext = null;
|
||||
receiver.onTabPrevious = null;
|
||||
receiver.onTabDiscover = null;
|
||||
receiver.onTabLibraries = null;
|
||||
receiver.onTabSearch = null;
|
||||
receiver.onTabDownloads = null;
|
||||
receiver.onTabSettings = null;
|
||||
receiver.onHome = null;
|
||||
receiver.onSearchAction = null;
|
||||
if (identical(receiver.navigationOwner, this)) {
|
||||
receiver.onTabNext = null;
|
||||
receiver.onTabPrevious = null;
|
||||
receiver.onTabDiscover = null;
|
||||
receiver.onTabLibraries = null;
|
||||
receiver.onTabSearch = null;
|
||||
receiver.onTabDownloads = null;
|
||||
receiver.onTabSettings = null;
|
||||
receiver.onHome = null;
|
||||
receiver.onSearchAction = null;
|
||||
receiver.navigationOwner = null;
|
||||
}
|
||||
}
|
||||
final shelfCallback = _systemShelfTapCallback;
|
||||
final systemShelf = SystemShelfService();
|
||||
if (shelfCallback != null && identical(systemShelf.onShelfItemTap, shelfCallback)) {
|
||||
systemShelf.onShelfItemTap = null;
|
||||
}
|
||||
|
||||
super.dispose();
|
||||
|
||||
@@ -3,6 +3,8 @@ part of '../../video_player_screen.dart';
|
||||
extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState {
|
||||
void _setupCompanionRemoteCallbacks() {
|
||||
final receiver = CompanionRemoteReceiver.instance;
|
||||
receiver.playerHomeFallback ??= receiver.onHome;
|
||||
receiver.playerOwner = this;
|
||||
receiver.onStop = () {
|
||||
if (mounted) _handleBackButton();
|
||||
};
|
||||
@@ -47,8 +49,9 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState {
|
||||
receiver.onAudioTracks = _cycleAudioTrack;
|
||||
receiver.onFullscreen = _toggleFullscreen;
|
||||
|
||||
// Override home to exit the player first (main screen handler runs after pop)
|
||||
_savedOnHome = receiver.onHome;
|
||||
// Override home to exit the player first. Replacements inherit the base
|
||||
// MainScreen callback rather than chaining through the outgoing player.
|
||||
_savedOnHome = receiver.playerHomeFallback;
|
||||
receiver.onHome = () {
|
||||
if (mounted) _handleHomeButton();
|
||||
};
|
||||
@@ -64,6 +67,10 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState {
|
||||
|
||||
void _cleanupCompanionRemoteCallbacks() {
|
||||
final receiver = CompanionRemoteReceiver.instance;
|
||||
if (!identical(receiver.playerOwner, this)) {
|
||||
_companionRemoteProvider = null;
|
||||
return;
|
||||
}
|
||||
receiver.onStop = null;
|
||||
receiver.onNextTrack = null;
|
||||
receiver.onPreviousTrack = null;
|
||||
@@ -75,10 +82,12 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState {
|
||||
receiver.onSubtitles = null;
|
||||
receiver.onAudioTracks = null;
|
||||
receiver.onFullscreen = null;
|
||||
receiver.onHome = _savedOnHome;
|
||||
receiver.onHome = receiver.playerHomeFallback;
|
||||
receiver.playerHomeFallback = null;
|
||||
receiver.playerOwner = null;
|
||||
_savedOnHome = null;
|
||||
|
||||
// Notify remote that player is no longer active
|
||||
// Notify only when the active player owner exits.
|
||||
_companionRemoteProvider?.sendCommand(RemoteCommandType.syncState, data: {'playerActive': false});
|
||||
_companionRemoteProvider = null;
|
||||
}
|
||||
|
||||
@@ -458,6 +458,7 @@ class VideoPlayerScreenState extends State<VideoPlayerScreen> with WidgetsBindin
|
||||
VideoPIPManager? _videoPIPManager;
|
||||
ShaderService? _shaderService;
|
||||
AmbientLightingService? _ambientLightingService;
|
||||
bool _fullscreenListenerAttached = false;
|
||||
Size? _lastVideoLayoutSize;
|
||||
Size? _pendingVideoLayoutSize;
|
||||
Player? _lastVideoLayoutPlayer;
|
||||
@@ -760,7 +761,10 @@ class VideoPlayerScreenState extends State<VideoPlayerScreen> with WidgetsBindin
|
||||
_displayModeService = DisplayModeService(settingsService, FullscreenStateManager());
|
||||
await _displayModeService!.syncWithNative();
|
||||
if (!mounted) return;
|
||||
FullscreenStateManager().addListener(_onFullscreenChanged);
|
||||
if (!_fullscreenListenerAttached) {
|
||||
FullscreenStateManager().addListener(_onFullscreenChanged);
|
||||
_fullscreenListenerAttached = true;
|
||||
}
|
||||
}
|
||||
|
||||
// One-native-instance rule: a live music session owns the only audio
|
||||
@@ -1297,8 +1301,9 @@ class VideoPlayerScreenState extends State<VideoPlayerScreen> with WidgetsBindin
|
||||
TraktScrobbleService.instance.stopPlayback();
|
||||
TrackerCoordinator.instance.stopPlayback();
|
||||
|
||||
if (Platform.isWindows && _displayModeService != null) {
|
||||
if (_fullscreenListenerAttached) {
|
||||
FullscreenStateManager().removeListener(_onFullscreenChanged);
|
||||
_fullscreenListenerAttached = false;
|
||||
}
|
||||
if (!isReplacingWithVideo &&
|
||||
Platform.isWindows &&
|
||||
|
||||
@@ -19,6 +19,12 @@ class CompanionRemoteReceiver {
|
||||
/// Same pattern as [GamepadService.onGamepadInput].
|
||||
static VoidCallback? onRemoteInput;
|
||||
|
||||
/// Owners prevent a disposed screen from clearing callbacks installed by a
|
||||
/// replacement screen later in the same frame.
|
||||
Object? navigationOwner;
|
||||
Object? playerOwner;
|
||||
VoidCallback? playerHomeFallback;
|
||||
|
||||
VoidCallback? onTabNext;
|
||||
VoidCallback? onTabPrevious;
|
||||
VoidCallback? onTabDiscover;
|
||||
|
||||
@@ -2,7 +2,6 @@ import 'dart:async';
|
||||
import 'dart:io';
|
||||
import 'dart:ui' as ui;
|
||||
|
||||
import 'package:flutter/scheduler.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter/widgets.dart';
|
||||
import 'package:universal_gamepad/universal_gamepad.dart';
|
||||
@@ -146,13 +145,41 @@ class GamepadService with WindowListener {
|
||||
/// Set by InputModeTracker when it initializes.
|
||||
static VoidCallback? onGamepadInput;
|
||||
|
||||
/// Callback for L1 bumper press (previous tab).
|
||||
/// Screens with tabs can listen to this.
|
||||
static VoidCallback? onL1Pressed;
|
||||
static final Map<Object, ({VoidCallback previous, VoidCallback next, bool Function() isActive})>
|
||||
_tabNavigationHandlers = {};
|
||||
|
||||
/// Callback for R1 bumper press (next tab).
|
||||
/// Screens with tabs can listen to this.
|
||||
static VoidCallback? onR1Pressed;
|
||||
/// Registers owner-scoped bumper navigation. Multiple tab screens can stay
|
||||
/// mounted; only the handler whose screen is currently visible runs.
|
||||
static void registerTabNavigation(
|
||||
Object owner, {
|
||||
required VoidCallback previous,
|
||||
required VoidCallback next,
|
||||
required bool Function() isActive,
|
||||
}) {
|
||||
_tabNavigationHandlers[owner] = (previous: previous, next: next, isActive: isActive);
|
||||
}
|
||||
|
||||
static void unregisterTabNavigation(Object owner) {
|
||||
_tabNavigationHandlers.remove(owner);
|
||||
}
|
||||
|
||||
static void _dispatchTabNavigation({required bool previous}) {
|
||||
for (final handler in _tabNavigationHandlers.values) {
|
||||
if (!handler.isActive()) continue;
|
||||
previous ? handler.previous() : handler.next();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@visibleForTesting
|
||||
static void debugDispatchTabNavigation({required bool previous}) {
|
||||
_dispatchTabNavigation(previous: previous);
|
||||
}
|
||||
|
||||
@visibleForTesting
|
||||
static void debugClearTabNavigationHandlers() {
|
||||
_tabNavigationHandlers.clear();
|
||||
}
|
||||
|
||||
// Deadzone for analog sticks (0.0 to 1.0)
|
||||
static const double _stickDeadzone = 0.5;
|
||||
@@ -408,9 +435,9 @@ class GamepadService with WindowListener {
|
||||
_logGamepadDiag('button simulates key press back ${_describeGamepadButton(event)}');
|
||||
_simulateKeyPress(LogicalKeyboardKey.gameButtonB);
|
||||
case GamepadButton.leftShoulder:
|
||||
onL1Pressed?.call();
|
||||
_dispatchTabNavigation(previous: true);
|
||||
case GamepadButton.rightShoulder:
|
||||
onR1Pressed?.call();
|
||||
_dispatchTabNavigation(previous: false);
|
||||
default:
|
||||
break;
|
||||
}
|
||||
@@ -458,11 +485,11 @@ class GamepadService with WindowListener {
|
||||
return;
|
||||
}
|
||||
|
||||
// Switch to keyboard mode on significant axis input
|
||||
// Switch to keyboard mode on significant axis input. Navigation itself
|
||||
// schedules frames only when the stick crosses the real deadzone.
|
||||
if (event.value.abs() > 0.3) {
|
||||
onGamepadInput?.call();
|
||||
_setTraditionalFocusHighlight();
|
||||
SchedulerBinding.instance.ensureVisualUpdate();
|
||||
}
|
||||
|
||||
switch (event.axis) {
|
||||
|
||||
@@ -8,7 +8,7 @@ import 'package:plezy/services/gamepad_service.dart';
|
||||
/// Tests stage [tabCount] focus nodes and read the resulting controller state
|
||||
/// after [initTabNavigation] runs.
|
||||
class _Probe extends StatefulWidget {
|
||||
const _Probe({required this.tabCount, required this.onState, this.initialIndex = 0});
|
||||
const _Probe({super.key, required this.tabCount, required this.onState, this.initialIndex = 0});
|
||||
|
||||
final int tabCount;
|
||||
final int initialIndex;
|
||||
@@ -60,16 +60,9 @@ void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
group('TabNavigationMixin', () {
|
||||
setUp(() {
|
||||
// Static gamepad callbacks are global state; isolate each test.
|
||||
GamepadService.onL1Pressed = null;
|
||||
GamepadService.onR1Pressed = null;
|
||||
});
|
||||
setUp(GamepadService.debugClearTabNavigationHandlers);
|
||||
|
||||
tearDown(() {
|
||||
GamepadService.onL1Pressed = null;
|
||||
GamepadService.onR1Pressed = null;
|
||||
});
|
||||
tearDown(GamepadService.debugClearTabNavigationHandlers);
|
||||
|
||||
testWidgets('initTabNavigation creates a TabController with the right length', (tester) async {
|
||||
late _ProbeState state;
|
||||
@@ -83,30 +76,45 @@ void main() {
|
||||
expect(state.suppressAutoFocus, isFalse);
|
||||
});
|
||||
|
||||
testWidgets('initTabNavigation registers L1/R1 gamepad callbacks', (tester) async {
|
||||
testWidgets('initTabNavigation registers owner-scoped bumper navigation', (tester) async {
|
||||
late _ProbeState state;
|
||||
await tester.pumpWidget(_Probe(tabCount: 3, onState: (s) => state = s));
|
||||
|
||||
// Mixin wires its private goToPreviousTab/goToNextTab to the static
|
||||
// callbacks; we can only assert non-null wiring (the closures are the
|
||||
// mixin's bound methods, not directly comparable).
|
||||
expect(GamepadService.onL1Pressed, isNotNull);
|
||||
expect(GamepadService.onR1Pressed, isNotNull);
|
||||
// Sanity: invoking R1 advances the tab via the mixin's goToNextTab.
|
||||
GamepadService.onR1Pressed!.call();
|
||||
GamepadService.debugDispatchTabNavigation(previous: false);
|
||||
await tester.pump();
|
||||
expect(state.tabController.index, 1);
|
||||
});
|
||||
|
||||
testWidgets('disposeTabNavigation clears the static gamepad callbacks', (tester) async {
|
||||
await tester.pumpWidget(_Probe(tabCount: 2, onState: (_) {}));
|
||||
expect(GamepadService.onL1Pressed, isNotNull);
|
||||
expect(GamepadService.onR1Pressed, isNotNull);
|
||||
testWidgets('disposing one tab screen preserves another owner registration', (tester) async {
|
||||
late _ProbeState first;
|
||||
late _ProbeState second;
|
||||
await tester.pumpWidget(
|
||||
Directionality(
|
||||
textDirection: TextDirection.ltr,
|
||||
child: Column(
|
||||
children: [
|
||||
_Probe(key: const ValueKey('first'), tabCount: 2, onState: (s) => first = s),
|
||||
_Probe(key: const ValueKey('second'), tabCount: 2, onState: (s) => second = s),
|
||||
],
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
// Replace the widget tree to fire dispose.
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
expect(GamepadService.onL1Pressed, isNull);
|
||||
expect(GamepadService.onR1Pressed, isNull);
|
||||
GamepadService.debugDispatchTabNavigation(previous: false);
|
||||
await tester.pump();
|
||||
expect(first.tabController.index, 1);
|
||||
expect(second.tabController.index, 0);
|
||||
|
||||
await tester.pumpWidget(
|
||||
Directionality(
|
||||
textDirection: TextDirection.ltr,
|
||||
child: _Probe(key: const ValueKey('second'), tabCount: 2, onState: (s) => second = s),
|
||||
),
|
||||
);
|
||||
GamepadService.debugDispatchTabNavigation(previous: false);
|
||||
await tester.pump();
|
||||
|
||||
expect(second.tabController.index, 1);
|
||||
});
|
||||
|
||||
testWidgets('tabChipFocusNodes drives tabCount; getTabChipFocusNode returns the right node', (tester) async {
|
||||
@@ -183,10 +191,11 @@ void main() {
|
||||
expect(state.tabController.length, 3);
|
||||
expect(state.tabController.index, 0);
|
||||
|
||||
// The original is disposed; touching it would throw — but the
|
||||
// mixin's references all point at the new instance now.
|
||||
expect(GamepadService.onL1Pressed, isNotNull);
|
||||
expect(GamepadService.onR1Pressed, isNotNull);
|
||||
// The original is disposed, while the owner-scoped registry points at
|
||||
// the newly initialized controller.
|
||||
GamepadService.debugDispatchTabNavigation(previous: false);
|
||||
await tester.pump();
|
||||
expect(state.tabController.index, 1);
|
||||
});
|
||||
|
||||
testWidgets('onTabChanged fires when tabController.index changes', (tester) async {
|
||||
|
||||
Reference in New Issue
Block a user