diff --git a/lib/providers/collab/remote_strategy_snapshot_provider.dart b/lib/providers/collab/remote_strategy_snapshot_provider.dart index 8e7e79be..c1a3f698 100644 --- a/lib/providers/collab/remote_strategy_snapshot_provider.dart +++ b/lib/providers/collab/remote_strategy_snapshot_provider.dart @@ -164,6 +164,9 @@ class RemoteEditorSnapshotNotifier } else { await _refreshFromServer(); } + // Closed, or another strategy opened, while this one was being read: + // there is nothing left to watch for it. + if (_activeStrategyPublicId != strategyPublicId) return; await _startShellSubscription(strategyPublicId); final pageId = _activePagePublicId; if (pageId != null) { @@ -383,6 +386,8 @@ class RemoteEditorSnapshotNotifier _pagesToOpen(shell).firstOrNull ?? shell.pages.firstOrNull?.publicId; void clear() { + // A read still on its way must not bring the strategy back. + _readEpoch += 1; _activeStrategyPublicId = null; _activePagePublicId = null; _shareToken = null; diff --git a/lib/providers/share_link_provider.dart b/lib/providers/share_link_provider.dart index 28262620..df721ff3 100644 --- a/lib/providers/share_link_provider.dart +++ b/lib/providers/share_link_provider.dart @@ -18,9 +18,12 @@ final pendingShareCodeStoreProvider = Provider( (ref) => createPendingShareCodeStore(), ); -/// The page's own URL on web (where a share link arrives), null on native. -/// Overridden in tests. -final sharePageUrlProvider = Provider((ref) => kIsWeb ? Uri.base : null); +/// Reads the page's own URL on web (where a share link arrives), null on +/// native. Read each time it is needed: the router and this controller both +/// change it while the page stays open. Overridden in tests. +final sharePageUrlProvider = Provider( + (ref) => () => kIsWeb ? Uri.base : null, +); /// Replaces the address bar URL without navigating. Overridden in tests. final replaceBrowserUrlProvider = @@ -281,6 +284,29 @@ class ShareLinkController extends Notifier { return redeemPendingIfPossible(source: 'add_to_library', explicit: true); } + /// Whether a page reload reopens the held link. The editor says so as the + /// open strategy changes (see openCloudStrategyRecorderProvider): no while + /// the reader has a strategy of their own open, because a reload reopens + /// that one, and yes again once the strategy they read through the link is + /// back. Without this a reload restored both, and the two editors fought. + /// The link stays held in memory either way, so going back to its strategy + /// in this tab still reads through it. + void reopenOnReload(bool reopen) { + final token = state; + // Only a link that has opened. One still waiting to open (it arrived + // offline, say) keeps its code and its address: a reload is its retry. + if (token == null || ref.read(shareLinkViewProvider)?.token != token) { + return; + } + final store = ref.read(pendingShareCodeStoreProvider); + if (reopen) { + store.write(token); + } else { + store.clear(); + } + _pointPageUrlAt(reopen ? token : null); + } + /// Whether [token]'s attempt is stale: a newer code arrived, or sign-in /// redeemed this one while the attempt was waiting on the server. bool _superseded(String token, int generation) => @@ -310,10 +336,14 @@ class ShareLinkController extends Notifier { /// finishing late cannot clear a newer code that arrived meanwhile. int _generation = 0; + /// The held code, the stored code and the address say the same link from + /// here on: a reload opens the link in the address, so an address left on + /// an older link would retry that one and drop this one. void _hold(String token) { _generation += 1; state = token; ref.read(pendingShareCodeStoreProvider).write(token); + _pointPageUrlAt(token); } void _release(int generation) { @@ -322,24 +352,31 @@ class ShareLinkController extends Notifier { } state = null; ref.read(pendingShareCodeStoreProvider).clear(); - _dropSharePathFromPageUrl(); + _pointPageUrlAt(null); } - /// Once a code is done with, the page must not keep its /share/ URL, + /// Makes the page's URL the link for [code], or drops the link from it + /// when [code] is null. A reload opens whatever link the URL names, so + /// once a code is done with the page must not keep its /share/ URL, /// or every reload would redeem it again (and, after the link is disabled, /// report a failure to someone who still has access). The fragment stays: /// Flutter's router keeps the open route there. - void _dropSharePathFromPageUrl() { - final page = ref.read(sharePageUrlProvider); - if (page == null || - !isIcarusShareUri(page, currentOrigin: currentShareOrigin())) { - return; - } + void _pointPageUrlAt(String? code) { + final page = ref.read(sharePageUrlProvider)(); + if (page == null) return; + final origin = currentShareOrigin(); + final isLink = isIcarusShareUri(page, currentOrigin: origin); + final alreadyThere = code == null + ? !isLink + : isLink && + extractIcarusShareCode(page.toString(), currentOrigin: origin) == + code; + if (alreadyThere) return; ref.read(replaceBrowserUrlProvider)(Uri( scheme: page.scheme, host: page.host, port: page.hasPort ? page.port : null, - path: '/', + path: code == null ? '/' : '/share/$code', fragment: page.fragment.isEmpty ? null : page.fragment, )); } diff --git a/lib/providers/strategy_page_session_provider.dart b/lib/providers/strategy_page_session_provider.dart index 25046a2c..2d719dde 100644 --- a/lib/providers/strategy_page_session_provider.dart +++ b/lib/providers/strategy_page_session_provider.dart @@ -5,6 +5,7 @@ import 'dart:math' show max; import 'package:flutter/foundation.dart' show listEquals, setEquals; import 'package:flutter/widgets.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:icarus/strategy/open_attempt.dart'; import 'package:hive_ce/hive.dart'; import 'package:icarus/collab/cloud_lineup_rows.dart'; import 'package:icarus/collab/collab_models.dart'; @@ -330,13 +331,17 @@ class StrategyPageSessionNotifier extends Notifier { required StrategySource source, required bool selectFirstPageIfNeeded, String? preferredPageId, + OpenAttempt? attempt, }) async { + // The open this is a step of, if any. It stops here too when the open + // gave way (see [OpenAttempt]). + final opening = attempt ?? OpenAttempt.unchallenged; _pageSessionGeneration++; // As in reset: a switch this supersedes leaves its transition here. ref.read(transitionProvider.notifier).complete(); state = state.copyWith(transitionState: PageTransitionState.idle); final pageSource = _resolvePageSource(strategyId, source); - final pageIds = await pageSource.listPageIds(); + final pageIds = await opening.wait(pageSource.listPageIds()); // Drawn from this computer's copy, the page the copy could show is the // only one there is to open (see RemoteEditorSnapshotNotifier). final copiedPage = source == StrategySource.cloud @@ -368,7 +373,9 @@ class StrategyPageSessionNotifier extends Notifier { ); if (selected != null) { - await _rehydrateActivePageFromSource(selected); + await opening.wait( + _rehydrateActivePageFromSource(selected, attempt: opening), + ); } // Snapshots that arrived while the strategy was opening went unheard: // catch up on pages that came back while it was closed. @@ -1185,11 +1192,15 @@ class StrategyPageSessionNotifier extends Notifier { } } + /// Reads the page an open starts on and puts it on the canvas. A step of + /// the open [attempt] belongs to, so it stops like one (see [OpenAttempt]). Future _rehydrateActivePageFromSource( String pageId, { + required OpenAttempt attempt, _RemotePageHydrationKey? hydrationKey, bool preserveTextDrafts = false, }) async { + final opening = attempt; final strategyState = ref.read(strategyProvider); final strategyId = strategyState.strategyId; final source = strategyState.source; @@ -1208,14 +1219,17 @@ class StrategyPageSessionNotifier extends Notifier { ); } final pageSource = _resolvePageSource(strategyId, source); - final pageData = await pageSource.loadPage(pageId); - await _applyLoadedPageData( - pageData, - strategyId: strategyId, - source: source, - hydrationKey: hydrationKey, - preserveTextDrafts: preserveTextDrafts, - loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot, + final pageData = await opening.wait(pageSource.loadPage(pageId)); + await opening.wait( + _applyLoadedPageData( + pageData, + strategyId: strategyId, + source: source, + hydrationKey: hydrationKey, + preserveTextDrafts: preserveTextDrafts, + loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot, + attempt: opening, + ), ); } @@ -1228,9 +1242,14 @@ class StrategyPageSessionNotifier extends Notifier { RemoteEditorSnapshot? loadedRemoteSnapshot, ({MapValue map, StrategyThemeState theme})? preservedMetadata, bool Function()? canApply, + OpenAttempt? attempt, }) async { - final availablePageIds = - await _resolvePageSource(strategyId, source).listPageIds(); + // The open this page is read for, if it is one: a page read for an open + // that gave way must not replace the canvas of what took its place. + final opening = attempt ?? OpenAttempt.unchallenged; + final availablePageIds = await opening.wait( + _resolvePageSource(strategyId, source).listPageIds(), + ); if (canApply != null && !canApply()) return; final preserveHistory = source == StrategySource.cloud && _lastHydratedRemotePageKey?.strategyPublicId == strategyId && @@ -1252,13 +1271,15 @@ class StrategyPageSessionNotifier extends Notifier { ? Map.from(ref.read(textDraftProvider)) : const {}; try { - await applyStrategyEditorPageData( - ref, - pageData, - themeProfileId: themeProfileId, - themeOverridePalette: themeOverridePalette, - preserveHistory: preserveHistory, - mapOverride: preservedMetadata?.map, + await opening.wait( + applyStrategyEditorPageData( + ref, + pageData, + themeProfileId: themeProfileId, + themeOverridePalette: themeOverridePalette, + preserveHistory: preserveHistory, + mapOverride: preservedMetadata?.map, + ), ); for (final entry in retainedTextDrafts.entries) { ref.read(textDraftProvider.notifier).setDraft(entry.key, entry.value); @@ -1286,11 +1307,14 @@ class StrategyPageSessionNotifier extends Notifier { : hydrationKey, ); } finally { - state = state.copyWith( - activePageId: pageData.pageId, - isApplyingPage: false, - ); - _resumePendingRemoteReapplyIfPossible(); + // After an open gave way the session belongs to what replaced it. + if (opening.isCurrent) { + state = state.copyWith( + activePageId: pageData.pageId, + isApplyingPage: false, + ); + _resumePendingRemoteReapplyIfPossible(); + } } } diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index 3f65d8a7..abaecfd3 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -14,6 +14,7 @@ import 'package:icarus/const/line_provider.dart'; import 'package:icarus/const/maps.dart'; import 'package:icarus/providers/image_provider.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:icarus/strategy/open_attempt.dart'; import 'package:icarus/const/hive_boxes.dart'; import 'package:icarus/const/settings.dart'; import 'package:icarus/l10n/l10n.dart'; @@ -100,6 +101,26 @@ typedef NewPageGaps = ({ }); class StrategyProvider extends Notifier { + /// Bumped when a strategy starts to open and when the open one is closed. + int _openAttempt = 0; + + /// Runs one attempt to open a strategy. Every wait in [steps] goes through + /// the attempt it is given (see [OpenAttempt]), so an open the reader left, + /// or replaced by opening another strategy, stops at that wait and changes + /// nothing after it. Giving way is not a failure, so nothing is thrown: + /// the answer is whether the strategy opened. False means something else + /// is open now (or nothing), and a caller that goes on to act on "its" + /// strategy must look first. + Future _open(Future Function(OpenAttempt opening) steps) async { + final attempt = ++_openAttempt; + try { + return await steps(OpenAttempt(() => attempt == _openAttempt)); + } on OpenGaveWay { + // The newer open, or the close, owns the outcome. + return false; + } + } + @override StrategyState build() { _registerPersistenceTrackingListeners(); @@ -340,8 +361,9 @@ class StrategyProvider extends Notifier { } /// Opens the cloud strategy [strategyID] in the editor, on [pageId] when - /// it has that page. - Future openCloudStrategy(String strategyID, {String? pageId}) async { + /// it has that page. False when it gave way to a newer open or a close + /// before it finished (see [_open]); throws when it could not be read. + Future openCloudStrategy(String strategyID, {String? pageId}) async { // A cloud copy still being built by a move from this device is the // mover's: an edit made in it would be lost if the move fails. if (ref.read(movingToCloudProvider).contains(strategyID)) { @@ -350,45 +372,54 @@ class StrategyProvider extends Notifier { cancelPendingSave(); ref.read(strategySaveStateProvider.notifier).reset(); - await ref - .read(remoteEditorSnapshotProvider.notifier) - .openStrategy(strategyID, activePagePublicId: pageId); - final snapshotState = ref.read(remoteEditorSnapshotProvider); - final snapshot = snapshotState.valueOrNull; - if (snapshot == null) { - // Returning silently here used to leave the editor on an eternal - // skeleton with no feedback. Throw so callers can surface the failure - // and back out. - throw StateError( - 'Cloud strategy snapshot unavailable for $strategyID' - '${snapshotState.hasError ? ': ${snapshotState.error}' : ''}', + return _open((opening) async { + await opening.wait( + ref + .read(remoteEditorSnapshotProvider.notifier) + .openStrategy(strategyID, activePagePublicId: pageId), ); - } + final snapshotState = ref.read(remoteEditorSnapshotProvider); + final snapshot = snapshotState.valueOrNull; + if (snapshot == null) { + // Returning silently here used to leave the editor on an eternal + // skeleton with no feedback. Throw so callers can surface the + // failure and back out. + throw StateError( + 'Cloud strategy snapshot unavailable for $strategyID' + '${snapshotState.hasError ? ': ${snapshotState.error}' : ''}', + ); + } - final storageDirectory = kIsWeb - ? null - : (await setStorageDirectory(snapshot.header.publicId)).path; - state = state.copyWith( - strategyId: snapshot.header.publicId, - strategyName: snapshot.header.name, - source: StrategySource.cloud, - storageDirectory: storageDirectory, - isOpen: true, - ); + final storageDirectory = kIsWeb + ? null + : (await opening.wait(setStorageDirectory(snapshot.header.publicId))) + .path; + state = state.copyWith( + strategyId: snapshot.header.publicId, + strategyName: snapshot.header.name, + source: StrategySource.cloud, + storageDirectory: storageDirectory, + isOpen: true, + ); - await ref.read(strategyPageSessionProvider.notifier).initializeForStrategy( - strategyId: snapshot.header.publicId, - source: StrategySource.cloud, - selectFirstPageIfNeeded: true, - preferredPageId: pageId, - ); - // Each page the editor reads is checked for images to upload as it - // arrives (see RemoteEditorSnapshotNotifier). - unawaited( - ref.read(cloudMediaUploadQueueProvider.notifier).setActiveStrategy( - snapshot.header.publicId, - ), - ); + await opening.wait( + ref.read(strategyPageSessionProvider.notifier).initializeForStrategy( + strategyId: snapshot.header.publicId, + source: StrategySource.cloud, + selectFirstPageIfNeeded: true, + preferredPageId: pageId, + attempt: opening, + ), + ); + // Each page the editor reads is checked for images to upload as it + // arrives (see RemoteEditorSnapshotNotifier). + unawaited( + ref.read(cloudMediaUploadQueueProvider.notifier).setActiveStrategy( + snapshot.header.publicId, + ), + ); + return true; + }); } Future switchPage(String pageID) async { @@ -796,6 +827,7 @@ class StrategyProvider extends Notifier { } Future clearCurrentStrategy() async { + _openAttempt += 1; cancelPendingSave(); ref.read(strategyThemeProvider.notifier).fromStrategy(); ref.read(strategySaveStateProvider.notifier).reset(); @@ -2036,7 +2068,7 @@ class StrategyProvider extends Notifier { } /// Opens strategy [id] on its first page, or on [pageId] when given. - Future loadFromHive(String id, {String? pageId}) async { + Future loadFromHive(String id, {String? pageId}) async { _requireLocalLibrary('open'); // Edits made while a strategy moves to the cloud would go with the // local original the move deletes. @@ -2044,61 +2076,79 @@ class StrategyProvider extends Notifier { throw StateError('This strategy is moving to the cloud.'); } cancelPendingSave(); - final newStrat = Hive.box(HiveBoxNames.strategiesBox) - .values - .where((StrategyData strategy) { - return strategy.id == id; - }).firstOrNull; - if (newStrat == null) { - return; - } - _withoutPersistenceTracking(() { - ref.read(actionProvider.notifier).resetActionState(); - }); + return _open((opening) async { + final newStrat = Hive.box(HiveBoxNames.strategiesBox) + .values + .where((StrategyData strategy) { + return strategy.id == id; + }).firstOrNull; - List pageImageData = []; - for (final page in newStrat.pages) { - pageImageData.addAll(page.imageData); - } - if (!kIsWeb) { - List allImageIds = []; + if (newStrat == null) { + return false; + } + _withoutPersistenceTracking(() { + ref.read(actionProvider.notifier).resetActionState(); + }); + + List pageImageData = []; for (final page in newStrat.pages) { - allImageIds.addAll(page.imageData.map((image) => image.pictureId)); - for (final link in page.lineUpLinks) { - allImageIds.addAll(link.images.map((image) => image.id)); + pageImageData.addAll(page.imageData); + } + if (!kIsWeb) { + List allImageIds = []; + for (final page in newStrat.pages) { + allImageIds.addAll(page.imageData.map((image) => image.pictureId)); + for (final link in page.lineUpLinks) { + allImageIds.addAll(link.images.map((image) => image.id)); + } } + await opening.wait( + ref + .read(placedImageProvider.notifier) + .deleteUnusedImages(newStrat.id, allImageIds), + ); } - await ref - .read(placedImageProvider.notifier) - .deleteUnusedImages(newStrat.id, allImageIds); - } - // We clear previous data to avoid artifacts when loading a new strategy - final migratedStrategy = StrategyMigrator.migrateToCurrentVersion(newStrat); + // We clear previous data to avoid artifacts when loading a new strategy + final migratedStrategy = + StrategyMigrator.migrateToCurrentVersion(newStrat); - if (migratedStrategy != newStrat) { - await Hive.box(HiveBoxNames.strategiesBox) - .put(migratedStrategy.id, migratedStrategy); - } - - final newDir = - kIsWeb ? null : await setStorageDirectory(migratedStrategy.id); - state = StrategyState( - strategyId: migratedStrategy.id, - strategyName: migratedStrategy.name, - source: StrategySource.local, - storageDirectory: newDir?.path, - isOpen: true, - ); - ref.read(strategySaveStateProvider.notifier).reset(); - await ref.read(strategyPageSessionProvider.notifier).initializeForStrategy( - strategyId: migratedStrategy.id, - source: StrategySource.local, - selectFirstPageIfNeeded: true, - preferredPageId: pageId, + if (migratedStrategy != newStrat) { + await opening.wait( + Hive.box(HiveBoxNames.strategiesBox) + .put(migratedStrategy.id, migratedStrategy), ); - ref.read(strategySaveStateProvider.notifier).markPersisted(); + } + + final newDir = kIsWeb + ? null + : await opening.wait(setStorageDirectory(migratedStrategy.id)); + // A cloud strategy still being read, or the one this replaces in + // place, has no say in a local one: its late answer, or a link its + // owner disables, must not reach this editor. + final remote = ref.read(remoteEditorSnapshotProvider.notifier); + if (remote.activeStrategyPublicId != null) remote.clear(); + state = StrategyState( + strategyId: migratedStrategy.id, + strategyName: migratedStrategy.name, + source: StrategySource.local, + storageDirectory: newDir?.path, + isOpen: true, + ); + ref.read(strategySaveStateProvider.notifier).reset(); + await opening.wait( + ref.read(strategyPageSessionProvider.notifier).initializeForStrategy( + strategyId: migratedStrategy.id, + source: StrategySource.local, + selectFirstPageIfNeeded: true, + preferredPageId: pageId, + attempt: opening, + ), + ); + ref.read(strategySaveStateProvider.notifier).markPersisted(); + return true; + }); } /// The page settings a new strategy starts with: the user's defaults. diff --git a/lib/services/open_cloud_strategy_store.dart b/lib/services/open_cloud_strategy_store.dart index b878423e..17b036b1 100644 --- a/lib/services/open_cloud_strategy_store.dart +++ b/lib/services/open_cloud_strategy_store.dart @@ -45,16 +45,24 @@ final openCloudStrategyStoreProvider = Provider( /// editor goes away the store is cleared. A strategy open through a share /// link is not recorded: its page keeps the /share/ URL, and a reload /// reopens it through the link. +/// +/// A reload reopens one strategy, so the two never both hold: while the +/// reader's own strategy is recorded, the link is told not to reopen, and +/// it is told to again when its strategy is the open one. final openCloudStrategyRecorderProvider = Provider.autoDispose((ref) { final store = ref.watch(openCloudStrategyStoreProvider); void record() { final open = ref.read(strategyProvider); final id = open.strategyId; - final throughLink = ref.read(shareLinkViewProvider)?.strategyPublicId == id; + final throughLink = + id != null && ref.read(shareLinkViewProvider)?.strategyPublicId == id; + final link = ref.read(shareLinkControllerProvider.notifier); if (open.source == StrategySource.cloud && id != null && !throughLink) { store.write(id); + link.reopenOnReload(false); } else { store.clear(); + if (throughLink) link.reopenOnReload(true); } } diff --git a/lib/strategy/open_attempt.dart b/lib/strategy/open_attempt.dart new file mode 100644 index 00000000..f42215d4 --- /dev/null +++ b/lib/strategy/open_attempt.dart @@ -0,0 +1,39 @@ +/// One attempt to open a strategy in the editor. +/// +/// Opening takes several waits (the disk, the server, the pages), and the +/// reader can leave or open something else during any of them. An open that +/// gave way must do nothing further: not mark itself open, not touch the save +/// state or the upload queue, which by then belong to whatever replaced it. +/// +/// So every wait inside an open goes through [wait], which ends the open +/// right there if it gave way. test/open_attempt_test.dart reads the +/// functions an open is made of and fails on an `await` in them that does +/// not; a function they call that waits on its own has to be given the +/// attempt and added to that list. +class OpenAttempt { + OpenAttempt(this._isCurrent); + + /// An attempt nothing supersedes, for a caller that is not an open. + static final unchallenged = OpenAttempt(() => true); + + final bool Function() _isCurrent; + + /// Whether this open is still the one wanted. + bool get isCurrent => _isCurrent(); + + /// Waits for [step], then throws [OpenGaveWay] if this open was closed or + /// replaced meanwhile. Whoever started the attempt catches it. + Future wait(Future step) async { + final result = await step; + if (!isCurrent) throw const OpenGaveWay(); + return result; + } +} + +/// Ends an open that was closed, or replaced by a newer one, while it waited. +class OpenGaveWay implements Exception { + const OpenGaveWay(); + + @override + String toString() => 'The open gave way to something newer.'; +} diff --git a/lib/strategy_view.dart b/lib/strategy_view.dart index f8fe632c..3d770eba 100644 --- a/lib/strategy_view.dart +++ b/lib/strategy_view.dart @@ -35,6 +35,7 @@ import 'package:icarus/widgets/strategy_quick_switcher.dart'; import 'package:icarus/widgets/touch_strategy_viewer.dart'; import 'package:icarus/widgets/turn_phone_notice.dart'; import 'package:icarus/widgets/map_selector.dart'; +import 'package:icarus/widgets/mouse_navigation.dart'; import 'package:icarus/widgets/pages_bar.dart'; import 'package:icarus/widgets/editor_toolbar.dart'; import 'package:icarus/widgets/strategy_presence.dart'; @@ -132,6 +133,19 @@ class _StrategyViewState extends ConsumerState bool _isInitialLoadPending = false; bool _hasInitialLoadCompleted = false; + /// Set when the reader leaves for the library. The editor stays mounted + /// while it animates out, so being mounted does not mean still wanted. + bool _hasLeft = false; + + /// How the initial load waits. A reader who left meanwhile gets nothing + /// more from it: the load ends at that wait. test/open_attempt_test.dart + /// fails on an `await` in the load that does not go through here. + Future _unlessLeft(Future step) async { + final result = await step; + if (_hasLeft || !mounted) throw const _LeftWhileLoading(); + return result; + } + @override void initState() { super.initState(); @@ -170,32 +184,45 @@ class _StrategyViewState extends ConsumerState return; } + // What was open before this load: only a strategy opened since then + // means this load gave way to another. + final openBefore = ref.read(strategyProvider).strategyId; try { switch (source) { case StrategySource.local: - await ref - .read(strategyProvider.notifier) - .loadFromHive(strategyId, pageId: widget.initialPageId); + await _unlessLeft( + ref + .read(strategyProvider.notifier) + .loadFromHive(strategyId, pageId: widget.initialPageId), + ); case StrategySource.cloud: final viewingThroughLink = ref.read(shareLinkViewProvider)?.strategyPublicId == strategyId; - if (!viewingThroughLink && !await _waitForCloudWorkspace()) { + if (!viewingThroughLink && + !await _unlessLeft(_waitForCloudWorkspace())) { // Signed out, or the cloud is unreachable: nothing failed to // load, and the library says which, so go back without an error. - if (mounted) Navigator.maybePop(context); + _leaveAfterFailedLoad(); return; } - await ref.read(strategyProvider.notifier).openCloudStrategy( - strategyId, - pageId: widget.initialPageId, - ); + await _unlessLeft( + ref.read(strategyProvider.notifier).openCloudStrategy( + strategyId, + pageId: widget.initialPageId, + ), + ); } final loadedStrategy = ref.read(strategyProvider); if (loadedStrategy.strategyId != strategyId || loadedStrategy.strategyName == null) { + // Giving way to another strategy is not a failed load. + if (_gaveWayTo(loadedStrategy.strategyId, openBefore)) return; throw StateError('Strategy "$strategyId" was not found.'); } _hasInitialLoadCompleted = true; + } on _LeftWhileLoading { + // The home button works from the first frame; the load went with the + // editor. } catch (error, stackTrace) { developer.log( 'Error loading strategy: $error', @@ -203,22 +230,26 @@ class _StrategyViewState extends ConsumerState error: error, stackTrace: stackTrace, ); + if (_hasLeft || !mounted) return; // Refused: the access-lost dialog says so over the skeleton, and // leads out (see StrategyAccessWatcher). if (ref.read(strategyAccessLostProvider)) return; + // Another strategy was opened while this one loaded: nothing failed + // that the reader should hear about. + if (_gaveWayTo(ref.read(strategyProvider).strategyId, openBefore)) { + return; + } final notKept = ref.read(remoteEditorSnapshotProvider).error is StrategyNotKeptOffline; - if (mounted) { - Settings.showToast( - message: notKept - ? context.l10n.syncStatusStrategyNeedsConnection - : context.l10n.strategyViewLoadFailed, - backgroundColor: notKept - ? Settings.tacticalVioletTheme.secondary - : Settings.tacticalVioletTheme.destructive, - ); - Navigator.maybePop(context); - } + Settings.showToast( + message: notKept + ? context.l10n.syncStatusStrategyNeedsConnection + : context.l10n.strategyViewLoadFailed, + backgroundColor: notKept + ? Settings.tacticalVioletTheme.secondary + : Settings.tacticalVioletTheme.destructive, + ); + _leaveAfterFailedLoad(); } finally { if (mounted) { setState(() => _isInitialLoadPending = false); @@ -226,6 +257,47 @@ class _StrategyViewState extends ConsumerState } } + /// Whether [open], the strategy open now, took this editor's place while + /// its own was loading: it is another strategy, opened since the load + /// began ([openBefore] is what was open then). If so, this settles where + /// it shows. + /// + /// It shows in an editor of its own above this one when it was opened from + /// outside (a share link arriving as this editor starts), and then this + /// editor goes. Or it shows in this very editor, when a share link or + /// back and forward swapped strategies in place: then this editor stays + /// and shows it. Popping here would leave it open with nothing showing it. + bool _gaveWayTo(String? open, String? openBefore) { + if (open == null || + open == widget.initialStrategyId || + open == openBefore) { + return false; + } + final route = ModalRoute.of(context); + final top = mouseNavigationRouteObserver.topRoute; + final anotherEditorShowsIt = + top != route && top?.settings.name == Routes.strategyView; + if (!anotherEditorShowsIt) { + _hasInitialLoadCompleted = true; + } else if (route != null && route.isActive) { + Navigator.of(context).removeRoute(route); + } + return true; + } + + /// Takes this editor off the screen after its strategy failed to open. + /// Another screen may have been pushed over it meanwhile (a share link + /// opening its own editor), and a plain pop would close that one and + /// leave this one behind it, showing its loading screen for good. + void _leaveAfterFailedLoad() { + final route = ModalRoute.of(context); + if (route == null || route.isCurrent) { + Navigator.maybePop(context); + } else if (route.isActive) { + Navigator.of(context).removeRoute(route); + } + } + /// Resolves [cloudWorkspaceOutcomeProvider] once it settles. After a page /// reload the editor can open before sign-in is restored, so a cloud /// strategy waits here rather than failing on an unready backend. @@ -254,6 +326,7 @@ class _StrategyViewState extends ConsumerState .read(agentFilterProvider.notifier) .updateFilterState(FilterState.all); ref.read(deleteMenuProvider.notifier).requestClose(); + _hasLeft = true; if (mounted) { Navigator.pop(context); } @@ -299,12 +372,31 @@ class _StrategyViewState extends ConsumerState onLeave: _leaveToLibrary, child: Scaffold( // The skeleton is the desktop editor's shape, panels and all. + // The way back to the library works from the first frame: a load + // that never finishes must not hold the reader here. body: touch - ? const Center(child: WingDotLoader()) + ? Stack( + children: [ + const Center(child: WingDotLoader()), + SafeArea( + child: Padding( + padding: const EdgeInsets.all(6), + child: _LeaveButton( + onPressed: _leaveToLibrary, + backTooltip: widget.backTooltip, + ), + ), + ), + ], + ) : StrategyViewSkeleton( strategyName: widget.initialStrategyName, mapValue: widget.initialMapValue, isAttack: widget.initialIsAttack, + leading: _LeaveButton( + onPressed: _leaveToLibrary, + backTooltip: widget.backTooltip, + ), ), ), ); @@ -333,25 +425,9 @@ class _StrategyViewState extends ConsumerState Row( children: [ const SizedBox(width: 6), - ShadTooltip( - builder: (context) => - Text(widget.backTooltip ?? - context.l10n.strategyViewBackToLibraryTooltip), - child: ShadIconButton.ghost( - width: 28, - height: 28, - foregroundColor: - Settings.tacticalVioletTheme.mutedForeground, - hoverForegroundColor: - Settings.tacticalVioletTheme.foreground, - onPressed: _leaveToLibrary, - icon: Icon( - widget.backTooltip == null - ? LucideIcons.house300 - : LucideIcons.chevronLeft300, - size: 18, - ), - ), + _LeaveButton( + onPressed: _leaveToLibrary, + backTooltip: widget.backTooltip, ), const IcarusWordmark(), const Expanded( @@ -470,6 +546,42 @@ class _StrategyViewState extends ConsumerState } } +/// Ends the editor's initial load once the reader has left it. +class _LeftWhileLoading implements Exception { + const _LeftWhileLoading(); +} + +/// The editor's way out, at the left of the window strip: the house back to +/// the library, or a back arrow when [backTooltip] names somewhere else. +class _LeaveButton extends StatelessWidget { + const _LeaveButton({required this.onPressed, this.backTooltip}); + + final VoidCallback onPressed; + final String? backTooltip; + + @override + Widget build(BuildContext context) { + return ShadTooltip( + builder: (context) => Text( + backTooltip ?? context.l10n.strategyViewBackToLibraryTooltip, + ), + child: ShadIconButton.ghost( + width: 28, + height: 28, + foregroundColor: Settings.tacticalVioletTheme.mutedForeground, + hoverForegroundColor: Settings.tacticalVioletTheme.foreground, + onPressed: onPressed, + icon: Icon( + backTooltip == null + ? LucideIcons.house300 + : LucideIcons.chevronLeft300, + size: 18, + ), + ), + ); + } +} + /// The bug-report link at the end of the editor strip: text when there is /// room, the glyph alone when there isn't. class _DiscordLink extends StatelessWidget { diff --git a/lib/widgets/mouse_navigation.dart b/lib/widgets/mouse_navigation.dart index 527c9101..96fa4a02 100644 --- a/lib/widgets/mouse_navigation.dart +++ b/lib/widgets/mouse_navigation.dart @@ -319,6 +319,9 @@ class _MouseNavigationState extends ConsumerState { if (strategyOpen) { // Already in the strategy view; swap strategies in place, exactly // like the quick switcher does. + // An open that gave way to a newer one (or to a close) did not + // move here, and history must not record that it did. + var opened = false; final bool switched = await guardUnsavedStrategyExit( context: navigator.context, ref: ref, @@ -326,15 +329,15 @@ class _MouseNavigationState extends ConsumerState { onContinue: () async { _resetStrategyUiState(); final strategies = ref.read(strategyProvider.notifier); - switch (source) { - case StrategySource.local: - await strategies.loadFromHive(strategyId); - case StrategySource.cloud: - await strategies.openCloudStrategy(strategyId); - } + opened = switch (source) { + StrategySource.local => + await strategies.loadFromHive(strategyId), + StrategySource.cloud => + await strategies.openCloudStrategy(strategyId), + }; }, ); - return switched && mounted; + return switched && opened && mounted; } if (source == StrategySource.cloud) { diff --git a/lib/widgets/strategy_tile/strategy_tile.dart b/lib/widgets/strategy_tile/strategy_tile.dart index 0ac81364..cd77b339 100644 --- a/lib/widgets/strategy_tile/strategy_tile.dart +++ b/lib/widgets/strategy_tile/strategy_tile.dart @@ -599,7 +599,11 @@ class _StrategyTileState extends ConsumerState { return; } - await ref.read(strategyProvider.notifier).loadFromHive(_strategyId); + // The export saves the open canvas first, so it must be this + // strategy's: not one the reader opened while this one loaded. + final opened = + await ref.read(strategyProvider.notifier).loadFromHive(_strategyId); + if (!opened) return; await StrategyImportExportService(ref).exportFile(_strategyId); } diff --git a/lib/widgets/strategy_view_skeleton.dart b/lib/widgets/strategy_view_skeleton.dart index 6e72afcf..32c9da3d 100644 --- a/lib/widgets/strategy_view_skeleton.dart +++ b/lib/widgets/strategy_view_skeleton.dart @@ -16,14 +16,42 @@ class StrategyViewSkeleton extends StatelessWidget { this.strategyName, this.mapValue, this.isAttack = true, + this.leading, }); final String? strategyName; final MapValue? mapValue; final bool isAttack; + /// The editor's leave button. It sits where the editor will show it and + /// is the one part of the skeleton that takes clicks. + final Widget? leading; + @override Widget build(BuildContext context) { + final leading = this.leading; + if (leading == null) return _buildSkeleton(showLeadingBlock: true); + return Stack( + children: [ + _buildSkeleton(showLeadingBlock: false), + Positioned( + top: 0, + left: 0, + height: kWindowStripHeight, + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + const MacTrafficLightInset(), + const SizedBox(width: 6), + leading, + ], + ), + ), + ], + ); + } + + Widget _buildSkeleton({required bool showLeadingBlock}) { final resolvedMap = mapValue ?? MapValue.ascent; return ExcludeSemantics( child: IgnorePointer( @@ -34,6 +62,7 @@ class StrategyViewSkeleton extends StatelessWidget { strategyName: strategyName, mapValue: resolvedMap, isAttack: isAttack, + showLeadingBlock: showLeadingBlock, ), Expanded( child: Stack( @@ -161,6 +190,7 @@ class _SkeletonTopBar extends StatelessWidget { const _SkeletonTopBar({ required this.mapValue, required this.isAttack, + required this.showLeadingBlock, this.strategyName, }); @@ -168,20 +198,26 @@ class _SkeletonTopBar extends StatelessWidget { final MapValue mapValue; final bool isAttack; + /// False when the real leave button is drawn over this spot. + final bool showLeadingBlock; + @override Widget build(BuildContext context) { final title = strategyName?.trim(); return AppWindowStrip( child: Stack( children: [ - const Row( + Row( children: [ - SizedBox(width: 6), - _SkeletonBlock(width: 28, height: 28, radius: 8), - IcarusWordmark(), - Spacer(), - _SkeletonBlock(width: 200, height: 28, radius: 8), - SizedBox(width: 10), + const SizedBox(width: 6), + if (showLeadingBlock) + const _SkeletonBlock(width: 28, height: 28, radius: 8) + else + const SizedBox(width: 28), + const IcarusWordmark(), + const Spacer(), + const _SkeletonBlock(width: 200, height: 28, radius: 8), + const SizedBox(width: 10), ], ), Center( diff --git a/test/mouse_navigation_test.dart b/test/mouse_navigation_test.dart index 9c3298a4..00d0cae6 100644 --- a/test/mouse_navigation_test.dart +++ b/test/mouse_navigation_test.dart @@ -207,6 +207,51 @@ void main() { expect(strategies.opened, ['${recorded.name}:shared-id']); }); } + + testWidgets('a back whose open gave way does not count as having moved', + (tester) async { + // An open that gives way to a newer one returns without opening. Back + // used to record the move anyway, and history gained a place the + // reader never reached. + final strategies = _RecordingStrategies()..givesWay.add('shared-id'); + await tester.pumpWidget( + ProviderScope( + overrides: [ + platformPolicyProvider.overrideWithValue(PlatformPolicy.desktop), + isCloudWorkspaceAvailableProvider.overrideWithValue(true), + strategyProvider.overrideWith(() => strategies), + ], + child: ShadApp( + navigatorKey: appNavigatorKey, + navigatorObservers: [mouseNavigationRouteObserver], + builder: (_, child) => MouseNavigation(child: child!), + home: const Scaffold(body: Text('library')), + ), + ), + ); + strategies.show('shared-id', StrategySource.local); + await tester.pump(); + strategies.show('other-id', StrategySource.local); + await tester.pump(); + + Future back(int pointer) async { + await tester.sendEventToBinding( + PointerDownEvent( + pointer: pointer, + kind: PointerDeviceKind.mouse, + buttons: kBackMouseButton, + position: const Offset(400, 300), + ), + ); + await tester.pumpAndSettle(); + } + + await back(1); + expect(strategies.state.strategyId, 'other-id'); + // Still one step back from where the reader is: back tries it again. + await back(2); + expect(strategies.opened, ['local:shared-id', 'local:shared-id']); + }); }); } @@ -214,6 +259,9 @@ void main() { class _RecordingStrategies extends StrategyProvider { final opened = []; + /// Strategies whose open gives way before it finishes. + final givesWay = {}; + @override StrategyState build() => const StrategyState(); @@ -227,14 +275,18 @@ class _RecordingStrategies extends StrategyProvider { } @override - Future loadFromHive(String id, {String? pageId}) async { + Future loadFromHive(String id, {String? pageId}) async { opened.add('local:$id'); + if (givesWay.contains(id)) return false; show(id, StrategySource.local); + return true; } @override - Future openCloudStrategy(String id, {String? pageId}) async { + Future openCloudStrategy(String id, {String? pageId}) async { opened.add('cloud:$id'); + if (givesWay.contains(id)) return false; show(id, StrategySource.cloud); + return true; } } diff --git a/test/open_attempt_test.dart b/test/open_attempt_test.dart new file mode 100644 index 00000000..e6d5c32f --- /dev/null +++ b/test/open_attempt_test.dart @@ -0,0 +1,125 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:icarus/strategy/open_attempt.dart'; + +/// The body of the function whose declaration starts with [signature], from +/// its opening brace to the matching closing one. +String _bodyOf(String path, String signature) { + final source = File(path).readAsStringSync(); + final start = source.indexOf(signature); + expect(start, isNot(-1), reason: '$signature is no longer in $path'); + // The parameter list can hold braces of its own (named parameters), so + // the body starts at the brace after `async`. + final open = source.indexOf('{', source.indexOf(') async', start)); + var depth = 0; + for (var i = open; i < source.length; i++) { + if (source[i] == '{') depth++; + if (source[i] == '}' && --depth == 0) return source.substring(open, i + 1); + } + fail('unbalanced braces after $signature'); +} + +/// Each `await` in [body] that is not followed by [through]. +List _bareWaits(String body, String through) { + final withoutComments = body.replaceAll(RegExp(r'//.*'), ''); + return [ + for (final match in RegExp(r'\bawait\b\s*').allMatches(withoutComments)) + if (!withoutComments.startsWith(through, match.end)) + withoutComments + .substring(match.start) + .split('\n') + .first + .trim(), + ]; +} + +void main() { + test('a wait returns what it waited for while the open is current', + () async { + final opening = OpenAttempt(() => true); + expect(await opening.wait(Future.value(7)), 7); + }); + + test('a wait ends an open that gave way while it waited', () async { + var current = true; + final opening = OpenAttempt(() => current); + final step = Completer(); + final waited = opening.wait(step.future); + // The reader leaves, or opens another strategy, before the step answers. + current = false; + step.complete(7); + await expectLater(waited, throwsA(isA())); + }); + + // Three rounds of review each found one more wait inside an open after + // which a superseded open went on to change something: mark itself open, + // mark another strategy saved, point the upload queue at itself. The rule + // that ends that is mechanical, so it is checked mechanically: inside the + // functions that make up an open, nothing waits except through the helper + // that stops a superseded open. + // + // What this promises: no plain `await` in the functions listed here. What + // it does not: it reads only those functions. A function they call that + // waits and then writes on its own is not seen (the first version missed + // the page read for exactly that reason), so a new step of an open has to + // take the attempt and be added to the list. The two tests after the + // loop check that the steps known today are handed it. + group('every wait inside an open can end it', () { + const provider = 'lib/providers/strategy_provider.dart'; + const session = 'lib/providers/strategy_page_session_provider.dart'; + const editor = 'lib/strategy_view.dart'; + + for (final (path, signature, through) in [ + (provider, 'Future openCloudStrategy(', 'opening.wait('), + (provider, 'Future loadFromHive(', 'opening.wait('), + (session, 'Future initializeForStrategy(', 'opening.wait('), + (session, 'Future _rehydrateActivePageFromSource(', 'opening.wait('), + (session, 'Future _applyLoadedPageData(', 'opening.wait('), + (editor, 'Future _loadInitialStrategy(', '_unlessLeft('), + ]) { + test('$signature in $path', () { + expect( + _bareWaits(_bodyOf(path, signature), through), + isEmpty, + reason: 'Wait with `await $through...)` here. A plain await lets ' + 'an open the reader already left carry on to its next step.', + ); + }); + } + + test('the page read under the page step is given the attempt', () { + final initialize = + _bodyOf(session, 'Future initializeForStrategy('); + expect(initialize, contains('_rehydrateActivePageFromSource(')); + expect(initialize, contains('attempt: opening')); + final rehydrate = + _bodyOf(session, 'Future _rehydrateActivePageFromSource('); + expect(rehydrate, contains('_applyLoadedPageData(')); + expect(rehydrate, contains('attempt: opening')); + }); + + test('the page step of both opens is given the attempt', () { + for (final signature in [ + 'Future openCloudStrategy(', + 'Future loadFromHive(', + ]) { + final body = _bodyOf(provider, signature); + expect(body, contains('initializeForStrategy(')); + expect( + body, + contains('attempt: opening'), + reason: 'Without it the page session waits on its own and cannot ' + 'be stopped when $signature gives way.', + ); + } + }); + + test('the check itself catches a plain await', () { + const body = '{\n await opening.wait(a());\n await b();\n' + ' // await in a comment\n}'; + expect(_bareWaits(body, 'opening.wait('), ['await b();']); + }); + }); +} diff --git a/test/pending_share_code_store_test.dart b/test/pending_share_code_store_test.dart index 15084378..a9c0e64f 100644 --- a/test/pending_share_code_store_test.dart +++ b/test/pending_share_code_store_test.dart @@ -145,7 +145,7 @@ void main() { pendingShareCodeStoreProvider .overrideWithValue(store ?? MemoryPendingShareCodeStore()), authProvider.overrideWith(() => _SettableAuthProvider(auth)), - sharePageUrlProvider.overrideWithValue(pageUrl), + sharePageUrlProvider.overrideWithValue(() => pageUrl), replaceBrowserUrlProvider.overrideWithValue(replacedUrls.add), if (repository != null) convexStrategyRepositoryProvider.overrideWithValue(repository), diff --git a/test/remote_editor_snapshot_provider_test.dart b/test/remote_editor_snapshot_provider_test.dart index 8779603e..1df39781 100644 --- a/test/remote_editor_snapshot_provider_test.dart +++ b/test/remote_editor_snapshot_provider_test.dart @@ -77,6 +77,35 @@ void main() { ); }); + test('a strategy closed while it was still opening stays closed', () async { + // The editor's loading screen has a working home button, so a reader can + // leave before the first read answers. The late answer used to open the + // strategy anyway, with no editor showing it, and start its live reads. + final repository = _Repository(); + final container = ProviderContainer(overrides: [ + convexStrategyRepositoryProvider.overrideWithValue(repository), + authProvider.overrideWith(_ReadyAuthProvider.new), + strategyOpQueueProvider.overrideWith(_IdleOpQueue.new), + cloudMediaUploadQueueProvider.overrideWith(_IdleMediaQueue.new), + cloudMediaAccountIdProvider.overrideWithValue(null), + ]); + addTearDown(container.dispose); + final remote = container.read(remoteEditorSnapshotProvider.notifier); + await container.read(remoteEditorSnapshotProvider.future); + + final gate = Completer(); + repository.pageReadGate = gate; + final opening = remote.openStrategy('strategy', activePagePublicId: 'a'); + await _settle(); + remote.clear(); + gate.complete(); + await opening; + + expect(container.read(remoteEditorSnapshotProvider).valueOrNull, isNull); + expect(repository.shells.hasListener, isFalse); + expect(repository.watchedPages, isEmpty); + }); + test( 'a page answer counts as live only on the connection it came on, and ' 'never from the copy', () async { diff --git a/test/strategy_view_skeleton_test.dart b/test/strategy_view_skeleton_test.dart index 9886ce44..56dffdb4 100644 --- a/test/strategy_view_skeleton_test.dart +++ b/test/strategy_view_skeleton_test.dart @@ -18,6 +18,48 @@ class _FixedPreferences extends AppPreferencesNotifier { } void main() { + testWidgets('the leave button works while the skeleton shows', + (tester) async { + // The rest of the skeleton takes no clicks. Without a live way out, a + // load that never finished held the reader on this screen. + CoordinateSystem(playAreaSize: const Size(1120, 630)); + var left = 0; + await tester.pumpWidget( + ProviderScope( + overrides: [ + appPreferencesProvider.overrideWith(_FixedPreferences.new), + ], + child: ShadApp( + themeMode: ThemeMode.dark, + darkTheme: ShadThemeData( + brightness: Brightness.dark, + colorScheme: Settings.tacticalVioletTheme, + ), + home: MediaQuery( + data: const MediaQueryData( + size: Size(800, 630), + disableAnimations: true, + ), + child: StrategyViewSkeleton( + leading: IconButton( + onPressed: () => left++, + icon: const Icon(Icons.home), + ), + ), + ), + ), + ), + ); + await tester.pump(); + + final button = tester.getRect(find.byType(IconButton)); + final strip = tester.getRect(find.byType(AppWindowStrip)); + expect(button.center.dy, closeTo(strip.center.dy, 0.5)); + await tester.tap(find.byType(IconButton)); + expect(left, 1); + expect(tester.takeException(), isNull); + }); + testWidgets('loading skeleton fits the minimum desktop window', (tester) async { // DotGrid reads the play area at paint time. diff --git a/test/web_editor_reload_test.dart b/test/web_editor_reload_test.dart index db5d407c..702f37db 100644 --- a/test/web_editor_reload_test.dart +++ b/test/web_editor_reload_test.dart @@ -5,8 +5,11 @@ import 'package:icarus/config/platform_policy.dart'; import 'package:icarus/const/routes.dart'; import 'package:icarus/providers/auth_provider.dart'; import 'package:icarus/providers/library_workspace_provider.dart'; +import 'package:icarus/providers/share_link_provider.dart'; import 'package:icarus/providers/strategy_provider.dart'; import 'package:icarus/services/open_cloud_strategy_store.dart'; +import 'package:icarus/share/pending_share_code_store.dart'; +import 'package:icarus/share/share_link_format.dart'; import 'package:icarus/strategy/strategy_page_models.dart'; import 'package:icarus/strategy_view.dart'; import 'package:icarus/widgets/editor_toolbar.dart'; @@ -130,6 +133,71 @@ void main() { expect(store.read(), isNull, reason: 'leaving the editor clears it'); }); + test('a reload reopens the strategy that is open, not a link left behind', + () async { + // A reader opened a view link, then their own strategy, in one tab. Both + // used to stay set for the reload: the link in the address and the held + // code, their own strategy in the store. The reload opened two editors + // and one was left on its loading screen. + const code = 'ICR-2345-6789-ABCD-EFGH'; + var page = icarusProductionShareOrigin.replace( + path: '/share/$code', + fragment: '/strategy_view', + ); + final open = MemoryOpenCloudStrategyStore(); + final heldCode = MemoryPendingShareCodeStore()..write(code); + final strategy = _SettableStrategy(); + final container = ProviderContainer(overrides: [ + openCloudStrategyStoreProvider.overrideWithValue(open), + pendingShareCodeStoreProvider.overrideWithValue(heldCode), + sharePageUrlProvider.overrideWithValue(() => page), + replaceBrowserUrlProvider.overrideWithValue((url) => page = url), + strategyProvider.overrideWith(() => strategy), + // Signed in, the cloud not ready yet: a new code waits, held. + authProvider.overrideWith( + () => _FixedAuth(_auth(ConvexAuthStatus.configuring)), + ), + ]); + addTearDown(container.dispose); + container.read(shareLinkViewProvider.notifier).state = + (strategyPublicId: 'shared', token: code); + container.listen(openCloudStrategyRecorderProvider, (_, __) {}); + + strategy.open('shared', StrategySource.cloud); + expect(open.read(), isNull); + expect(heldCode.read(), code); + expect(page.path, '/share/$code'); + + strategy.open('mine', StrategySource.cloud); + expect(open.read(), 'mine'); + expect(heldCode.read(), isNull, reason: 'a reload must not reopen it'); + expect(page.path, '/', reason: 'the address follows what is open'); + expect(page.fragment, '/strategy_view', reason: 'the route stays'); + + // Back to the shared strategy in the same tab: the link still reads it, + // and it is what a reload reopens again. + strategy.open('shared', StrategySource.cloud); + expect(open.read(), isNull); + expect(heldCode.read(), code); + expect(page.path, '/share/$code'); + + // A second link arrived and has not opened yet (the cloud was not + // ready). The address names it from then on, with the stored code, so + // a reload retries that link and not the older one. + const waiting = 'ICR-XXXX-XXXX-XXXX-XXXX'; + await container + .read(shareLinkControllerProvider.notifier) + .redeemToken(waiting); + expect(heldCode.read(), waiting); + expect(page.path, '/share/$waiting'); + + // It is not left behind when a strategy opens: the reload is its retry. + strategy.open('mine', StrategySource.cloud); + expect(heldCode.read(), waiting); + expect(page.path, '/share/$waiting'); + expect(open.read(), 'mine'); + }); + testWidgets('the web beta toolbar never offers a local Save', (tester) async { for (final (policy, expected) in [ (PlatformPolicy.webBeta, findsNothing),