Actually looking at these 2 classes in source again, I think this might still not be "safe".

The TextInputChannel has a private variable parsingMethodHandler to the non-static inner class MethodChannel.MethodCallHandler which retains the whole TextInputChannel class instance. When we made the outer class, it gave it to the channel method call handler so that's still stored in DartMessenger's messageHandlers forever. Next time we add a variable to the outer class TextInputChannel, we'll leak it again.

Since TextInputChannel is owned by TextInputPlugin and TextInputPlugin already has a strong semantic signal (destroy()), let's add a destroy() to TextInputChannel that unsets both the textInputMethodHandler and sets the "flutter/textinput" channel method call handler to null.

Otherwise, as it stands, we're turning a binary messenger -> TextInputChannel -> TextInputPlugin leak into a binary messenger -> TextInputChannel leak but we really should get rid of both.

This would make your testing a bit easier too. We're not so concerned about whether TextInputChannel and TextInputPlugin references each other as they both become orphaned. That's an implementation detail. We should just make sure that the long lived dart executor / binary messenger doesn't hold anything after TextInputPlugin.destroy(). e.g. hold onto the mock(DartExecutor.class) that you injected into TextInputPlugin and after the destroy, assert that a setMessageHandler("flutter/textinput", null) was called.

This isn't really a leak.

The FlutterEngine creates instances of all of the embedder's built-in channels at startup. The channels that receive incoming messages then register their message handlers with the DartMessenger. If the channel needs to forward incoming messages to another component, then the message handler will be something like the TextInputChannel.parsingMessageHandler that parses the message and then delegates to a client interface.

TextInputChannel is special in that it is created both at FlutterEngine startup (https://github.com/flutter/engine/blob/master/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java#L225) and then recreated when the FlutterView creates a TextInputPlugin instance (https://github.com/flutter/engine/blob/master/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java#L53)

The new TextInputChannel instance will replace the old flutter/textinput handler in the DartMessenger with its instance of parsingMessageHandler. So the DartMessenger is no longer holding a reference to the old handler, and there is no leak.

However, it does seem inconsistent that TextInputChannel is being created and held by the FlutterEngine while a second instance created by the TextInputPlugin is the one that actually receives messages.

I think the solution is to have TextInputPlugin's constructor take the FlutterEngine's TextInputChannel as an argument. The TextInputPlugin lifecycle is already tied to the FlutterView attaching and detaching to a FlutterEngine.

PlatformPlugin currently seems to be doing something like this (https://github.com/flutter/engine/blob/master/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java#L90)

What you're proposing sounds ok too (and probably less cognitive load since that pattern is used elsewhere for the other channels). My worry with the present pattern is that it looks like the TextInputPlugin owns this TextInputChannel (since it made it). For someone adding a new feature, it might look less wrong to add another reference to something from TextInputPlugin from the TextInputChannel outer class because it looks like they have the same lifecycle, but that leaks the TextInputPlugin because despite appearances, the TextInputChannel is long lived.

If it doesn't look like the TextInputPlugin owns the TextInputChannel like you're suggesting, that error is less likely.