Skip to content

Commit

Permalink
[CT][EngagementSignals] Add root scroll offset to GestureEventAck
Browse files Browse the repository at this point in the history
Bug: 1340074
Change-Id: Icb04b103c13185215c7e1e991e8187a9c93055bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3782937
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035193}
  • Loading branch information
Donn Denman authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent edf3596 commit b52cea6
Show file tree
Hide file tree
Showing 40 changed files with 397 additions and 182 deletions.
12 changes: 10 additions & 2 deletions content/browser/android/gesture_listener_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents_observer.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom-forward.h"
#include "ui/events/android/gesture_event_type.h"
#include "ui/events/blink/did_overscroll_params.h"
#include "ui/gfx/geometry/size_f.h"
Expand Down Expand Up @@ -154,7 +155,8 @@ void GestureListenerManager::SetHasListenersAttached(JNIEnv* env,

void GestureListenerManager::GestureEventAck(
const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result) {
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data) {
// This is called to fix crash happening while WebContents is being
// destroyed. See https://crbug.com/803244#c20
if (web_contents_->IsBeingDestroyed())
Expand All @@ -175,8 +177,14 @@ void GestureListenerManager::GestureEventAck(
env, j_obj, /*isDirectionUp*/ event.data.scroll_begin.delta_y_hint > 0);
return;
}
float x = -1.f, y = -1.f;
if (scroll_result_data && scroll_result_data->root_scroll_offset) {
x = scroll_result_data->root_scroll_offset->x();
y = scroll_result_data->root_scroll_offset->y();
}

Java_GestureListenerManagerImpl_onEventAck(
env, j_obj, static_cast<int>(event.GetType()), consumed);
env, j_obj, static_cast<int>(event.GetType()), consumed, x, y);
}

void GestureListenerManager::DidStopFlinging() {
Expand Down
4 changes: 3 additions & 1 deletion content/browser/android/gesture_listener_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "content/browser/android/render_widget_host_connector.h"
#include "content/common/content_export.h"
#include "third_party/blink/public/mojom/input/input_event_result.mojom-shared.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom-forward.h"

namespace blink {
class WebGestureEvent;
Expand Down Expand Up @@ -55,7 +56,8 @@ class CONTENT_EXPORT GestureListenerManager : public RenderWidgetHostConnector {
bool has_listeners_attached() const { return has_listeners_attached_; }
void SetHasListenersAttached(JNIEnv* env, jboolean enabled);
void GestureEventAck(const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result);
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data);
void DidStopFlinging();
bool FilterInputEvent(const blink::WebInputEvent& event);
void DidOverscroll(const ui::DidOverscrollParams& params);
Expand Down
13 changes: 9 additions & 4 deletions content/browser/renderer_host/cross_process_frame_connector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "third_party/blink/public/common/frame/frame_visual_properties.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/mojom/frame/intrinsic_sizing_info.mojom.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom-forward.h"
#include "ui/gfx/geometry/dip_util.h"

namespace content {
Expand Down Expand Up @@ -260,7 +261,8 @@ bool CrossProcessFrameConnector::TransformPointToCoordSpaceForView(

void CrossProcessFrameConnector::ForwardAckedTouchpadZoomEvent(
const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result) {
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data) {
auto* root_view = GetRootRenderWidgetHostView();
if (!root_view)
return;
Expand All @@ -269,7 +271,8 @@ void CrossProcessFrameConnector::ForwardAckedTouchpadZoomEvent(
const gfx::PointF root_point =
view_->TransformPointToRootCoordSpaceF(event.PositionInWidget());
root_event.SetPositionInWidget(root_point);
root_view->GestureEventAck(root_event, ack_result);
root_view->GestureEventAck(root_event, ack_result,
std::move(scroll_result_data));
}

bool CrossProcessFrameConnector::BubbleScrollEvent(
Expand Down Expand Up @@ -507,12 +510,14 @@ void CrossProcessFrameConnector::DidUpdateVisualProperties(

void CrossProcessFrameConnector::DidAckGestureEvent(
const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result) {
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data) {
auto* root_view = GetRootRenderWidgetHostView();
if (!root_view)
return;

root_view->ChildDidAckGestureEvent(event, ack_result);
root_view->ChildDidAckGestureEvent(event, ack_result,
std::move(scroll_result_data));
}

void CrossProcessFrameConnector::SetVisibilityForChildViews(
Expand Down
7 changes: 5 additions & 2 deletions content/browser/renderer_host/cross_process_frame_connector.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "third_party/blink/public/mojom/frame/lifecycle.mojom.h"
#include "third_party/blink/public/mojom/frame/viewport_intersection_state.mojom.h"
#include "third_party/blink/public/mojom/input/input_event_result.mojom-shared.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom-forward.h"
#include "third_party/blink/public/mojom/input/pointer_lock_result.mojom-shared.h"
#include "ui/display/screen_infos.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -165,7 +166,8 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
// for processing.
void ForwardAckedTouchpadZoomEvent(
const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result);
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data);

// A gesture scroll sequence that is not consumed by a child must be bubbled
// to ancestors who may consume it.
Expand Down Expand Up @@ -251,7 +253,8 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
bool has_size() const { return has_size_; }

void DidAckGestureEvent(const blink::WebGestureEvent& event,
blink::mojom::InputEventResultState ack_result);
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data);

// Called by RenderWidgetHostViewChildFrame to update the visibility of any
// nested child RWHVCFs inside it.
Expand Down
24 changes: 17 additions & 7 deletions content/browser/renderer_host/input/gesture_event_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/trace_event/trace_event.h"
#include "content/browser/renderer_host/input/touchpad_tap_suppression_controller.h"
#include "content/browser/renderer_host/input/touchscreen_tap_suppression_controller.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom.h"
#include "ui/events/blink/blink_features.h"
#include "ui/events/blink/web_input_event_traits.h"

Expand All @@ -16,8 +17,8 @@ using blink::WebInputEvent;

namespace content {

GestureEventQueue::GestureEventWithLatencyInfoAndAckState::
GestureEventWithLatencyInfoAndAckState(
GestureEventQueue::GestureEventWithLatencyInfoAckStateAndScrollResultData::
GestureEventWithLatencyInfoAckStateAndScrollResultData(
const GestureEventWithLatencyInfo& event)
: GestureEventWithLatencyInfo(event) {}

Expand Down Expand Up @@ -157,7 +158,8 @@ void GestureEventQueue::ProcessGestureAck(
blink::mojom::InputEventResultSource ack_source,
blink::mojom::InputEventResultState ack_result,
WebInputEvent::Type type,
const ui::LatencyInfo& latency) {
const ui::LatencyInfo& latency,
blink::mojom::ScrollResultDataPtr scroll_result_data) {
TRACE_EVENT0("input", "GestureEventQueue::ProcessGestureAck");

if (sent_events_awaiting_ack_.empty()) {
Expand All @@ -174,6 +176,7 @@ void GestureEventQueue::ProcessGestureAck(
if (outstanding_event.event.GetType() == type) {
outstanding_event.latency.AddNewLatencyFrom(latency);
outstanding_event.set_ack_info(ack_source, ack_result);
outstanding_event.set_scroll_result_data(std::move(scroll_result_data));
break;
}
}
Expand All @@ -191,17 +194,24 @@ void GestureEventQueue::AckCompletedEvents() {
auto iter = sent_events_awaiting_ack_.begin();
if (iter->ack_state() == blink::mojom::InputEventResultState::kUnknown)
break;
GestureEventWithLatencyInfoAndAckState event = *iter;
GestureEventWithLatencyInfoAckStateAndScrollResultData event = *iter;
sent_events_awaiting_ack_.erase(iter);
AckGestureEventToClient(event, event.ack_source(), event.ack_state());

auto scroll_result_data = blink::mojom::ScrollResultData::New(
event.scroll_result_data().root_scroll_offset);

AckGestureEventToClient(event, event.ack_source(), event.ack_state(),
std::move(scroll_result_data));
}
}

void GestureEventQueue::AckGestureEventToClient(
const GestureEventWithLatencyInfo& event_with_latency,
blink::mojom::InputEventResultSource ack_source,
blink::mojom::InputEventResultState ack_result) {
client_->OnGestureEventAck(event_with_latency, ack_source, ack_result);
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data) {
client_->OnGestureEventAck(event_with_latency, ack_source, ack_result,
std::move(scroll_result_data));
}

TouchpadTapSuppressionController*
Expand Down
42 changes: 33 additions & 9 deletions content/browser/renderer_host/input/gesture_event_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
#include "content/browser/renderer_host/event_with_latency_info.h"
#include "content/browser/renderer_host/input/fling_controller.h"
#include "content/common/content_export.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/mojom/input/input_event_result.mojom-shared.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom.h"

namespace content {
class GestureEventQueueTest;
Expand All @@ -33,7 +35,8 @@ class CONTENT_EXPORT GestureEventQueueClient {
virtual void OnGestureEventAck(
const GestureEventWithLatencyInfo& event,
blink::mojom::InputEventResultSource ack_source,
blink::mojom::InputEventResultState ack_result) = 0;
blink::mojom::InputEventResultState ack_result,
blink::mojom::ScrollResultDataPtr scroll_result_data) = 0;
};

// Despite its name, this class isn't so much one queue as it is a collection
Expand Down Expand Up @@ -104,7 +107,8 @@ class CONTENT_EXPORT GestureEventQueue {
void ProcessGestureAck(blink::mojom::InputEventResultSource ack_source,
blink::mojom::InputEventResultState ack_result,
blink::WebInputEvent::Type type,
const ui::LatencyInfo& latency);
const ui::LatencyInfo& latency,
blink::mojom::ScrollResultDataPtr scroll_result_data);

// Returns the |TouchpadTapSuppressionController| instance.
TouchpadTapSuppressionController* GetTouchpadTapSuppressionController();
Expand Down Expand Up @@ -141,10 +145,12 @@ class CONTENT_EXPORT GestureEventQueue {
friend class GestureEventQueueTest;
friend class MockRenderWidgetHost;

class GestureEventWithLatencyInfoAndAckState
class GestureEventWithLatencyInfoAckStateAndScrollResultData
: public GestureEventWithLatencyInfo {
public:
GestureEventWithLatencyInfoAndAckState(const GestureEventWithLatencyInfo&);
GestureEventWithLatencyInfoAckStateAndScrollResultData(
const GestureEventWithLatencyInfo&);
~GestureEventWithLatencyInfoAckStateAndScrollResultData() = default;
blink::mojom::InputEventResultState ack_state() const { return ack_state_; }
void set_ack_info(blink::mojom::InputEventResultSource source,
blink::mojom::InputEventResultState state) {
Expand All @@ -154,12 +160,28 @@ class CONTENT_EXPORT GestureEventQueue {
blink::mojom::InputEventResultSource ack_source() const {
return ack_source_;
}
void set_scroll_result_data(
blink::mojom::ScrollResultDataPtr scroll_result_data) {
// Creating a new instance and setting the field(s) explicitly because
// having blink::mojom::ScrollResultDataPtr as a field makes this class
// move-only and causes issues pushing into and removing from
// sent_events_awaiting_ack_.
// TODO(sinansahin): This class can probably be refactored to work with
// being move-only.
scroll_result_data_ = blink::mojom::ScrollResultData(
scroll_result_data ? scroll_result_data->root_scroll_offset
: absl::nullopt);
}
const blink::mojom::ScrollResultData& scroll_result_data() {
return scroll_result_data_;
}

private:
blink::mojom::InputEventResultSource ack_source_ =
blink::mojom::InputEventResultSource::kUnknown;
blink::mojom::InputEventResultState ack_state_ =
blink::mojom::InputEventResultState::kUnknown;
blink::mojom::ScrollResultData scroll_result_data_;
};

// Inovked on the expiration of the debounce interval to release
Expand All @@ -173,9 +195,11 @@ class CONTENT_EXPORT GestureEventQueue {
// ACK completed events in order until we have reached an incomplete event.
// Will preserve the FIFO order as events originally arrived.
void AckCompletedEvents();
void AckGestureEventToClient(const GestureEventWithLatencyInfo&,
blink::mojom::InputEventResultSource,
blink::mojom::InputEventResultState);
void AckGestureEventToClient(
const GestureEventWithLatencyInfo&,
blink::mojom::InputEventResultSource,
blink::mojom::InputEventResultState,
blink::mojom::ScrollResultDataPtr scroll_result_data);

bool FlingInProgressForTest() const;

Expand All @@ -187,8 +211,8 @@ class CONTENT_EXPORT GestureEventQueue {

bool processing_acks_ = false;

using GestureQueueWithAckState =
base::circular_deque<GestureEventWithLatencyInfoAndAckState>;
using GestureQueueWithAckState = base::circular_deque<
GestureEventWithLatencyInfoAckStateAndScrollResultData>;

// Stores outstanding events that have been sent to the renderer but not yet
// been ACK'd. These are kept in the order they were sent in so that they can
Expand Down

0 comments on commit b52cea6

Please sign in to comment.