Skip to content

Commit

Permalink
For mozilla-mobile#18836: prevent StartupActivityLog from growing inf…
Browse files Browse the repository at this point in the history
…initely.

We do this is as a separate commit over the original implementation
because it's simpler to implement the class without this optimization.
  • Loading branch information
mcomella committed Apr 8, 2021
1 parent 7ed32dc commit b11b200
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ private val logger = Logger("StartupActivityLog")
* A record of the [Activity] created, started, and stopped events as well as [Application]
* foreground and background events. See [log] for the log. This class is expected to be
* registered in [Application.onCreate] by calling [registerInAppOnCreate].
*
* To prevent this list from growing infinitely, we clear the list when the application is stopped.
* This is acceptable from the current requirements: we never need to inspect more than the current
* start up.
*/
class StartupActivityLog {

Expand Down Expand Up @@ -66,8 +70,9 @@ class StartupActivityLog {
}

override fun onStop(owner: LifecycleOwner) {
_log.add(LogEntry.AppStopped)
logEntries()
_log.clear() // Optimization: see class kdoc for details.
_log.add(LogEntry.AppStopped)
}
}

Expand Down
28 changes: 23 additions & 5 deletions app/src/test/java/org/mozilla/fenix/perf/StartupActivityLogTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,20 @@ class StartupActivityLogTest {
@Test // we test start and stop individually due to the clear-on-stop behavior.
fun `WHEN app observer start is called THEN it is added directly to the log`() {
assertTrue(log.log.isEmpty())
val expected = mutableListOf<LogEntry>()

appObserver.onStart(mockk())
expected.add(LogEntry.AppStarted)
assertEquals(expected, log.log)
assertEquals(listOf(LogEntry.AppStarted), log.log)

appObserver.onStart(mockk())
assertEquals(listOf(LogEntry.AppStarted, LogEntry.AppStarted), log.log)
}

@Test // we test start and stop individually due to the clear-on-stop behavior.
fun `WHEN app observer stop is called THEN it is added directly to the log`() {
assertTrue(log.log.isEmpty())

appObserver.onStop(mockk())
expected.add(LogEntry.AppStopped)
assertEquals(expected, log.log)
assertEquals(listOf(LogEntry.AppStopped), log.log)
}

@Test
Expand All @@ -81,6 +86,19 @@ class StartupActivityLogTest {
assertEquals(expected, log.log)
}

@Test
fun `WHEN app STOPPED is called THEN the log is emptied expect for the stop event`() {
assertTrue(log.log.isEmpty())

activityCallbacks.onActivityCreated(mockk(), null)
activityCallbacks.onActivityStarted(mockk())
appObserver.onStart(mockk())
assertEquals(3, log.log.size)

appObserver.onStop(mockk())
assertEquals(listOf(LogEntry.AppStopped), log.log)
}

@Test
fun `GIVEN debug log level WHEN logEntries is called THEN there is no logcat call`() {
log.logEntries(logger, Priority.DEBUG)
Expand Down

0 comments on commit b11b200

Please sign in to comment.