New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move Notifications to get triggered from NotificationEventListener #2395
Changes from all commits
8efb01b
d097654
6cc7765
cea3972
67a5471
6163083
50cede4
488efdd
463c4fa
49e561b
2362723
7f92cb9
2a065d3
2089175
aee2a2a
2e2dc04
a8b2698
b841f1f
8e7f940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,25 @@ | ||
package leakcanary.internal | ||
|
||
import android.app.Application | ||
import android.app.Notification | ||
import android.app.NotificationManager | ||
import android.content.Context | ||
import android.content.res.Resources.NotFoundException | ||
import android.os.Handler | ||
import android.os.SystemClock | ||
import com.squareup.leakcanary.core.R | ||
import java.util.UUID | ||
import leakcanary.AppWatcher | ||
import leakcanary.EventListener | ||
import leakcanary.EventListener.Event.DismissNoRetainedOnTapNotification | ||
import leakcanary.EventListener.Event.DumpingHeap | ||
import leakcanary.EventListener.Event.HeapDump | ||
import leakcanary.EventListener.Event.HeapDumpFailed | ||
import leakcanary.EventListener.Event.ShowNoMoreRetainedObjectFoundNotification | ||
import leakcanary.GcTrigger | ||
import leakcanary.KeyedWeakReference | ||
import leakcanary.LeakCanary.Config | ||
import leakcanary.ObjectWatcher | ||
import leakcanary.internal.HeapDumpControl.ICanHazHeap.Nope | ||
import leakcanary.internal.HeapDumpControl.ICanHazHeap.NotifyingNope | ||
import leakcanary.internal.InternalLeakCanary.onRetainInstanceListener | ||
import leakcanary.internal.NotificationReceiver.Action.CANCEL_NOTIFICATION | ||
import leakcanary.internal.NotificationReceiver.Action.DUMP_HEAP | ||
import leakcanary.internal.NotificationType.LEAKCANARY_LOW | ||
import leakcanary.internal.RetainInstanceEvent.CountChanged.BelowThreshold | ||
import leakcanary.internal.RetainInstanceEvent.CountChanged.DumpHappenedRecently | ||
import leakcanary.internal.RetainInstanceEvent.CountChanged.DumpingDisabled | ||
|
@@ -39,10 +36,6 @@ internal class HeapDumpTrigger( | |
private val configProvider: () -> Config | ||
) { | ||
|
||
private val notificationManager | ||
get() = | ||
application.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | ||
|
||
private val applicationVisible | ||
get() = applicationInvisibleAt == -1L | ||
|
||
|
@@ -53,14 +46,6 @@ internal class HeapDumpTrigger( | |
|
||
private var lastHeapDumpUptimeMillis = 0L | ||
|
||
private val scheduleDismissRetainedCountNotification = { | ||
dismissRetainedCountNotification() | ||
} | ||
|
||
private val scheduleDismissNoRetainedOnTapNotification = { | ||
dismissNoRetainedOnTapNotification() | ||
} | ||
|
||
/** | ||
* When the app becomes invisible, we don't dump the heap immediately. Instead we wait in case | ||
* the app came back to the foreground, but also to wait for new leaks that typically occur on | ||
|
@@ -78,7 +63,9 @@ internal class HeapDumpTrigger( | |
// Needs to be lazy because on Android 16, UUID.randomUUID().toString() will trigger a disk read | ||
// violation by calling RandomBitsSupplier.getUnixDeviceRandom() | ||
// Can't be lazy because this is a var. | ||
private var currentEventUniqueId: String? = null | ||
private val currentEventUniqueId: String by lazy { | ||
UUID.randomUUID().toString() | ||
} | ||
|
||
fun onApplicationVisibilityChanged(applicationVisible: Boolean) { | ||
if (applicationVisible) { | ||
|
@@ -154,7 +141,6 @@ internal class HeapDumpTrigger( | |
return | ||
} | ||
|
||
dismissRetainedCountNotification() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this has been replaced by new behavior in the event listener class, as I'm seeing no direct call to |
||
val visibility = if (applicationVisible) "visible" else "not visible" | ||
dumpHeap( | ||
retainedReferenceCount = retainedReferenceCount, | ||
|
@@ -173,11 +159,8 @@ internal class HeapDumpTrigger( | |
val heapDumpFile = directoryProvider.newHeapDumpFile() | ||
|
||
val durationMillis: Long | ||
if (currentEventUniqueId == null) { | ||
currentEventUniqueId = UUID.randomUUID().toString() | ||
} | ||
try { | ||
InternalLeakCanary.sendEvent(DumpingHeap(currentEventUniqueId!!)) | ||
InternalLeakCanary.sendEvent(DumpingHeap(currentEventUniqueId)) | ||
if (heapDumpFile == null) { | ||
throw RuntimeException("Could not create heap dump file") | ||
} | ||
|
@@ -193,10 +176,9 @@ internal class HeapDumpTrigger( | |
lastDisplayedRetainedObjectCount = 0 | ||
lastHeapDumpUptimeMillis = SystemClock.uptimeMillis() | ||
objectWatcher.clearObjectsWatchedBefore(heapDumpUptimeMillis) | ||
currentEventUniqueId = UUID.randomUUID().toString() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
InternalLeakCanary.sendEvent(HeapDump(currentEventUniqueId!!, heapDumpFile, durationMillis, reason)) | ||
InternalLeakCanary.sendEvent(HeapDump(currentEventUniqueId, heapDumpFile, durationMillis, reason)) | ||
} catch (throwable: Throwable) { | ||
InternalLeakCanary.sendEvent(HeapDumpFailed(currentEventUniqueId!!, throwable, retry)) | ||
InternalLeakCanary.sendEvent(HeapDumpFailed(currentEventUniqueId, throwable, retry)) | ||
if (retry) { | ||
scheduleRetainedObjectCheck( | ||
delayMillis = WAIT_AFTER_DUMP_FAILED_MILLIS | ||
|
@@ -237,32 +219,13 @@ internal class HeapDumpTrigger( | |
|
||
fun onDumpHeapReceived(forceDump: Boolean) { | ||
backgroundHandler.post { | ||
dismissNoRetainedOnTapNotification() | ||
InternalLeakCanary.sendEvent(DismissNoRetainedOnTapNotification(currentEventUniqueId)) | ||
gcTrigger.runGc() | ||
val retainedReferenceCount = objectWatcher.retainedObjectCount | ||
if (!forceDump && retainedReferenceCount == 0) { | ||
SharkLog.d { "Ignoring user request to dump heap: no retained objects remaining after GC" } | ||
@Suppress("DEPRECATION") | ||
val builder = Notification.Builder(application) | ||
.setContentTitle( | ||
application.getString(R.string.leak_canary_notification_no_retained_object_title) | ||
) | ||
.setContentText( | ||
application.getString( | ||
R.string.leak_canary_notification_no_retained_object_content | ||
) | ||
) | ||
.setAutoCancel(true) | ||
.setContentIntent(NotificationReceiver.pendingIntent(application, CANCEL_NOTIFICATION)) | ||
val notification = | ||
Notifications.buildNotification(application, builder, LEAKCANARY_LOW) | ||
notificationManager.notify( | ||
R.id.leak_canary_notification_no_retained_object_on_tap, notification | ||
) | ||
backgroundHandler.postDelayed( | ||
scheduleDismissNoRetainedOnTapNotification, | ||
DISMISS_NO_RETAINED_OBJECT_NOTIFICATION_MILLIS | ||
) | ||
InternalLeakCanary.sendEvent(ShowNoMoreRetainedObjectFoundNotification(currentEventUniqueId)) | ||
|
||
lastDisplayedRetainedObjectCount = 0 | ||
return@post | ||
} | ||
|
@@ -359,64 +322,25 @@ internal class HeapDumpTrigger( | |
} | ||
|
||
private fun showNoMoreRetainedObjectNotification() { | ||
backgroundHandler.removeCallbacks(scheduleDismissRetainedCountNotification) | ||
if (!Notifications.canShowNotification) { | ||
return | ||
} | ||
val builder = Notification.Builder(application) | ||
.setContentTitle( | ||
application.getString(R.string.leak_canary_notification_no_retained_object_title) | ||
) | ||
.setContentText( | ||
application.getString( | ||
R.string.leak_canary_notification_no_retained_object_content | ||
) | ||
) | ||
.setAutoCancel(true) | ||
.setContentIntent(NotificationReceiver.pendingIntent(application, CANCEL_NOTIFICATION)) | ||
val notification = | ||
Notifications.buildNotification(application, builder, LEAKCANARY_LOW) | ||
notificationManager.notify(R.id.leak_canary_notification_retained_objects, notification) | ||
backgroundHandler.postDelayed( | ||
scheduleDismissRetainedCountNotification, DISMISS_NO_RETAINED_OBJECT_NOTIFICATION_MILLIS | ||
) | ||
InternalLeakCanary.sendEvent(ShowNoMoreRetainedObjectFoundNotification(currentEventUniqueId)) | ||
} | ||
|
||
private fun showRetainedCountNotification( | ||
objectCount: Int, | ||
contentText: String | ||
) { | ||
backgroundHandler.removeCallbacks(scheduleDismissRetainedCountNotification) | ||
if (!Notifications.canShowNotification) { | ||
return | ||
} | ||
@Suppress("DEPRECATION") | ||
val builder = Notification.Builder(application) | ||
.setContentTitle( | ||
application.getString(R.string.leak_canary_notification_retained_title, objectCount) | ||
InternalLeakCanary.sendEvent( | ||
EventListener.Event.ShowRetainedCountNotification( | ||
uniqueId = currentEventUniqueId, | ||
objectCount = objectCount, | ||
contentText = contentText | ||
) | ||
.setContentText(contentText) | ||
.setAutoCancel(true) | ||
.setContentIntent(NotificationReceiver.pendingIntent(application, DUMP_HEAP)) | ||
val notification = | ||
Notifications.buildNotification(application, builder, LEAKCANARY_LOW) | ||
notificationManager.notify(R.id.leak_canary_notification_retained_objects, notification) | ||
} | ||
|
||
private fun dismissRetainedCountNotification() { | ||
backgroundHandler.removeCallbacks(scheduleDismissRetainedCountNotification) | ||
notificationManager.cancel(R.id.leak_canary_notification_retained_objects) | ||
} | ||
|
||
private fun dismissNoRetainedOnTapNotification() { | ||
backgroundHandler.removeCallbacks(scheduleDismissNoRetainedOnTapNotification) | ||
notificationManager.cancel(R.id.leak_canary_notification_no_retained_object_on_tap) | ||
) | ||
} | ||
|
||
companion object { | ||
internal const val WAIT_AFTER_DUMP_FAILED_MILLIS = 5_000L | ||
private const val WAIT_FOR_OBJECT_THRESHOLD_MILLIS = 2_000L | ||
private const val DISMISS_NO_RETAINED_OBJECT_NOTIFICATION_MILLIS = 30_000L | ||
private const val WAIT_BETWEEN_HEAP_DUMPS_MILLIS = 60_000L | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,13 +113,13 @@ class HprofRetainedHeapPerfTest { | |
} | ||
|
||
assertThat(retained after PARSING_HEAP_DUMP).isEqualTo(5.57 MB +-5 % margin) | ||
assertThat(retained after EXTRACTING_METADATA).isEqualTo(5.62 MB +-5 % margin) | ||
assertThat(retained after FINDING_RETAINED_OBJECTS).isEqualTo(5.72 MB +-5 % margin) | ||
assertThat(retained after FINDING_PATHS_TO_RETAINED_OBJECTS).isEqualTo(7.12 MB +-5 % margin) | ||
assertThat(retained after FINDING_DOMINATORS).isEqualTo(7.12 MB +-5 % margin) | ||
assertThat(retained after INSPECTING_OBJECTS).isEqualTo(7.13 MB +-5 % margin) | ||
assertThat(retained after COMPUTING_NATIVE_RETAINED_SIZE).isEqualTo(7.13 MB +-5 % margin) | ||
assertThat(retained after COMPUTING_RETAINED_SIZE).isEqualTo(6.05 MB +-5 % margin) | ||
assertThat(retained after EXTRACTING_METADATA).isEqualTo(5.53 MB +-5 % margin) | ||
assertThat(retained after FINDING_RETAINED_OBJECTS).isEqualTo(5.58 MB +-5 % margin) | ||
assertThat(retained after FINDING_PATHS_TO_RETAINED_OBJECTS).isEqualTo(7.02 MB +-5 % margin) | ||
assertThat(retained after FINDING_DOMINATORS).isEqualTo(7.02 MB +-5 % margin) | ||
assertThat(retained after INSPECTING_OBJECTS).isEqualTo(7.02 MB +-5 % margin) | ||
assertThat(retained after COMPUTING_NATIVE_RETAINED_SIZE).isEqualTo(7.02 MB +-5 % margin) | ||
assertThat(retained after COMPUTING_RETAINED_SIZE).isEqualTo(5.77 MB +-5 % margin) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely an unexpected change, yes it changes per env. I'd try rebasing & seeing if CI passes with and without this. |
||
} | ||
|
||
private fun indexRecordsOf(hprofFile: File): HprofIndex { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the names of the events wouldn't have "Notification" in them, and instead describe "what just happened". Then the notification listener can turn those events into the proper notifications.