getAdapter() in CategoryImagesListFragment now returns the instance o…

[ashishkumar468]

…f the adapter instead of the gridview itself [Issue#1722]

Memory leak in CategoryDetailsActivity

Fixes #1722 [1.5 MB memory leak in CategoryDetailsActivity]

Description (required)

Fixes #1722 [1.5 MB memory leak in CategoryDetailsActivity]

In order to access the media displayed in CategoryImagesListFragment, the getAdapter() function in CategoryImagesListFragment used to return gridView.getAdapter(). I have replaced that with return gridAdapter.Holding a reference of a ui element for accessing a dao object has never been recommended. Hope this solves the leak to some extent.

Tests performed (required)

Tested on {27 & google pixel emulator}, with {build variant, betaDebug}.

Screenshots showing what changed (optional)

NA
Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

[codecov-io]

Codecov Report

Merging #1792 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1792      +/-   ##
=========================================
+ Coverage    3.56%   3.57%   +<.01%     
=========================================
  Files         186     190       +4     
  Lines        9329    9501     +172     
  Branches      811     839      +28     
=========================================
+ Hits          333     340       +7     
- Misses       8974    9138     +164     
- Partials       22      23       +1

Impacted Files	Coverage Δ
...w/commons/category/CategoryImagesListFragment.java	0% <0%> (ø)	⬆️
.../java/fr/free/nrw/commons/auth/SessionManager.java	10.71% <0%> (-4.68%)	⬇️
.../commons/contributions/ContributionController.java	0% <0%> (ø)	⬆️
...free/nrw/commons/upload/MultipleShareActivity.java	0% <0%> (ø)	⬆️
.../nrw/commons/category/CategoryContentProvider.java	10.71% <0%> (ø)	⬆️
...ree/nrw/commons/auth/WikiAccountAuthenticator.java	0% <0%> (ø)	⬆️
...ain/java/fr/free/nrw/commons/upload/FileUtils.java	3.22% <0%> (ø)	⬆️
...ain/java/fr/free/nrw/commons/auth/AccountUtil.java	0% <0%> (ø)	⬆️
...ns/modifications/ModificationsContentProvider.java	10% <0%> (ø)	⬆️
...a/fr/free/nrw/commons/upload/UploadController.java	0% <0%> (ø)	⬆️
... and 18 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12a83da...1f69c9d. Read the comment docs.

[ashishkumar468]

@neslihanturan @maskaravivek Hi guys, can you please review it.

[maskaravivek]

@ashishkumar468 Can you use leak canary to verify if your change has actually fixed the leak or not. All you would need to do is:

• first, reproduce the issue using a build without your fix. Simply using CategoryDetailsActivity a few times should cause this leak and leak canary will automatically detect it.
• then use the build with your fix. Leak canary should no longer be able to detect this leak. :)

[ashishkumar468]

@maskaravivek Sure, will update once done

[neslihanturan]

@ashishkumar468 did you have time to test with leak canary?

[ashishkumar468]

@neslihanturan Sure I will do it today. Sorry for the delay.

[neslihanturan]

Up:)

[ashishkumar468]

Hi @neslihanturan I actually tried to verify the leak by visiting CategoryDetailsActivity multiple times, the build created using this pull request did not actually lead to leaks. It will be great if someone else also verifies

[domdomegg]

Rebasing...

[domdomegg]

I could not reproduce the memory leak in 24a76b1 so cannot say for sure it is gone. However, having read through CategoriesImageListFragment I think this does improve the code, so recommend merging it.

The null checks were added by in #1918 by @henrique-gb. Would likely be useful to get your input on this but I think this should avoid NPEs.

[domdomegg]

Tested 2.9.0-debug-feature/bug_fix#1722~58fe52d00 on Galaxy Nexus (emulator) at API level 28

Automated tests pass