Check mInputEventReceiver before sending timeline
Previously, it was possible to receive a FrameMetrics callback after the view was already detached from window. In that situation, the mInputEventReceiver is set to null and the object is disposed. But InputMetricsListener used to store another reference to mInputEventReceiver. So it's own object was never set to null. We would then try to send the timeline to native input receiver, and crash because the native object has already be deleted by the earlier dispose() call. So the sequence of events was: dispatchDetachedFromWindow mInputEventReceiver.dispose() native input receiver is deleted InputMetricsListener::onFrameMetricsAvailable mInputEventReceiver.reportTimeline try to access a native object using a null pointer crash A few options to fix this were investigated: 1) Unregister the observer when mAttachInfo.mThreadedRenderer is set to null. This is good to do, but it's not sufficient. The problem is that the native call to RenderProxy 'removeObserver' is not serviced immediately, but is posted to be completed sometime in the future. Therefore, the crash would not be fixed by it. Still, we should always register the observer for the active threadedRenderer, which is done in this CL. 2) Keep a weak reference to mInputEventReceiver inside InputMetricsListener. This would allow InputMetricsListener to check on the status of mInputEventReceiver. When it's disposed, it would be also set to null, so the weak reference resolution would fail. Unfortunately, 'mInputEventReceiver' is not the only reference to the object of WindowInputEventReceiver. It turns out that the receiver is also stored inside the queued events (see class QueuedInputEvent { private InputEventReceiver mReceiver }). From reviewing ag/153113, it should be OK to remove the receiver from QueuedInputEvent and simply keep track of whether the event is synthesized or not. But, that change would be too significant to make in this CL. Also, weak references have performance impact, so this may not be desirable anyways. 3) Do not store mInputEventReceiver in InputMetricsListener The chosen option is to simply use the variable mInputEventReceiver from the outer class. If the receiver is null, we don't notify about the metrics. This reverts commit d187cc76. Reason for revert: fixing this properly instead Bug: 184255546 Bug: 169866723 Test: settings -> privacy -> permission manager -> body sensors -> show system (only click once) -> google play services -> deny -> deny anyway Repeat the above 20 times. Observe that there's no crash of the activity. Change-Id: I5cf36ef068f7964572ab1a1475ff8ac53ae6beb5
Loading
Please register or sign in to comment