tree 48d2715632b84cff87fe6f637b4c75104116a059
parent ff09af37dd4a88119362ed396f31ace31e95ebf6
author Siarhei Vishniakou <svv@google.com> 1619161982 +0000
committer Siarhei Vishniakou <svv@google.com> 1620355597 -0700

Check mInputEventReceiver before sending timeline -- 2

Try to submit the 'Check mInputEventReceiver' patch again.

This reverts commit 35fccc4535dd8eedd5e60af21651e20215ef4768.

Reason for revert: Added a check to ensure that only valid data is
getting sent. Now, the InputChannel will not be closed because all
InputMessages will always be valid.
Added logging to help investigate the reason why the present time is in
the past of the gpuCompletedTime.

Original commit message:

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.

Bug: 169866723
Bug: 184255546
Bug: 184771626
Bug: 185015591
Bug: 186664409
Test: verified on fold 2 device, open/close the phone, touch remains
operational
Test: atest InputEventSenderAndReceiverTest
Change-Id: I01076d771e9432f08a3b5f8426c14759b56f3e12

Change-Id: Ie71c9ed38051e3e5ec9cb089951d759f52937fdd