fix gecko view initialization issues

2026-08-09 03:17:24 +02:00
parent 00e8ae7ff0
commit d7055a6f44
3 changed files with 114 additions and 26 deletions
@@ -281,21 +281,15 @@ class _BrowserViewState extends ConsumerState<BrowserView>
            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();

@@ -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) { _ -> }
    }
 }
@@ -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<void> 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<bool> viewReadyEvents;
+
  final Future<void> Function()? postInitializationStep;

  const GeckoView({
    super.key,
-    this.preInitializationStep,
+    required this.viewReadyEvents,
    this.postInitializationStep,
  });

@@ -36,17 +48,58 @@ class _GeckoViewState extends State<GeckoView> {

  final browserService = GeckoBrowserService();
  late final AppLifecycleListener _listener;
+  StreamSubscription<bool>? _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<void> _attachQueue = Future<void>.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<void> _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<GeckoView> {
    });
  }

+  /// 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<bool> _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<GeckoView> {
  @override
  void dispose() {
    platform.setMethodCallHandler(null);
+    unawaited(_viewReadySubscription?.cancel());
    _listener.dispose();

    super.dispose();
@@ -118,9 +179,11 @@ class _GeckoViewState extends State<GeckoView> {
            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();
            });
          })