1238066 - Add LeakCanary to local builds

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

LeakCanary is a memory leak detection library for Android and Java:
https://github.com/square/leakcanary

We just want to use this in local builds (maybe in Nightly?).

Square offers two libraries: The actual lib and a no-op lib that can be used to compile release versions without the need to change the code using LeakCanary:
* com.squareup.leakcanary:leakcanary-android:1.3.1
* com.squareup.leakcanary:leakcanary-android-no-op:1.3.1

By default LeakCanary will detect Activity leaks. For everything else we'll need to manually add references to RefWatcher if they are interesting to monitor.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

I recently did this locally (just in the gradle build because that's easy) and it worked without issues (I didn't find any leaks though).

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Sebastian Kaspari (:sebastian) from comment #1)
> I recently did this locally (just in the gradle build because that's easy)
> and it worked without issues (I didn't find any leaks though).

Somebody (mcomella?  mhaigh?) did this before.  Post the patch and let's get it into Gradle, at least.  We can try to get the no-op thing working in moz.build if it's useful.

Comment 3

[:Margaret Leibovic]

(In reply to Nick Alexander :nalexander from comment #2)
> (In reply to Sebastian Kaspari (:sebastian) from comment #1)
> > I recently did this locally (just in the gradle build because that's easy)
> > and it worked without issues (I didn't find any leaks though).
> 
> Somebody (mcomella?  mhaigh?) did this before.  Post the patch and let's get
> it into Gradle, at least.  We can try to get the no-op thing working in
> moz.build if it's useful.

I had a gist that did this long ago:
https://gist.github.com/leibovic/38c21068e4ac58e5c751

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1238066 - Add LeakCanary to local (gradle) builds. r?nalexander

This patch adds LeakCanary to gradle builds and the no-op library to
mach-based builds.

Review commit: https://reviewboard.mozilla.org/r/31731/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31731/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

I tested this patch with a manually created leak.

I mean eventually we could add LeakCanary to mach based builds too (There are a bunch of classes though..) and hide it behind a build flag.

By default LeakCanary only detects Activity leaks. If we want to use it for more then we should expose the RefWatcher instance like described here:
https://github.com/square/leakcanary#how-do-i-use-it

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba4d8b18ee51

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8710373 [details]
MozReview Request: Bug 1238066 - Add LeakCanary to local (gradle) builds. r?nalexander

https://reviewboard.mozilla.org/r/31731/#review28719

I'd like to see a green try build, but other than that, rock on!

I will have to rebase a bunch of Gradle work on top of this, but whatever :)

::: mobile/android/app/base/build.gradle:121
(Diff revision 1)
> +    // Gradle based debug builds include LeakCanary. Mach based builds only include the no-op

It's not quite true that it's debug only.  As written, it's *all* Gradle builds.  (Unless LeakCanary itself differentiates, which it can and might.)  Let's do not be clever about debug/release in our build.gradle files for now, since it's easier.

::: mobile/android/config/proguard/leakcanary-keeps.cfg:6
(Diff revision 1)
> +# For Android 6.0 compatibility

nit: full sentences with periods, please.

::: mobile/android/thirdparty/com/squareup/leakcanary/LeakCanary.java:1
(Diff revision 1)
> +package com.squareup.leakcanary;

nit: Include license.  I think this should be MPL, since it's our version.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e22ca2de5f9

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/2ff2a6523bc06d93a5d55d848c8638296f0f574f
Bug 1238066 - Add LeakCanary to local (gradle) builds. r=nalexander

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2ff2a6523bc0