[Android] Fix the issue of blank or frozen pages in shared engine scenarios

[0xZOne]

Consider this scenario: In an add-to-app context, where multiple Flutter activities share the same engine, a situation occurs. When navigating from FlutterActivity1 to FlutterActivity2, the Flutter view associated with FlutterActivity1 is detached from the engine. Then, the Flutter view of FlutterActivity2 is attached. Upon navigating back to FlutterActivity1, its Flutter view is re-attached to the shared engine.

The expected behavior is: When a Flutter view detaches from the shared engine, the associated surface should be released. When the Flutter view re-attaches, a new surface should be created.

After #47358, no new surface is created when the Flutter view is attached again. This results in the Flutter view having no underlying surface, which causes the page to appear blank or freeze without responding.

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• [ ] I listed at least one issue that this PR fixes in the description above.
• [x] I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I signed the CLA.
• [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard[bot]]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #50947 at sha c74ff37d1f5fad2702dadccee90bb52bdede2d7a

[0xZOne]

The expected behavior is: When a Flutter view detaches from the shared engine, the associated surface should be released. When the Flutter view re-attaches, a new surface should be created. After #47358, no new surface is created when the Flutter view is attached again. This results in the Flutter view having no underlying surface, which causes the page to appear blank or freeze without responding.

Given your description, I would expect you to fix the bug that no new surface is created when the view is attached again. It looks like you've just removed calls to pause() which isn't addressing the root problem.

It works fine after removing the call to pause() in the detachFromRenderer function. Calling pause() sets isPaused to true, which prevents startRenderingToSurface from creating a new surface.

[johnmccutchan]

The expected behavior is: When a Flutter view detaches from the shared engine, the associated surface should be released. When the Flutter view re-attaches, a new surface should be created. After #47358, no new surface is created when the Flutter view is attached again. This results in the Flutter view having no underlying surface, which causes the page to appear blank or freeze without responding.

Given your description, I would expect you to fix the bug that no new surface is created when the view is attached again. It looks like you've just removed calls to pause() which isn't addressing the root problem.

It works fine after removing the call to pause() in the detachFromRenderer function. Calling pause() sets isPaused to true, which prevents startRenderingToSurface from creating a new surface.

Why can't you just move the isPaused = false; so it executes before connectSurfaceToRenderer is called?

[0xZOne]

Why can't you just move the isPaused = false; so it executes before connectSurfaceToRenderer is called?

On one hand, I believe the call to pause() in detachFromRenderer should be removed.

As stated in the comments of the detachFromRenderer function, if the detachFromRenderer function is called, it indicates that the RenderSurface is no longer needed and its underlying surface will also be detached.

  /**
   * Invoked by the owner of this {@code FlutterSurfaceView} when it no longer wants to render a
   * Flutter UI to this {@code FlutterSurfaceView}.
   *
   * <p>This method will cease any on-going rendering from Flutter to this {@code
   * FlutterSurfaceView}.
   */
  public void detachFromRenderer() {
    ...
  }

Calling pause() in detachFromRenderer() is superfluous and contributes nothing to the problem that https://github.com/flutter/engine/pull/47358 aims to solve. Moreover, it results in the required new surface not being created. Therefore, the call to pause() in detachFromRenderer should be removed.

On the other hand, if isPaused = false; is moved to execute before the call to connectSurfaceToRenderer, it would result in #47358 losing its effect. That is to say, a new surface would be recreated upon returning from HC mode.

  public void resume() {
    if (flutterRenderer == null) {
      Log.w(TAG, "resume() invoked when no FlutterRenderer was attached.");
      return;
    }
    this.flutterRenderer.addIsDisplayingFlutterUiListener(flutterUiDisplayListener);

    // If we're already attached to an Android window then we're now attached to both a renderer
    // and the Android window. We can begin rendering now.
    if (isSurfaceAvailableForRendering()) {
      Log.v(
          TAG,
          "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
      connectSurfaceToRenderer();
    }
    isPaused = false;
  }

[johnmccutchan]

Thanks for your patience and explanations. I believe you're correct and will land this PR soon

[flutteractionsbot]

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.