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
Such that we only set the flag in case window visibility actually
changes. There are a lot of other cases where we need to do a
window relayout which can lead to visible jank.
Test: FrameMetricsListenerTest, manual
Bug: 185902609
Change-Id: Ia09ce640f3a89bf9dd4ff0f4e4469e88a513709a
InputDispatcher will receive the timing information from the app. This
timing information will contain:
1) inputEventId : the input event id that corresponds to the current set
of times
2) gpuCompletedTime : the time at which gpu has finished processing the
frame and the buffer is getting sent from the app to surfaceflinger
3) presentTime : time time at which the frame was displayed
Bug: 169866723
Test: tested by printing the data that was passed
Test: atest InputEventSenderAndReceiverTest
Change-Id: I9bf38473d07c7bd4df3de6bc77b0173faa257f06
To reduce the size of 'receiveFinishedSignals' and to simplify the
future CLs in this area, let's move the part where the Java code is
being notified into a separate function.
The behaviour should remain the same.
Test: atest InputEventSenderAndReceiverTest
Bug: 169866723
Change-Id: If0f0d630600a754d01d761f7a3bafce342b96d26
InputEventAssigner does not currently get notified about all frames. In
the existing logic, only the cases for batched consumption would be
counted as 'choreographer callback'. This misses several cases, such as:
1) CALLBACK_INPUT was never scheduled, but a frame is produced
2) CALLBACK_INPUT was scheduled, but no events were consumed
However, it is critical that InputEventAssigner knows about all frames,
because it needs to reset the 'down' state for proper frame attribution.
At the same time, we refactor the logic in InputEventAssigner. In the
updated logic, we will specifically store the event id with ACTION_DOWN
only. In all other cases, we will use the current / latest input event.
In this CL, we reset the 'down' state after the input event id is
used for the current frame. A good proxy for that is the
ThreadedRenderer::draw call, which in turns
calls attachInfo.mViewRootImpl.getUpdatedFrameInfo().
Also, rename "onChoreographerCallback" since it's not exactly clear
which callback this is referring to. In the new logic, this callback
will happen from 'draw'/'onDraw' functions.
Bug: 167947340
Test: add LOG_ALWAYS_FATAL crashes in LatencyTracker for duplicate
'reportTimeline' calls. The crashes are gone after this CL.
Change-Id: I5fad6665d10e0594963209b138e9bd2bdf00ca38
To match the new signature of receiveFinishedSignal api, update the
InputEventSender.
Bug: 167947340
Test: atest libinput_tests
Test: atest InputEventSenderAndReceiverTest
b6957c6cf7676a90d9c091bf6bed33ed33a9652a
Change-Id: Ida7829dbdd42c1c608c3244380538d587b245b2a
When a frame is determined to be caused by an input event, it will be
labeled with a specific input event id. The newly added entity,
InputEventAssigner, is responsible for determining which input
event we should use.
For regular ACTION_MOVE events, we will take the latest input event as
the event that has caused the frame. For the initial gesture
(ACTION_DOWN), we will use the first event (the ACTION_DOWN itself) to
figure out the down latency. This will allow us to split up 'down' and
'scroll' latencies.
Bug: 169866723
Test: looked at the data printed locally to logcat
Test: atest InputTests
Change-Id: I8513a36960e5d652d07655d1267e758d0c59ced7
Present time is available to apps, but is not currently stored inside
FrameInfo. In this CL, we add a location for storing the present time inside
FrameInfo.
Currently, the metrics reporter is triggered after the buffer is sent to
SurfaceFlinger. That means, metrics for anything that happens after gpu
draw completes are currently not available.
In a future CL, we will populate the 'presentTime' field.
In a future CL, we will register a metrics observer inside ViewRootImpl
in order to send this data to InputDispatcher. This will enable
end-to-end touch latency metrics collection.
Bug: 169866723
Test: printed present time inside FrameTimeline.cpp (surfaceflinger
side) and compared to the values printed inside a metrics observer
registered in ViewRootImpl (not done in this CL)
Test: atest ViewFrameInfoTest
Change-Id: I7d0f8c5d7b5a2572abdc4e107123e8938a36f582
Bug: 174932174
Test: I solemnly swear I tested this conflict resolution.
Exempt-From-Owner-Approval: refactoring with team leads buy-in
Change-Id: I9262a08ffc1ccede8e519d0eed90ed2bfcf0232c
As general background, OWNERS files expedite code reviews by helping
code authors quickly find relevant reviewers, and they also ensure
that stakeholders are involved in code changes in their areas.
Some teams under frameworks/base/ have been using OWNERS files
successfully for many years, and we're ready to expand them to cover
more areas. Here's the historical coverage statistics for the last
two years of changes before these new OWNERS changes land:
-- 56% of changes are fully covered by OWNERS
-- 17% of changes are partially covered by OWNERS
-- 25% of changes have no OWNERS coverage
Working closely with team leads, we've now identified clear OWNERS on
a per-package basis, and we're using "include" directives whenever
possible to to simplify future maintenance. With this extensive
effort, we've now improved our coverage as follows:
-- 98% of changes are fully covered by OWNERS
-- 1% of changes are partially covered by OWNERS
-- 1% of changes have no OWNERS coverage
This specific change is automatically generated by a script that
identifies relevant "include" directives.
Bug: 174932174
Test: manual
Exempt-From-Owner-Approval: refactoring with team leads buy-in
Merged-In: I3480ddf2fe7ba3dfb922b459d4da01fa17a2c813
Change-Id: I3480ddf2fe7ba3dfb922b459d4da01fa17a2c813
FrameInfo will now be per-window; that is, per-ViewRootImpl.
Some of the information should remain “global” (it remain in Choreographer),
while some information is going to become ViewRootImpl-specific.
Before the information gets passed to the native layer,
the ViewRootImpl-specific info will be stitched together
with the general Choreographer info.
This change is useful in order to correctly correlate frames with a specific
input event. In the unlikely scenario of a user touching two windows of the
same app simultaneously, this change will allow us to correctly measure the
latency of both frames produced by the windows.
Design doc: https://docs.google.com/document/d/1KMpMBlOxnl7zkWBCbXZZE6ZlaHEA4efYnN6WYK8n3FE/edit?resourcekey=0-eqooVNP0SskupljlTFvtOQ
Test: atest ViewFrameInfoTest
Bug: 169866723
Change-Id: Ib0bf9cd51cbcc0b9b70460c929c480eb490ec322
android-support-test is long deprecated, and the runner flakes if both
androidx.test and android-support-test are included.
Bug: 171910851
Bug: 172570181
Test: m -j InputTests
Change-Id: Icd257481583e9555875d5fd0e93caba49d44caab
This patch reverts the earlier patch, ag/11959351, because the correct
solution is aosp/1459888.
When batched input is available, the Choreographer::CALLBACK_INPUT is
scheduled by ViewRootImpl. The callback always runs, even if onStop has
been called. So the earlier patch was invalid.
In practice, what happened was that the batched input was not getting
consumed after FOCUS event was received in the receiver. This was fixed
in aosp/1459888.
The consumption mechanism is complex, so I added a doc explaining how it
works, for future readers.
The newly added test, IncompleteMotionTest, ensures that this regression
does not happen again.
Test: atest IncompleteMotionTest
Bug: 160561987
Change-Id: Ic0a453ebbce1f847de8e9cb87bdffdaf72c12637
To prevent regressions, such as the ones introduced by the recent
InputChannel refactor, put InputTests into presubmit.
Test: none
Bug: 169154685
Change-Id: I8b8aa04e836455ed7bd21cf5f63c9fa8ffabacc2
If a gesture monitor is unresponsive, today there will not be an ANR
dialog. Since the input channel token for the gesture monitor is not
provided to WindowManager, there is no way for the proper ANR to occur.
That means, the user will not know that the gesture monitor isn't
working.
To fix the issue, we record the pid of the caller when first registering
a gesture monitor. This will be stored in InputManagerService.
Next, when an ANR for this gesture monitor occurs, we will provide this
pid to the WM.
WM will use this pid to properly blame that process.
Bug: 161904619
Bug: 160903019
Test: atest AnrTest
Test: adb shell input dump
Change-Id: Ie1a16352a116914ba6550958ad41de07cff063be
