VerticalGridView leaking views in various Leanback Fragments [171909417]

Fixed

Bug

Status Update

No update yet.

Description

mc...@gmail.com

created issue #1

Oct 28, 2020 08:24PM

Component used: androidx.leanback:leanback:1.1.0-alpha05

RowsSupportFragment will be used in the following example, but the issue is also present in other fragments.

App uses single activity architecture with Androidx Navigation component. RowsSupportFragment is declared in the fragment layout xml via a FragmentContainerView using the android:name attribute.
When navigating away and the current fragment is placed in the backstack, the RowsSupportFragment instance is retained and re-used, and leaks the VerticalGridView, and by extension the entire fragment view hierarchy, that should have been destroyed.

When the adapter is set on the RowsSupportFragment, the VerticalGridView's adapter is also set via mVerticalGridView.setAdapter(mBridgeAdapter), which registers the view as an observer inside mBridgeAdapter. In BaseRowSupportFragment's onDestroyView, the mVerticalGridView reference is nulled. However, this is insufficient as mBridgeAdapter still holds a reference to the view as an observer. The VerticalGridView's adapter should also be nulled to clear this reference.

A workaround is by overriding RowsSupportFragment and manually nulling the VerticalGridView's adapter in onDestroyView. But this does not work for something like VerticalGridSupportFragment which does not expose the internal VerticalGridView.

Comments

da...@google.com <da...@google.com> Nov 19, 2020 10:51PM

Accepted by da...@google.com.

ap...@google.com <ap...@google.com> #2Nov 20, 2020 03:50AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 29c24dd640943772ffaadc1635f0350f16557cbe
Author: Dake Gu <dake@google.com>
Date: Thu Nov 19 14:46:43 2020

leanback: fix View leak when pushing RowsSupportFragment to backstack

There are two leaks when RowsSupportFragment is pushed to backstack:

1. RowsSupportFragment holds mSelectedViewHolder after onDestroyView()
2. RowsSupportFragment.mBridgeAdapter registered VerticalGridView as
observer.

Bug: 171909417
Test: verified by leak tool on a sample app pushing RowsSupportFragment
to backstack

Change-Id: I985d4b09ef8d137f6f21590fdef615d7dca322cd

M leanback/leanback/src/main/java/androidx/leanback/app/BaseRowFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BaseRowSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BrowseFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BrowseSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/RowsFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/RowsSupportFragment.java

https://android-review.googlesource.com/1506430

ap...@google.com <ap...@google.com> #3Nov 21, 2020 12:10AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 2f2fa284ef504e466c21b91820b7bb3a18ddea30
Author: Dake Gu <dake@google.com>
Date: Fri Nov 20 12:19:47 2020

leanback: fix View leak of DetailsSupportFragment

Fixed leak in:
1. BrandedSupportFragment holds titleView
2. ActivityTransition listener holds refrence to DetailsSupportFragment
3. DetailsOverviewSharedElementHelper and
FullWidthOverviewSharedElementHelper hold reference to ViewHolder
4. DetailsOverviewRow holds reference to actions

Bug: 171909417
Test: tested with heapdump leanback-demos DetailsSupportFragment and
NewDetailsSupportFragment.

Change-Id: Ifec9e735bddd83831a07119c26c1e80c71cd0a6d

M leanback/leanback/src/main/java/androidx/leanback/app/BrandedFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BrandedSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/DetailsFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/DetailsSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/GuidedStepFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/PlaybackFragment.java
M leanback/leanback/src/main/java/androidx/leanback/widget/DetailsOverviewRowPresenter.java
M leanback/leanback/src/main/java/androidx/leanback/widget/DetailsOverviewSharedElementHelper.java
M leanback/leanback/src/main/java/androidx/leanback/widget/FullWidthDetailsOverviewRowPresenter.java
M leanback/leanback/src/main/java/androidx/leanback/widget/FullWidthDetailsOverviewSharedElementHelper.java

https://android-review.googlesource.com/1506488

ap...@google.com <ap...@google.com> #4Nov 23, 2020 06:32PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit e3cd7798ee735eb7cdc4f3001ea8d13c12330cff
Author: Dake Gu <dake@google.com>
Date: Fri Nov 20 16:09:59 2020

leanback: add RowsSupportFragment view leak test

Test: RowsSupportFragmentTest#viewLeakTest
Bug: 171909417
Change-Id: I770c57a188c3f0c559a61edca6b37499d469efd0

M leanback/leanback/src/androidTest/java/androidx/leanback/app/DetailsFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/app/RowsFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/app/RowsSupportFragmentTest.java
A leanback/leanback/src/androidTest/java/androidx/leanback/testutils/LeakDetector.java

https://android-review.googlesource.com/1507839

ap...@google.com <ap...@google.com> #5Nov 23, 2020 07:15PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 639e85d4c0a1e7b125f5d70608c702901cb75dcc
Author: Dake Gu <dake@google.com>
Date: Mon Nov 23 09:59:38 2020

leanback: add DetailsSupportFragment viewLeakTest

Test: DetailsSupportFragmentTest#viewLeakTest
Bug: 171909417
Change-Id: I99abdb3b512c7156d40ecdc0463284155fedfea8

M leanback/leanback/src/androidTest/java/androidx/leanback/app/DetailsFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/app/DetailsSupportFragmentTest.java

https://android-review.googlesource.com/1507126

ap...@google.com <ap...@google.com> #6Nov 24, 2020 07:26PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 78e0f950b943b3b5c24e16ca12ae152a82f595ff
Author: Dake Gu <dake@google.com>
Date: Mon Nov 23 19:12:04 2020

leanback: fix BrowseSupportFragment view leak

Fix leaking in onDestroyView, ProgressBarManager

Bug: 171909417
Test: BrowseSupportFragmentTest#viewLeakTest
Change-Id: I344835e2d429a7bacd2416077f054fee20f59dcb

M leanback/leanback/src/androidTest/java/androidx/leanback/app/BrowseFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/app/BrowseSupportFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/testutils/LeakDetector.java
M leanback/leanback/src/main/java/androidx/leanback/app/BaseFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BaseSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BrowseFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/BrowseSupportFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/ProgressBarManager.java

https://android-review.googlesource.com/1509370

ap...@google.com <ap...@google.com> #7Nov 25, 2020 12:06AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 6c3af0f344dc262b50e3c5fd2621f01109daab32
Author: Dake Gu <dake@google.com>
Date: Tue Nov 24 13:02:27 2020

leanback: fix View leak of VerticalGridSupportFragment

Test: VerticalGridSupportFragmentTest#viewLeakTest
Bug: 171909417
Change-Id: Iaac695b36b0c2e1bddddb2e496ad0fc5038d8e1e

M leanback/leanback/src/androidTest/java/androidx/leanback/app/VerticalGridFragmentTest.java
M leanback/leanback/src/androidTest/java/androidx/leanback/app/VerticalGridSupportFragmentTest.java
M leanback/leanback/src/main/java/androidx/leanback/app/VerticalGridFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/VerticalGridSupportFragment.java

https://android-review.googlesource.com/1510180

ap...@google.com <ap...@google.com> #8Dec 3, 2020 07:33PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 6c47a1e08882d4932ace063ff2d1ed11f0fe37da
Author: Dake Gu <dake@google.com>
Date: Tue Nov 24 12:09:31 2020

leanback: fix View leak of SearchSupportFragment

Test: SearchSupportFragmentTest#viewLeakTest
Bug: 171909417
Change-Id: Iabe6ba6981650567479534fa17e9b6e33b6c7824

M leanback/leanback/api/1.1.0-beta01.txt
M leanback/leanback/api/current.txt
M leanback/leanback/api/public_plus_experimental_1.1.0-beta01.txt
M leanback/leanback/api/public_plus_experimental_current.txt
M leanback/leanback/api/restricted_1.1.0-beta01.txt
M leanback/leanback/api/restricted_current.txt
M leanback/leanback/src/androidTest/generatev4.py
A leanback/leanback/src/androidTest/java/androidx/leanback/app/SearchFragmentTest.java
A leanback/leanback/src/androidTest/java/androidx/leanback/app/SearchSupportFragmentTest.java
M leanback/leanback/src/main/java/androidx/leanback/app/SearchFragment.java
M leanback/leanback/src/main/java/androidx/leanback/app/SearchSupportFragment.java

https://android-review.googlesource.com/1510177

da...@google.com <da...@google.com> Dec 3, 2020 09:28PM

Marked as fixed.

ma...@gmail.com <ma...@gmail.com> #9Dec 12, 2020 01:31AM

Add an optional extended distribution...

th...@gmail.com <th...@gmail.com> #10May 9, 2021 04:44PM

Unfortunately one of the commits to fix a leak broke the DetailsSupportFragmentBackgroundController class.

See for more information: https://issuetracker.google.com/issues/187577716