Refactor SlowRenderingDetectorImpl so that it's thread-safe

[mateuszrzeszutek]

Fixes #359

Removed the FrameMetricsAggregator, because it kept internal state that was not subject to any sort of synchronization whatsoever, and was accessed from multiple threads in an unsafe way. I refactored (simplified perhaps?) SlowRenderingDetectorImpl so that it uses FrameMetrics directly -- which, what's interesting, will allow us to collect per-activity metrics in the future, if we wish to do so.

[breedx-splk]

Removed the FrameMetricsAggregator, because it kept internal state that was not subject to any sort of synchronization whatsoever, and was accessed from multiple threads in an unsafe way.

This is interesting because every usage of the frameMetrics instance within SlowRenderingDetectorImpl was definitely guarded by the lock, so I'm curious about where you observed unsafe accesses? Maybe you were thinking about the lack of coordination between the internal synchronization and our usage of it?

[breedx-splk]

It makes me a little nervous to kinda reimplement some of what already exists in the guts of the platform sdk class FrameMetricsAggregator and how this could be impacted in the future. However, I do appreciate the sensitivity to thread-safety and a desire to make that better.

Aside from the reimplementation, I don't see anything to stop this change (but I would still like to better understand where the original thread safety concern came from). In the interest in getting the RC build out, let's go with this.

[mateuszrzeszutek]

(but I would still like to better understand where the original thread safety concern came from).

FrameMetricsAggregator internally started a thread (a HandlerThread for running the frame metrics listener) that modified the SparseIntArray[] metrics field without any sort of synchronization. So, while we were synchronizing all the reads in our own, the writes happening in the core library were completely unguarded.