[webview_flutter_wkwebview] Updates platform views on iOS to only have a weak reference to the native view

[bparrishMines]

Updates PlatformViewImpl to only store a weak reference to the native view. This issue seems to indicate the PlatformViewsController is keeping a reference to the native view even after it is removed from the InstanceManager in webview_flutter_wkwebview. This change should allow the pigeon call to happen when the reference to the Dart instance is deallocated. This should be safe because if the Dart instance is deallocated, then the Widget tree should no longer contain the PlatformView. And therefore the native UIView should no longer need to be used. Nor any callbacks that need the UIView since it is not on screen.

Potential fix for flutter/flutter#168535, but should still update engine to notify plugins that they are detached before setting PlatformViewsController to nil.

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I followed the version and CHANGELOG instructions, using semantic versioning and the repository CHANGELOG style, or I have commented below to indicate which documented exception this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

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

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

[gemini-code-assist]

Code Review

This pull request updates PlatformViewImpl on iOS to hold a weak reference to its underlying UIView, which is intended to prevent a memory leak. A new test, PlatformViewImplTests.swift, is added to verify this behavior. The implementation of PlatformViewImpl's view() method is changed to handle cases where the underlying UIView may have been deallocated. My review includes one suggestion to make this handling more robust by failing explicitly in what should be an unreachable state, rather than returning a new empty view.

[gemini-code-assist]

Returning a new, empty UIView when uiView is nil could lead to subtle bugs and unexpected behavior. For instance, the Flutter engine might add this empty view to the view hierarchy, resulting in a blank space where the webview should be, without any clear error indicating what went wrong.

Given the assumption that view() should not be called after the underlying native view is deallocated, it would be more robust to enforce this with a fatalError. This will help catch incorrect usage during development and provide a clear error message.

    func view() -> UIView {
      guard let uiView = uiView else {
        fatalError("The platform view is being accessed after it has been released.")
      }
      return uiView
    }

[stuartmorgan-g]

This change should allow the pigeon call to happen when the reference to the Dart instance is deallocated.

I'm not sure how this part follows; nothing would prevent the engine from keeping a strong reference to the view returned by view. It seems like this is assuming that the engine will only hold a reference to the platform view container rather than the view itself, but we can't control that.

Could we instead make sure internally that the proxy system can't ever try to send messages after detachFromEngine is called?

[bparrishMines]

@stuartmorgan-g I think the problem is that detachFromEngine hasn't been called in this scenario and that's why it's crashing. webview_flutter_wkwebview already handles preventing callbacks when detachFromEngine is called: https://github.com/flutter/packages/blob/main/packages/webview_flutter/webview_flutter_wkwebview/darwin/webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/WebViewFlutterPlugin.swift#L35. Or at least it should if it works as expected.

As I pointed out in this comment, detachFromEngine isn't called until FlutterEngine.dealloc, but it seems the FlutterBinaryMessenger is invalid before that happens.

Now that I think about it, we should probably create a separate issue if the binaryMessenger is becoming invalid before plugins are notified.

I'm not sure how this part follows; nothing would prevent the engine from keeping a strong reference to the view returned by view. It seems like this is assuming that the engine will only hold a reference to the platform view container rather than the view itself, but we can't control that.

This is true and I considered this as well. Based on the logs, it looks like the container is deallocated before the native UIView, so I assumed this could work as a quick workaround:

21 Flutter                        0x365c0 std::_fl::__tree<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::__map_value_compare<long long, std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::less<long long>, true>, std::_fl::allocator<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>>>::destroy(std::_fl::__tree_node<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, void*>*) + 133 (new.cpp:133)
22 Flutter                        0x365b4 std::_fl::__tree<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::__map_value_compare<long long, std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::less<long long>, true>, std::_fl::allocator<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>>>::destroy(std::_fl::__tree_node<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, void*>*) + 86 (scoped_typeref.h:86)
23 Flutter                        0x365b4 std::_fl::__tree<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::__map_value_compare<long long, std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, std::_fl::less<long long>, true>, std::_fl::allocator<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>>>::destroy(std::_fl::__tree_node<std::_fl::__value_type<long long, fml::scoped_nsobject<NSObject<FlutterPlatformView>>>, void*>*) + 86 (scoped_typeref.h:86)
24 Flutter                        0x19b38 std::_fl::__shared_ptr_pointer<flutter::FlutterPlatformViewsController*, std::_fl::shared_ptr<flutter::FlutterPlatformViewsController>::__shared_ptr_default_delete<flutter::FlutterPlatformViewsController, flutter::FlutterPlatformViewsController>, std::_fl::allocator<flutter::FlutterPlatformViewsController>>::__on_zero_shared() + 1087 (__tree:1087)

But I can probably just close this and work on the fix I talked about in the comment.

[stuartmorgan-g]

As I pointed out in this comment, detachFromEngine isn't called until FlutterEngine.dealloc, but it seems the FlutterBinaryMessenger is invalid before that happens.

Oof. Sorry, I'd paged that conversation out. Yes, the engine internals being torn down before telling the plugins is definitely bad news.

We can give this a try in the short term then, but please add a comment in the code explaining what this code is doing and why, and referencing the issue tracking the engine fix, so that we know when we can remove it again.

I'm still somewhat skeptical that this will actually work reliably in practice since UIViews sometimes have lifetime extensions from the system (e.g., if there's an animation happening around the teardown), but we can give it a shot.