Skip to content

[plugins] Add engine retain cycle tests to all plugins #103914

@stuartmorgan-g

Description

@stuartmorgan-g

We've had bugs in a number of plugins where just setting them up creates a retain cycle that keeps the engine alive forever (e.g., due to engine foot-guns like #102990).

We should have a test in every plugin that calls registerWithRegistrar with a fake registrar that holds a strong reference to it when a channel handler is added, and also that returns itself as any of the called helper objects (binary messenger, texture registrar, etc.—using a strict mock to make sure we aren't missing any and accidentally returning nil). The test should drop its reference to the fake registrar, and then ensure that it nils out.

This would be more strict than the engine actually is (e.g., the binary messenger won't make a loop any more), but it should probably never actually be necessary to have a strong reference to something that's owned by the engine anyway, because if the engine is gone calling it is at best a no-op anyway (and in some cases an error).

This would ensure we aren't relying on add-to-app customers to find these one at a time, and also be a good backstop against accidentally introducing retain loop foot-guns with any new engine APIs.

/cc @jmagman @cyanglaz

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work lista: tests"flutter test", flutter_test, or one of our testsc: contributor-productivityTeam-specific productivity, code health, technical debt.packageflutter/packages repository. See also p: labels.platform-iosiOS applications specificallyteam-iosOwned by iOS platform teamtriaged-iosTriaged by iOS platform team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions