Skip to content

Commit b7aaf6a

Browse files
author
W. James MacLean
committed
Fix an issue where touch events were dispatched to the incorrect webview
Was caused by touchscreen_gesture_target_queue_ + prevent default. The webview target was calculated on TouchStart and pushed to the the queue. It was removed on its corresponding GestureTapDown. However, if the TouchStart had default behavior prevented, no GestureTapDown would ever occur, and the queue would forever have an extra element. Fixed by converting touchscreen_gesture_target_queue_ to a map with unique_touch_event_id as keys and gesture target data as values. Also updates AUTHORS file. [email protected] (cherry picked from commit 09a2f88) Bug: 736623, 739831 Change-Id: If3bf844e2bcfb9a1ca6d6a56cdc7575e0767e6b6 Reviewed-on: https://chromium-review.googlesource.com/547669 Commit-Queue: James MacLean <[email protected]> Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Ken Buchanan <[email protected]> Reviewed-by: James MacLean <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#488759} Reviewed-on: https://chromium-review.googlesource.com/586747 Cr-Commit-Position: refs/branch-heads/3163@{#54} Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
1 parent 7ba12ce commit b7aaf6a

7 files changed

+238
-66
lines changed

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,7 @@ Endless Mobile, Inc. <*@endlessm.com>
863863
Google Inc. <*@google.com>
864864
Hewlett-Packard Development Company, L.P. <*@hp.com>
865865
Igalia S.L. <*@igalia.com>
866+
NIKE, Inc. <*@nike.com>
866867
NVIDIA Corporation <*@nvidia.com>
867868
Neverware Inc. <*@neverware.com>
868869
Opera Software ASA <*@opera.com>

content/browser/renderer_host/render_widget_host_input_event_router.cc

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
5757
active_touches_ = 0;
5858
}
5959

60-
// If the target that's being destroyed is in the gesture target queue, we
60+
// If the target that's being destroyed is in the gesture target map, we
6161
// replace it with nullptr so that we maintain the 1:1 correspondence between
62-
// queue entries and the touch sequences that underly them.
63-
for (size_t i = 0; i < touchscreen_gesture_target_queue_.size(); ++i) {
64-
if (touchscreen_gesture_target_queue_[i].target == view)
65-
touchscreen_gesture_target_queue_[i].target = nullptr;
62+
// map entries and the touch sequences that underly them.
63+
for (auto it : touchscreen_gesture_target_map_) {
64+
if (it.second.target == view)
65+
it.second.target = nullptr;
6666
}
6767

6868
if (view == mouse_capture_target_.target)
@@ -390,6 +390,15 @@ unsigned CountChangedTouchPoints(const blink::WebTouchEvent& event) {
390390

391391
} // namespace
392392

393+
// Any time a touch start event is handled/consumed/default prevented it is
394+
// removed from the gesture map, because it will never create a gesture.
395+
void RenderWidgetHostInputEventRouter::OnHandledTouchStartOrFirstTouchMove(
396+
uint32_t unique_touch_event_id) {
397+
// unique_touch_event_id of 0 implies a gesture not created by a touch.
398+
DCHECK_NE(unique_touch_event_id, 0U);
399+
touchscreen_gesture_target_map_.erase(unique_touch_event_id);
400+
}
401+
393402
void RenderWidgetHostInputEventRouter::RouteTouchEvent(
394403
RenderWidgetHostViewBase* root_view,
395404
blink::WebTouchEvent* event,
@@ -413,7 +422,11 @@ void RenderWidgetHostInputEventRouter::RouteTouchEvent(
413422
// might be to always transform each point to the |touch_target_.target|
414423
// for the duration of the sequence.
415424
touch_target_.delta = transformed_point - original_point;
416-
touchscreen_gesture_target_queue_.push_back(touch_target_);
425+
DCHECK(touchscreen_gesture_target_map_.find(
426+
event->unique_touch_event_id) ==
427+
touchscreen_gesture_target_map_.end());
428+
touchscreen_gesture_target_map_[event->unique_touch_event_id] =
429+
touch_target_;
417430

418431
if (!touch_target_.target)
419432
return;
@@ -835,23 +848,45 @@ void RenderWidgetHostInputEventRouter::RouteTouchscreenGestureEvent(
835848
return;
836849
}
837850

851+
auto gesture_target_it =
852+
touchscreen_gesture_target_map_.find(event->unique_touch_event_id);
853+
bool no_matching_id =
854+
gesture_target_it == touchscreen_gesture_target_map_.end();
855+
856+
// We use GestureTapDown to detect the start of a gesture sequence since
857+
// there is no WebGestureEvent equivalent for ET_GESTURE_BEGIN. Note that
858+
// this means the GestureFlingCancel that always comes between
859+
// ET_GESTURE_BEGIN and GestureTapDown is sent to the previous target, in
860+
// case it is still in a fling.
861+
bool is_gesture_start =
862+
event->GetType() == blink::WebInputEvent::kGestureTapDown;
863+
838864
// On Android it is possible for touchscreen gesture events to arrive that
839865
// are not associated with touch events, because non-synthetic events can be
840-
// created by ContentView. In that case the target queue will be empty.
841-
if (touchscreen_gesture_target_queue_.empty()) {
866+
// created by ContentView.
867+
// These gesture events should always have a unique_touch_event_id of 0.
868+
bool no_gesture_target =
869+
event->unique_touch_event_id != 0 && no_matching_id && is_gesture_start;
870+
871+
if (no_gesture_target) {
872+
UMA_HISTOGRAM_BOOLEAN("Event.FrameEventRouting.NoGestureTarget",
873+
no_gesture_target);
874+
NOTREACHED() << "Gesture sequence start detected with no target available.";
875+
// It is still safe to continue; we will recalculate the target.
876+
}
877+
878+
// If this is a non-touch gesture or there was an no_matching_id error on
879+
// gesture start, then the target must be recalculated.
880+
if (event->unique_touch_event_id == 0 ||
881+
(no_matching_id && is_gesture_start)) {
842882
gfx::Point transformed_point;
843883
gfx::Point original_point(event->x, event->y);
844884
touchscreen_gesture_target_.target =
845885
FindEventTarget(root_view, original_point, &transformed_point);
846886
touchscreen_gesture_target_.delta = transformed_point - original_point;
847-
} else if (event->GetType() == blink::WebInputEvent::kGestureTapDown) {
848-
// We use GestureTapDown to detect the start of a gesture sequence since
849-
// there is no WebGestureEvent equivalent for ET_GESTURE_BEGIN. Note that
850-
// this means the GestureFlingCancel that always comes between
851-
// ET_GESTURE_BEGIN and GestureTapDown is sent to the previous target, in
852-
// case it is still in a fling.
853-
touchscreen_gesture_target_ = touchscreen_gesture_target_queue_.front();
854-
touchscreen_gesture_target_queue_.pop_front();
887+
} else if (is_gesture_start) {
888+
touchscreen_gesture_target_ = gesture_target_it->second;
889+
touchscreen_gesture_target_map_.erase(gesture_target_it);
855890

856891
// Abort any scroll bubbling in progress to avoid double entry.
857892
if (touchscreen_gesture_target_.target &&

content/browser/renderer_host/render_widget_host_input_event_router.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
#include <stdint.h>
99

10-
#include <deque>
10+
#include <map>
1111
#include <unordered_map>
1212
#include <vector>
1313

@@ -67,6 +67,7 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
6767
void RouteGestureEvent(RenderWidgetHostViewBase* root_view,
6868
blink::WebGestureEvent* event,
6969
const ui::LatencyInfo& latency);
70+
void OnHandledTouchStartOrFirstTouchMove(uint32_t unique_touch_event_id);
7071
void RouteTouchEvent(RenderWidgetHostViewBase* root_view,
7172
blink::WebTouchEvent *event,
7273
const ui::LatencyInfo& latency);
@@ -125,7 +126,7 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
125126

126127
TargetData() : target(nullptr) {}
127128
};
128-
using TargetQueue = std::deque<TargetData>;
129+
using TargetMap = std::map<uint32_t, TargetData>;
129130

130131
void ClearAllObserverRegistrations();
131132

@@ -158,7 +159,7 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
158159
const blink::WebGestureEvent& event);
159160

160161
FrameSinkIdOwnerMap owner_map_;
161-
TargetQueue touchscreen_gesture_target_queue_;
162+
TargetMap touchscreen_gesture_target_map_;
162163
TargetData touch_target_;
163164
TargetData touchscreen_gesture_target_;
164165
TargetData touchpad_gesture_target_;
@@ -185,7 +186,9 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
185186

186187
DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostInputEventRouter);
187188
FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
188-
InputEventRouterGestureTargetQueueTest);
189+
InputEventRouterGestureTargetMapTest);
190+
FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
191+
InputEventRouterGesturePreventDefaultTargetMapTest);
189192
FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
190193
InputEventRouterTouchpadGestureTargetTest);
191194
};

content/browser/renderer_host/render_widget_host_view_android.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,6 +1748,13 @@ void RenderWidgetHostViewAndroid::ProcessAckedTouchEvent(
17481748
gesture_provider_.OnTouchEventAck(
17491749
touch.event.unique_touch_event_id, event_consumed,
17501750
InputEventAckStateIsSetNonBlocking(ack_result));
1751+
if (touch.event.touch_start_or_first_touch_move && event_consumed &&
1752+
host_->delegate() && host_->delegate()->GetInputEventRouter()) {
1753+
host_->delegate()
1754+
->GetInputEventRouter()
1755+
->OnHandledTouchStartOrFirstTouchMove(
1756+
touch.event.unique_touch_event_id);
1757+
}
17511758
}
17521759

17531760
void RenderWidgetHostViewAndroid::GestureEventAck(

content/browser/renderer_host/render_widget_host_view_aura.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,14 @@ void RenderWidgetHostViewAura::ProcessAckedTouchEvent(
10711071
host->dispatcher()->ProcessedTouchEvent(
10721072
touch.event.unique_touch_event_id, window_, result,
10731073
InputEventAckStateIsSetNonBlocking(ack_result));
1074+
if (touch.event.touch_start_or_first_touch_move &&
1075+
result == ui::ER_HANDLED && host_->delegate() &&
1076+
host_->delegate()->GetInputEventRouter()) {
1077+
host_->delegate()
1078+
->GetInputEventRouter()
1079+
->OnHandledTouchStartOrFirstTouchMove(
1080+
touch.event.unique_touch_event_id);
1081+
}
10741082
sent_ack = true;
10751083
}
10761084
}

0 commit comments

Comments
 (0)