diff --git a/apps/weblibre/lib/features/geckoview/features/browser/presentation/widgets/browser_modules/browser_view.dart b/apps/weblibre/lib/features/geckoview/features/browser/presentation/widgets/browser_modules/browser_view.dart index 98b3bfb1..c98410da 100644 --- a/apps/weblibre/lib/features/geckoview/features/browser/presentation/widgets/browser_modules/browser_view.dart +++ b/apps/weblibre/lib/features/geckoview/features/browser/presentation/widgets/browser_modules/browser_view.dart @@ -281,21 +281,15 @@ class _BrowserViewState extends ConsumerState child: Visibility( visible: isGeckoViewVisible, child: GeckoView( - preInitializationStep: () async { - await ref - .read(eventServiceProvider) - .viewReadyStateEvents - .firstWhere((state) => state == true) - .timeout( - const Duration(seconds: 3), - onTimeout: () { - logger.e( - 'Browser fragement not reported ready, trying to intitialize anyways', - ); - return true; - }, - ); - }, + // Reports when the native container enters the window, which + // under the [Offstage] above is not until the home surface is + // dismissed. [GeckoView] attaches the browser fragment on every + // such report, so an engine kept alive but unpainted for the + // whole of startup still gets its fragment the moment it is + // shown. See https://github.com/FaFre/WebLibre/issues/557. + viewReadyEvents: ref + .read(eventServiceProvider) + .viewReadyStateEvents, postInitializationStep: () async { await widget.postInitializationStep?.call(); diff --git a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/GeckoPlatformView.kt b/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/GeckoPlatformView.kt index a1dd345d..8afda38f 100644 --- a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/GeckoPlatformView.kt +++ b/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/GeckoPlatformView.kt @@ -42,6 +42,32 @@ private class NativeFragmentView( private val container: View + /** + * Reports whether the container is reachable through [Activity.findViewById], which is what + * `GeckoBrowserApiImpl.showFragmentCallback` needs before it can attach the browser fragment. + * + * Hybrid composition (and HC++) only insert the platform view into the Flutter view hierarchy + * the first time its layer is composited — `PlatformViewsController#onDisplayPlatformView` -> + * `initializePlatformViewIfNeeded` -> `flutterView.addView(parentView)`. A widget that lays the + * view out but does not paint it (an `Offstage` ancestor, for instance) therefore keeps the + * container out of the hierarchy indefinitely, and every attach attempt made in the meantime + * fails. + * + * [onFlutterViewAttached] is no signal for this: Flutter calls it while constructing the + * platform view, so reporting readiness from there claims the container is usable long before + * it is. The container's own attach state is the fact that matters, so it is what gets + * reported. See https://github.com/FaFre/WebLibre/issues/557. + */ + private val attachStateListener = object : View.OnAttachStateChangeListener { + override fun onViewAttachedToWindow(v: View) { + flutterEvents.onViewReadyStateChange(EventSequence.next(), true) { _ -> } + } + + override fun onViewDetachedFromWindow(v: View) { + flutterEvents.onViewReadyStateChange(EventSequence.next(), false) { _ -> } + } + } + init { val vParams: ViewGroup.LayoutParams = FrameLayout.LayoutParams( @@ -56,13 +82,13 @@ private class NativeFragmentView( container = BackGestureFilterFrameLayout(activity, activity) container.layoutParams = vParams container.id = containerId + container.addOnAttachStateChangeListener(attachStateListener) } override fun onFlutterViewAttached(flutterView: View) { super.onFlutterViewAttached(flutterView) components.engineReportedInitialized = false - flutterEvents.onViewReadyStateChange(EventSequence.next(), true) { _ -> } } override fun getView(): View { @@ -70,6 +96,11 @@ private class NativeFragmentView( } override fun dispose() { - // Clean up if needed + container.removeOnAttachStateChangeListener(attachStateListener) + + // Removing the listener suppresses the detach callback that tearing the view down would + // otherwise deliver, so report the container gone explicitly. Dart must not keep believing + // an attach is possible against a container that no longer exists. + flutterEvents.onViewReadyStateChange(EventSequence.next(), false) { _ -> } } } \ No newline at end of file diff --git a/packages/flutter_mozilla_components/lib/src/geckoview_widget.dart b/packages/flutter_mozilla_components/lib/src/geckoview_widget.dart index ab635015..64004df5 100644 --- a/packages/flutter_mozilla_components/lib/src/geckoview_widget.dart +++ b/packages/flutter_mozilla_components/lib/src/geckoview_widget.dart @@ -16,12 +16,24 @@ import 'package:flutter/services.dart'; import 'package:flutter_mozilla_components/src/domain/services/gecko_browser.dart'; class GeckoView extends StatefulWidget { - final Future Function()? preInitializationStep; + /// Whether the native container backing this platform view is attached to the + /// window, as reported by `NativeFragmentView`. + /// + /// The browser fragment can only be attached while this holds `true`, and the + /// container is only inserted into the Flutter view hierarchy once the + /// platform-view layer is first composited — which an ancestor that lays the + /// view out without painting it (`Offstage`) defers for as long as it stays + /// offstage. Every attach attempt is therefore driven by this stream rather + /// than by a single burst of retries after creation, which would otherwise + /// expire while the container is still unreachable and never run again. + /// See https://github.com/FaFre/WebLibre/issues/557. + final Stream viewReadyEvents; + final Future Function()? postInitializationStep; const GeckoView({ super.key, - this.preInitializationStep, + required this.viewReadyEvents, this.postInitializationStep, }); @@ -36,17 +48,58 @@ class _GeckoViewState extends State { final browserService = GeckoBrowserService(); late final AppLifecycleListener _listener; + StreamSubscription? _viewReadySubscription; + + /// Serialises attach attempts. + /// + /// The container can be reported attached while an earlier attempt is still + /// retrying, and two concurrent attempts would both find no usable fragment + /// and race to replace each other's. + Future _attachQueue = Future.value(); @override void initState() { super.initState(); _setupMethodCallHandler(); _listener = AppLifecycleListener( - onResume: () async { - //Make sure fragment visible after rsuming the app in case native resources have been disposed - await _showNativeFragment(); + onResume: () { + //Make sure fragment visible after resuming the app in case native resources have been disposed + unawaited(_enqueueShowNativeFragment()); }, ); + + _viewReadySubscription = widget.viewReadyEvents + .where((ready) => ready) + .listen((_) => unawaited(_enqueueShowNativeFragment())); + } + + /// Queues an attach attempt behind any that is still running. + /// + /// Returns when this attempt is done, so callers that need to sequence work + /// after it can await it; failures are contained so one bad attempt cannot + /// poison the queue for the ones the ready stream triggers later. + Future _enqueueShowNativeFragment() { + final attempt = _attachQueue.then((_) async { + if (!mounted) { + return; + } + + try { + await _showNativeFragment(); + } catch (error, stackTrace) { + developer.log( + 'Fragment attach attempt failed', + name: 'GeckoView', + level: 900, + error: error, + stackTrace: stackTrace, + ); + } + }); + + _attachQueue = attempt; + + return attempt; } void _setupMethodCallHandler() { @@ -57,8 +110,15 @@ class _GeckoViewState extends State { }); } + /// Attaches the browser fragment to the native container. + /// + /// The retries cover the transient reasons an attach can fail once the + /// container is reachable — a saved fragment-manager state, a frame in which + /// the fragment's view has no size yet. They deliberately do *not* cover + /// waiting for the container to appear in the first place: that wait is + /// unbounded, and [viewReadyEvents] reports it instead. Future _showNativeFragment({ - int maxRetries = 100, + int maxRetries = 10, /// Default ist about one frame Duration retryDelay = const Duration(milliseconds: 1000 ~/ 60), @@ -90,6 +150,7 @@ class _GeckoViewState extends State { @override void dispose() { platform.setMethodCallHandler(null); + unawaited(_viewReadySubscription?.cancel()); _listener.dispose(); super.dispose(); @@ -118,9 +179,11 @@ class _GeckoViewState extends State { params.onPlatformViewCreated(value); SchedulerBinding.instance.addPostFrameCallback((_) async { - await widget.preInitializationStep?.call(); - - await _showNativeFragment(); + // A first attempt for the common case where the view is painted + // from the frame it is created in, so the container is already + // attached by now. When it is not, this attempt is cheap and the + // ready subscription takes over as soon as it becomes attached. + await _enqueueShowNativeFragment(); await widget.postInitializationStep?.call(); }); })