diff --git a/lib/providers/shader_provider.dart b/lib/providers/shader_provider.dart index 0023b8ba..ae9e12da 100644 --- a/lib/providers/shader_provider.dart +++ b/lib/providers/shader_provider.dart @@ -1,15 +1,16 @@ +import 'dart:async' show unawaited; + import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import '../mixins/disposable_change_notifier_mixin.dart'; import '../models/shader_preset.dart'; +import '../services/settings_binding_owner.dart'; import '../services/settings_service.dart'; import '../services/shader_asset_loader.dart'; class ShaderProvider extends ChangeNotifier with DisposableChangeNotifierMixin { - SettingsService? _settingsService; - ValueNotifier? _savedPresetListenable; - ValueNotifier>>? _customPresetsListenable; + late final SettingsBindingOwner _settingsBinding; ShaderPreset _savedPreset = ShaderPreset.none; ShaderPreset _currentPreset = ShaderPreset.none; @@ -17,30 +18,14 @@ class ShaderProvider extends ChangeNotifier with DisposableChangeNotifierMixin { bool _initialized = false; ShaderProvider() { - _initialize(); + _settingsBinding = SettingsBindingOwner( + prefs: [SettingsService.globalShaderPreset, SettingsService.customShaderPresets], + onRefresh: _syncFromSettings, + ); + unawaited(_settingsBinding.bind()); } - Future _initialize() async { - final service = await SettingsService.getInstance(); - if (_settingsService == service && _savedPresetListenable != null && _customPresetsListenable != null) { - _syncFromSettings(); - return; - } - - _savedPresetListenable?.removeListener(_onSettingsChanged); - _customPresetsListenable?.removeListener(_onSettingsChanged); - _settingsService = service; - _savedPresetListenable = service.listenable(SettingsService.globalShaderPreset)..addListener(_onSettingsChanged); - _customPresetsListenable = service.listenable(SettingsService.customShaderPresets)..addListener(_onSettingsChanged); - _syncFromSettings(); - } - - void _onSettingsChanged() => _syncFromSettings(); - - void _syncFromSettings() { - final service = _settingsService; - if (service == null) return; - + void _syncFromSettings(SettingsService service) { final customData = service.read(SettingsService.customShaderPresets); final customPresets = customData.map((json) => ShaderPreset.fromJson(json)).toList(); _customPresets = customPresets; @@ -55,8 +40,7 @@ class ShaderProvider extends ChangeNotifier with DisposableChangeNotifierMixin { @override void dispose() { - _savedPresetListenable?.removeListener(_onSettingsChanged); - _customPresetsListenable?.removeListener(_onSettingsChanged); + _settingsBinding.dispose(); super.dispose(); } @@ -72,12 +56,12 @@ class ShaderProvider extends ChangeNotifier with DisposableChangeNotifierMixin { } Future setPreset(ShaderPreset preset) async { - final service = _settingsService ?? await SettingsService.getInstance(); + final service = _settingsBinding.settings ?? await SettingsService.getInstance(); await service.write(SettingsService.globalShaderPreset, preset.id); final changed = _savedPreset.id != preset.id || _currentPreset.id != preset.id; _savedPreset = preset; _currentPreset = preset; - if (changed || _savedPresetListenable == null) { + if (changed || _settingsBinding.settings == null) { safeNotifyListeners(); } } @@ -119,7 +103,7 @@ class ShaderProvider extends ChangeNotifier with DisposableChangeNotifierMixin { } Future _saveCustomPresets() async { - final service = _settingsService ?? await SettingsService.getInstance(); + final service = _settingsBinding.settings ?? await SettingsService.getInstance(); final data = _customPresets.map((p) => p.toJson()).toList(); await service.write(SettingsService.customShaderPresets, data); } diff --git a/lib/providers/theme_provider.dart b/lib/providers/theme_provider.dart index 0469fec1..ea0bfe8a 100644 --- a/lib/providers/theme_provider.dart +++ b/lib/providers/theme_provider.dart @@ -1,14 +1,15 @@ +import 'dart:async' show unawaited; import 'dart:io' show Platform; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:material_symbols_icons/symbols.dart'; import '../mixins/disposable_change_notifier_mixin.dart'; +import '../services/settings_binding_owner.dart'; import '../services/settings_service.dart' as settings; import '../theme/mono_theme.dart'; class ThemeProvider extends ChangeNotifier with DisposableChangeNotifierMixin { - settings.SettingsService? _settingsService; - ValueNotifier? _themeModeListenable; + late final SettingsBindingOwner _settingsBinding; settings.ThemeMode _themeMode = settings.ThemeMode.system; late Brightness _systemBrightness; @@ -19,7 +20,11 @@ class ThemeProvider extends ChangeNotifier with DisposableChangeNotifierMixin { // async path below lands a microtask too late for the first build. final loaded = settings.SettingsService.instanceOrNull; if (loaded != null) _themeMode = loaded.read(settings.SettingsService.themeMode); - _initializeSettings(); + _settingsBinding = SettingsBindingOwner( + prefs: const [settings.SettingsService.themeMode], + onRefresh: (service) => _syncThemeMode(service.read(settings.SettingsService.themeMode)), + ); + unawaited(_settingsBinding.bind()); WidgetsBinding.instance.platformDispatcher.onPlatformBrightnessChanged = _onBrightnessChanged; } @@ -32,33 +37,13 @@ class ThemeProvider extends ChangeNotifier with DisposableChangeNotifierMixin { @override void dispose() { - _themeModeListenable?.removeListener(_onThemeModeSettingChanged); + _settingsBinding.dispose(); if (WidgetsBinding.instance.platformDispatcher.onPlatformBrightnessChanged == _onBrightnessChanged) { WidgetsBinding.instance.platformDispatcher.onPlatformBrightnessChanged = null; } super.dispose(); } - Future _initializeSettings() async { - final service = await settings.SettingsService.getInstance(); - if (_settingsService == service && _themeModeListenable != null) { - _syncThemeMode(service.read(settings.SettingsService.themeMode)); - return; - } - - _themeModeListenable?.removeListener(_onThemeModeSettingChanged); - _settingsService = service; - _themeModeListenable = service.listenable(settings.SettingsService.themeMode) - ..addListener(_onThemeModeSettingChanged); - _syncThemeMode(_themeModeListenable!.value); - } - - void _onThemeModeSettingChanged() { - final listenable = _themeModeListenable; - if (listenable == null) return; - _syncThemeMode(listenable.value); - } - void _syncThemeMode(settings.ThemeMode mode, {bool forceNotify = false}) { final changed = _themeMode != mode; _themeMode = mode; @@ -106,14 +91,14 @@ class ThemeProvider extends ChangeNotifier with DisposableChangeNotifierMixin { Future setThemeMode(settings.ThemeMode mode) async { if (_themeMode == mode) return; - final service = _settingsService ?? await settings.SettingsService.getInstance(); + final service = _settingsBinding.settings ?? await settings.SettingsService.getInstance(); await service.write(settings.SettingsService.themeMode, mode); - if (_themeModeListenable == null) _syncThemeMode(mode); + if (_settingsBinding.settings == null) _syncThemeMode(mode); } Future reload() async { - await _initializeSettings(); - final service = _settingsService; + await _settingsBinding.bind(); + final service = _settingsBinding.settings; if (service != null) _syncThemeMode(service.read(settings.SettingsService.themeMode), forceNotify: true); } diff --git a/lib/services/keyboard_shortcuts_service.dart b/lib/services/keyboard_shortcuts_service.dart index 9ec3c8c7..64c59e7c 100644 --- a/lib/services/keyboard_shortcuts_service.dart +++ b/lib/services/keyboard_shortcuts_service.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import '../models/hotkey_model.dart'; import '../i18n/strings.g.dart'; import '../mpv/mpv.dart'; +import 'settings_binding_owner.dart'; import 'settings_service.dart'; import '../utils/platform_detector.dart'; import '../utils/player_utils.dart'; @@ -14,14 +15,26 @@ class KeyboardShortcutsService extends ChangeNotifier { static const Set _repeatableVideoActions = {'zoom_in', 'zoom_out'}; static KeyboardShortcutsService? _instance; - late SettingsService _settingsService; - final List _settingsDisposers = []; + late final SettingsBindingOwner _settingsBinding; Map _hotkeys = {}; int _seekTimeSmall = 10; // Default, loaded from settings int _seekTimeLarge = 30; // Default, loaded from settings int _maxVolume = 100; // Default, loaded from settings (100-300%) + bool _settingsInitialized = false; - KeyboardShortcutsService._(); + KeyboardShortcutsService._() { + _settingsBinding = SettingsBindingOwner( + prefs: [ + SettingsService.keyboardHotkeys, + SettingsService.seekTimeSmall, + SettingsService.seekTimeLarge, + SettingsService.maxVolume, + ], + onRefresh: _syncFromSettings, + ); + } + + SettingsService get _settingsService => _settingsBinding.settings!; static Future getInstance() async { if (_instance == null) { @@ -37,32 +50,14 @@ class KeyboardShortcutsService extends ChangeNotifier { } Future _init() async { - _settingsService = await SettingsService.getInstance(); - _bindSettings(); - _syncFromSettings(notify: false); + await _settingsBinding.bind(); } - void _bindSettings() { - if (_settingsDisposers.isNotEmpty) return; - void bind(Pref pref) { - final notifier = _settingsService.listenable(pref); - notifier.addListener(_onSettingsChanged); - _settingsDisposers.add(() => notifier.removeListener(_onSettingsChanged)); - } - - bind(SettingsService.keyboardHotkeys); - bind(SettingsService.seekTimeSmall); - bind(SettingsService.seekTimeLarge); - bind(SettingsService.maxVolume); - } - - void _onSettingsChanged() => _syncFromSettings(); - - void _syncFromSettings({bool notify = true}) { - final hotkeys = _settingsService.read(SettingsService.keyboardHotkeys); - final seekTimeSmall = _settingsService.read(SettingsService.seekTimeSmall); - final seekTimeLarge = _settingsService.read(SettingsService.seekTimeLarge); - final maxVolume = _settingsService.read(SettingsService.maxVolume); + void _syncFromSettings(SettingsService service) { + final hotkeys = service.read(SettingsService.keyboardHotkeys); + final seekTimeSmall = service.read(SettingsService.seekTimeSmall); + final seekTimeLarge = service.read(SettingsService.seekTimeLarge); + final maxVolume = service.read(SettingsService.maxVolume); final changed = !_hotkeyMapsEqual(_hotkeys, hotkeys) || @@ -75,6 +70,8 @@ class KeyboardShortcutsService extends ChangeNotifier { _seekTimeLarge = seekTimeLarge; _maxVolume = maxVolume; + final notify = _settingsInitialized; + _settingsInitialized = true; if (notify && changed) notifyListeners(); } @@ -99,7 +96,7 @@ class KeyboardShortcutsService extends ChangeNotifier { } Future refreshFromStorage() async { - _syncFromSettings(); + _settingsBinding.refresh(); } Future resetToDefaults() async { @@ -109,10 +106,7 @@ class KeyboardShortcutsService extends ChangeNotifier { @override void dispose() { - for (final dispose in _settingsDisposers) { - dispose(); - } - _settingsDisposers.clear(); + _settingsBinding.dispose(); if (identical(_instance, this)) _instance = null; super.dispose(); } diff --git a/lib/services/settings_binding_owner.dart b/lib/services/settings_binding_owner.dart new file mode 100644 index 00000000..78c6937a --- /dev/null +++ b/lib/services/settings_binding_owner.dart @@ -0,0 +1,87 @@ +import 'package:flutter/foundation.dart'; + +import 'settings_service.dart'; + +typedef SettingsServiceAcquirer = Future Function(); + +/// Owns a group of preference listeners backed by one [SettingsService]. +class SettingsBindingOwner { + factory SettingsBindingOwner({ + required Iterable> prefs, + required void Function(SettingsService service) onRefresh, + SettingsServiceAcquirer? acquireSettings, + }) => SettingsBindingOwner._(List.unmodifiable(prefs), onRefresh, acquireSettings); + + SettingsBindingOwner._(this._prefs, this._onRefresh, this._acquireSettings); + + final List> _prefs; + final void Function(SettingsService service) _onRefresh; + final SettingsServiceAcquirer? _acquireSettings; + final List _listenables = []; + + SettingsService? _settings; + Future? _bindingFuture; + int _generation = 0; + bool _disposed = false; + + SettingsService? get settings => _settings; + bool get isBound => !_disposed && _settings != null; + + Future bind() { + if (_disposed) return Future.value(); + + final pending = _bindingFuture; + if (pending != null) return pending; + + final generation = ++_generation; + late final Future future; + future = _bind(generation).whenComplete(() { + if (identical(_bindingFuture, future)) _bindingFuture = null; + }); + _bindingFuture = future; + return future; + } + + Future _bind(int generation) async { + final acquirer = _acquireSettings; + final service = await (acquirer?.call() ?? SettingsService.getInstance()); + if (_disposed || generation != _generation) return; + + if (!identical(_settings, service)) { + _removeListeners(); + _settings = service; + for (final pref in _prefs) { + final listenable = service.listenableOf(pref)..addListener(_handlePreferenceChanged); + _listenables.add(listenable); + } + } + + _onRefresh(service); + } + + void refresh() { + final service = _settings; + if (_disposed || service == null) return; + _onRefresh(service); + } + + void _handlePreferenceChanged() { + final service = _settings; + if (_disposed || service == null) return; + _onRefresh(service); + } + + void _removeListeners() { + for (final listenable in _listenables) { + listenable.removeListener(_handlePreferenceChanged); + } + _listenables.clear(); + } + + void dispose() { + if (_disposed) return; + _disposed = true; + _generation++; + _removeListeners(); + } +} diff --git a/test/services/settings_binding_owner_test.dart b/test/services/settings_binding_owner_test.dart new file mode 100644 index 00000000..50c51f58 --- /dev/null +++ b/test/services/settings_binding_owner_test.dart @@ -0,0 +1,88 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/services/settings_binding_owner.dart'; +import 'package:plezy/services/settings_service.dart'; + +import '../test_helpers/prefs.dart'; + +void main() { + setUp(() { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + }); + + test('refreshes when a bound preference changes', () async { + final settings = await SettingsService.getInstance(); + final values = []; + final binding = SettingsBindingOwner( + prefs: [SettingsService.maxVolume], + onRefresh: (service) => values.add(service.read(SettingsService.maxVolume)), + ); + addTearDown(binding.dispose); + + await binding.bind(); + await settings.write(SettingsService.maxVolume, 175); + + expect(values, [100, 175]); + }); + + test('disposal before initialization ignores the stale completion', () async { + final settings = await SettingsService.getInstance(); + final acquired = Completer(); + var refreshes = 0; + final binding = SettingsBindingOwner( + prefs: [SettingsService.maxVolume], + onRefresh: (_) => refreshes++, + acquireSettings: () => acquired.future, + ); + + final initialized = binding.bind(); + binding.dispose(); + acquired.complete(settings); + await initialized; + await settings.write(SettingsService.maxVolume, 150); + + expect(refreshes, 0); + expect(binding.isBound, isFalse); + }); + + test('duplicate binding shares initialization and registers one listener', () async { + final settings = await SettingsService.getInstance(); + final acquired = Completer(); + var acquisitions = 0; + var refreshes = 0; + final binding = SettingsBindingOwner( + prefs: [SettingsService.maxVolume], + onRefresh: (_) => refreshes++, + acquireSettings: () { + acquisitions++; + return acquired.future; + }, + ); + addTearDown(binding.dispose); + + final first = binding.bind(); + final second = binding.bind(); + acquired.complete(settings); + await Future.wait([first, second]); + await binding.bind(); + await settings.write(SettingsService.maxVolume, 125); + + expect(acquisitions, 2); + expect(refreshes, 3); + }); + + test('does not refresh after disposal', () async { + final settings = await SettingsService.getInstance(); + var refreshes = 0; + final binding = SettingsBindingOwner(prefs: [SettingsService.maxVolume], onRefresh: (_) => refreshes++); + + await binding.bind(); + binding.dispose(); + await settings.write(SettingsService.maxVolume, 200); + binding.refresh(); + + expect(refreshes, 1); + }); +}