[Bug] SyncedTabsFeature on TabsTrayFragment leaks memory

[jonalmeida]

No particular STR identified, just opening, closing the tabs tray while browsing.

The 36.5 MB retained is a bit concerning.

Built at 0c76929.

┬───
│ GC Root: System class
│
├─ android.provider.FontsContract class
│    Leaking: NO (DebugFenixApplication↓ is not leaking and a class is never leaking)
│    ↓ static FontsContract.sContext
├─ org.mozilla.fenix.DebugFenixApplication instance
│    Leaking: NO (Application is a singleton)
│    mBase instance of android.app.ContextImpl
│    ↓ FenixApplication.components$delegate
│                       ~~~~~~~~~~~~~~~~~~~
├─ kotlin.SynchronizedLazyImpl instance
│    Leaking: UNKNOWN
│    Retaining 20 B in 1 objects
│    ↓ SynchronizedLazyImpl._value
│                           ~~~~~~
├─ org.mozilla.fenix.components.Components instance
│    Leaking: UNKNOWN
│    Retaining 114.9 kB in 345 objects
│    context instance of org.mozilla.fenix.DebugFenixApplication
│    ↓ Components.services$delegate
│                 ~~~~~~~~~~~~~~~~~
├─ kotlin.SynchronizedLazyImpl instance
│    Leaking: UNKNOWN
│    Retaining 326 B in 16 objects
│    ↓ SynchronizedLazyImpl._value
│                           ~~~~~~
├─ org.mozilla.fenix.components.Services instance
│    Leaking: UNKNOWN
│    Retaining 306 B in 15 objects
│    context instance of org.mozilla.fenix.DebugFenixApplication
│    ↓ Services.accountManager
│               ~~~~~~~~~~~~~~
├─ mozilla.components.service.fxa.manager.FxaAccountManager instance
│    Leaking: UNKNOWN
│    Retaining 5.4 kB in 156 objects
│    context instance of org.mozilla.fenix.DebugFenixApplication
│    ↓ FxaAccountManager.$$delegate_0
│                        ~~~~~~~~~~~~
├─ mozilla.components.support.base.observer.ObserverRegistry instance
│    Leaking: UNKNOWN
│    Retaining 1.6 kB in 49 objects
│    ↓ ObserverRegistry.observers
│                       ~~~~~~~~~
├─ java.util.LinkedHashSet instance
│    Leaking: UNKNOWN
│    Retaining 641 B in 19 objects
│    ↓ LinkedHashSet[element()]
│                   ~~~~~~~~~~~
├─ mozilla.components.feature.syncedtabs.presenter.DefaultPresenter$SyncedTabsAccountObserver instance
│    Leaking: UNKNOWN
│    Retaining 16 B in 1 objects
│    ↓ DefaultPresenter$SyncedTabsAccountObserver.view
│                                                 ~~~~
├─ org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsIntegration instance
│    Leaking: UNKNOWN
│    Retaining 36.5 MB in 18723 objects
│    context instance of org.mozilla.fenix.HomeActivity with mDestroyed = false
│    ↓ SyncedTabsIntegration.syncedTabsFeature$delegate
│                            ~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ kotlin.SynchronizedLazyImpl instance
│    Leaking: UNKNOWN
│    Retaining 36.5 MB in 18169 objects
│    ↓ SynchronizedLazyImpl._value
│                           ~~~~~~
├─ mozilla.components.feature.syncedtabs.SyncedTabsFeature instance
│    Leaking: UNKNOWN
│    Retaining 36.5 MB in 18168 objects
│    ↓ SyncedTabsFeature.presenter
│                        ~~~~~~~~~
├─ mozilla.components.feature.syncedtabs.presenter.DefaultPresenter instance
│    Leaking: UNKNOWN
│    Retaining 36.5 MB in 18166 objects
│    context instance of org.mozilla.fenix.HomeActivity with mDestroyed = false
│    ↓ DefaultPresenter.lifecycleOwner
│                       ~~~~~~~~~~~~~~
╰→ org.mozilla.fenix.tabstray.TabsTrayFragment instance
​     Leaking: YES (ObjectWatcher was watching this because org.mozilla.fenix.tabstray.TabsTrayFragment received
​     Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     Retaining 36.5 MB in 18165 objects
​     key = 87e43fec-16a8-4099-9f22-890451c078c8
​     watchDurationMillis = 72887
​     retainedDurationMillis = 67887

METADATA

Build.VERSION.SDK_INT: 31
Build.MANUFACTURER: Google
LeakCanary version: 2.8.1
App process name: org.mozilla.fenix.debug
Stats: LruCache[maxSize=3000,hits=141576,misses=304778,hitRate=31%]

cc: @MatthewTighe @MozillaNoah

┆Issue is synchronized with this Jira Task

[jonalmeida]

From a 5 minute investigation into this, this doesn't seem like a new bug, but one that was only surfaced after moving the feature to the root of the fragment?

I think we can fix by unregistering the eventObserver and accountObserver here.

[Mugurell]

Indeed this is bad.
Seems like everytime tabs tray was opened we leak the tabs tray view (with 20 tabs on a QHD device I saw a 178MB leak) through FxaAccountManager which is an app level singleton.
So an easy way to to reproduce this: open the tabs tray then navigate to another fragment, even just tap on a tab to be opened in browser.
Unregistering the above observers does indeed fix the issue.

[jonalmeida]

This landed in the last AC bump; closing as fixed.