From d7f0b314234433e749a232b587177cdb9b6499fb Mon Sep 17 00:00:00 2001 From: Kartar Singh <kartarsingh@google.com> Date: Fri, 28 Mar 2025 14:18:28 -0700 Subject: [PATCH] [InputVizard] Relax CHECK for same downtime in all events of sequence. We already know this is not true in some cases, due to Android platform bug(b/395610162), where we see the downtime is different for some of the touch moves. We see the CHECK failing in field which means this must be true for some other cases as well. For now we can relax the check until the platform side bugs are identified and fixed. Bug: 401233890 Change-Id: I810e940bc7d2208a182a871c109f0ba3e7718ee1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6403095 Reviewed-by: Jonathan Ross <jonross@chromium.org> Commit-Queue: Kartar Singh <kartarsingh@google.com> Reviewed-by: Petr Cermak <petrcermak@chromium.org> Reviewed-by: Andrew Grieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/main@{#1439706} --- base/android/scoped_input_event.cc | 32 +++++++++++++++++++ base/android/scoped_input_event.h | 7 ++++ base/tracing/protos/chrome_track_event.proto | 20 +++++++++++- .../input/android_state_transfer_handler.cc | 22 +++++++++---- 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/base/android/scoped_input_event.cc b/base/android/scoped_input_event.cc index 5ec87678ad13e..10d2fe4ee62a1 100644 --- a/base/android/scoped_input_event.cc +++ b/base/android/scoped_input_event.cc @@ -40,6 +40,38 @@ ScopedInputEvent& ScopedInputEvent::operator=(ScopedInputEvent&& other) { return *this; } +#if BUILDFLAG(ENABLE_BASE_TRACING) +void ScopedInputEvent::WriteIntoTrace( + perfetto::TracedProto<perfetto::protos::pbzero::EventForwarder> forwarder) + const { + if (!a_input_event_) { + return; + } + + const int history_size = + static_cast<const int>(AMotionEvent_getHistorySize(a_input_event_)); + forwarder->set_history_size(history_size); + + forwarder->set_latest_time_ns(AMotionEvent_getEventTime(a_input_event_)); + if (history_size > 0) { + forwarder->set_oldest_time_ns(AMotionEvent_getHistoricalEventTime( + a_input_event_, /* history_index= */ 0)); + } + forwarder->set_down_time_ns(AMotionEvent_getDownTime(a_input_event_)); + + forwarder->set_x_pixel( + AMotionEvent_getX(a_input_event_, /* pointer_index= */ 0)); + forwarder->set_y_pixel( + AMotionEvent_getY(a_input_event_, /* pointer_index= */ 0)); + + const int action = + AMotionEvent_getAction(a_input_event_) & AMOTION_EVENT_ACTION_MASK; + forwarder->set_action( + static_cast<perfetto::protos::pbzero::EventForwarder::AMotionEventAction>( + action)); +} +#endif // BUILDFLAG(ENABLE_BASE_TRACING) + void ScopedInputEvent::DestroyIfNeeded() { if (a_input_event_ == nullptr) { return; diff --git a/base/android/scoped_input_event.h b/base/android/scoped_input_event.h index 0efb4c0a1c17e..2a81cda82ec8c 100644 --- a/base/android/scoped_input_event.h +++ b/base/android/scoped_input_event.h @@ -9,6 +9,7 @@ #include "base/base_export.h" #include "base/memory/raw_ptr.h" +#include "base/trace_event/base_tracing.h" namespace base::android { @@ -31,6 +32,12 @@ class BASE_EXPORT ScopedInputEvent { const AInputEvent* a_input_event() const { return a_input_event_.get(); } +#if BUILDFLAG(ENABLE_BASE_TRACING) + void WriteIntoTrace( + perfetto::TracedProto<perfetto::protos::pbzero::EventForwarder> forwarder) + const; +#endif // BUILDFLAG(ENABLE_BASE_TRACING) + private: void DestroyIfNeeded(); diff --git a/base/tracing/protos/chrome_track_event.proto b/base/tracing/protos/chrome_track_event.proto index 2b1c79436cd79..ebf42b9e5acd6 100644 --- a/base/tracing/protos/chrome_track_event.proto +++ b/base/tracing/protos/chrome_track_event.proto @@ -1677,7 +1677,7 @@ message ScrollMetrics { // // All data comes from MotionEvent getters so read these for more context: // https://developer.android.com/reference/android/view/MotionEvent -// Next id: 9 +// Next id: 10 message EventForwarder { // The events getHistorySize(). optional int32 history_size = 1; @@ -1696,6 +1696,24 @@ message EventForwarder { optional bool has_x_movement = 6; // Determine if the previous forwarded event changed y coordinate. optional bool has_y_movement = 7; + + // The enum comes from Android NDK header: `android/input.h`. + enum AMotionEventAction { + AMOTION_EVENT_ACTION_DOWN = 0; + AMOTION_EVENT_ACTION_UP = 1; + AMOTION_EVENT_ACTION_MOVE = 2; + AMOTION_EVENT_ACTION_CANCEL = 3; + AMOTION_EVENT_ACTION_OUTSIDE = 4; + AMOTION_EVENT_ACTION_POINTER_DOWN = 5; + AMOTION_EVENT_ACTION_POINTER_UP = 6; + AMOTION_EVENT_ACTION_HOVER_MOVE = 7; + AMOTION_EVENT_ACTION_SCROLL = 8; + AMOTION_EVENT_ACTION_HOVER_ENTER = 9; + AMOTION_EVENT_ACTION_HOVER_EXIT = 10; + AMOTION_EVENT_ACTION_BUTTON_PRESS = 11; + AMOTION_EVENT_ACTION_BUTTON_RELEASE = 12; + } + optional AMotionEventAction action = 9; } // TouchDispositionGestureFilter is a class on android that detects and forwards diff --git a/components/viz/service/input/android_state_transfer_handler.cc b/components/viz/service/input/android_state_transfer_handler.cc index b83bfaf0b7937..4d5b691af5435 100644 --- a/components/viz/service/input/android_state_transfer_handler.cc +++ b/components/viz/service/input/android_state_transfer_handler.cc @@ -96,7 +96,14 @@ void AndroidStateTransferHandler::StateOnTouchTransfer( bool AndroidStateTransferHandler::OnMotionEvent( base::android::ScopedInputEvent input_event, const FrameSinkId& root_frame_sink_id) { - TRACE_EVENT("input", "AndroidStateTransferHandler::OnMotionEvent"); + TRACE_EVENT("input", "AndroidStateTransferHandler::OnMotionEvent", + [&](perfetto::EventContext& ctx) { + auto* chrome_track_event = + ctx.event<perfetto::protos::pbzero::ChromeTrackEvent>(); + auto* forwarder = chrome_track_event->set_event_forwarder(); + + input_event.WriteIntoTrace(ctx.Wrap(forwarder)); + }); const int action = AMotionEvent_getAction(input_event.a_input_event()) & AMOTION_EVENT_ACTION_MASK; @@ -200,14 +207,17 @@ void AndroidStateTransferHandler::EmitPendingTransfersHistogram() { void AndroidStateTransferHandler::HandleTouchEvent( base::android::ScopedInputEvent input_event) { + // TODO(crbug.com/406986388) : Add flow events to track the events starting + // from when they were first were processed by Viz. + TRACE_EVENT("input", "AndroidStateTransferHandler::HandleTouchEvent"); CHECK(state_for_curr_sequence_.has_value()); const int action = AMotionEvent_getAction(input_event.a_input_event()) & AMOTION_EVENT_ACTION_MASK; - // Due to an Android platform bug b/395610162, we see some motion events have - // different down time than the rest of the sequence. - CHECK(action == AMOTION_EVENT_ACTION_MOVE || - GetEventDowntime(input_event) == - state_for_curr_sequence_->transfer_state->down_time_ms); + + if (GetEventDowntime(input_event) != + state_for_curr_sequence_->transfer_state->down_time_ms) { + TRACE_EVENT_INSTANT("input", "DifferentDownTimeInSequence"); + } if (!state_for_curr_sequence_->rir_support) { if (action == AMOTION_EVENT_ACTION_CANCEL ||