Unregister ZoomLayout's OnGlobalLayoutListener

[dmazzoni]

Environment

Library version: com.otaliastudios:zoomlayout:1.3.0
Android version: Android 9 (API 28), tested on a Google Pixel 2

Issue Description

While testing an application that uses ZoomLayout, a memory leak was reported by LeakCanary. It seems that the leak is caused by the OnGlobalLayoutListener registered in ZoomLayout.addView(), which is never unregistered.

Leak Information

The leak info exported from LeakCanary can be found at square/leakcanary#1126. I had opened that issue because I thought that the leak was caused by the Android SDK and should therefore be added to LeakCanary's exclusion list.

Tests with ZoomLayout

I then re-tested my application with a fork of ZoomLayout in which I modified the OnGlobalLayoutListener to unregister itself the first time it is invoked. No leaks were reported by LeakCanary, and everything in ZoomLayout seemed to be working as expected. Do you think that there could be any negative side effects with that modification?

[natario1]

Yes, we want to keep listening to changes in child, not just the first time.

It's not clear to me where the leak is... I thought ViewTreeObserver would die with the view.
You could unregister the listener during removeView, but I don't know if that's called in your case. Is it?

[dmazzoni]

Thanks for your feedback!
No, in my case removeView is not called, so I can't unregister the listener there.

[dmazzoni]

Anyway, regardless of its cause, the leak may be prevented by turning the anonymous layout listener into a static nested class which holds weak references to mEngine, mChildRect and the view being added.

[markusressel]

@dmazzoni Since you have already done a lot of work on this are you interested in opening a PR for this?

[dmazzoni]

Yes, I'll probably be able to open it tomorrow