Skip to content

Commit

Permalink
Populate pointerId, pointerType for clicks from touch
Browse files Browse the repository at this point in the history
Pointer id and pointer type are not available in GestureManager
where we initiate the click from GestureTap events.

Pointer id algorithm is contained in PointerEventFactory. The problem
with surfacing pointer id in GestureManager is made harder because,
when we need to dispatch the click event, the pointer id for the
pointer event sequence that generates the click has already been
deleted from PointerEventFactory.

We will keep track of the last pointerdown event's pointer id, per
pointer type in PointerEventManager.

Using this approach, when we dispatch click event in GestureManager
we can then use PointerEventManager to access
the already deleted pointer id for the last pointer event sequence
that generated the click event. This works because we store the
pointer id of the last pointerdown event.

Bug: 1150593,1150441
TEST: third_party/blink/web_tests/external/wpt/pointerevents/pointerevent_click_is_a_pointerevent.html
Change-Id: I7e921934f99c78298bad96b69aaa163e63c4be29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2800231
Commit-Queue: Liviu Tinta <liviutinta@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#881062}
  • Loading branch information
liviutinta authored and Chromium LUCI CQ committed May 10, 2021
1 parent 253edc1 commit 5088421
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 84 deletions.
38 changes: 20 additions & 18 deletions third_party/blink/renderer/core/events/pointer_event_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,6 @@ inline int ToInt(WebPointerProperties::PointerType t) {
return static_cast<int>(t);
}

const char* PointerTypeNameForWebPointPointerType(
WebPointerProperties::PointerType type) {
// TODO(mustaq): Fix when the spec starts supporting hovering erasers.
switch (type) {
case WebPointerProperties::PointerType::kUnknown:
return "";
case WebPointerProperties::PointerType::kTouch:
return "touch";
case WebPointerProperties::PointerType::kPen:
return "pen";
case WebPointerProperties::PointerType::kMouse:
return "mouse";
default:
NOTREACHED();
return "";
}
}

uint16_t ButtonToButtonsBitfield(WebPointerProperties::Button button) {
#define CASE_BUTTON_TO_BUTTONS(enumLabel) \
case WebPointerProperties::Button::enumLabel: \
Expand Down Expand Up @@ -636,4 +618,24 @@ PointerId PointerEventFactory::GetPointerEventId(
return kInvalidId;
}

// static
const char* PointerEventFactory::PointerTypeNameForWebPointPointerType(
WebPointerProperties::PointerType type) {
switch (type) {
case WebPointerProperties::PointerType::kUnknown:
return "";
case WebPointerProperties::PointerType::kTouch:
return "touch";
case WebPointerProperties::PointerType::kPen:
return "pen";
case WebPointerProperties::PointerType::kMouse:
return "mouse";
default:
// TODO(mustaq): Fix when the spec starts supporting hovering erasers.
// See spec https://github.com/w3c/pointerevents/issues/134
NOTREACHED();
return "";
}
}

} // namespace blink
18 changes: 12 additions & 6 deletions third_party/blink/renderer/core/events/pointer_event_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ class CORE_EXPORT PointerEventFactory {
const FloatPoint& position_in_screen,
WebInputEvent::Type event_type);

static const char* PointerTypeNameForWebPointPointerType(
WebPointerProperties::PointerType type);

private:
// We use int64_t to cover the whole range for PointerId with no
// deleted hash value.
Expand All @@ -114,6 +117,14 @@ class CORE_EXPORT PointerEventFactory {
}
int RawId() const { return second; }
} IncomingId;

using IncomingIdToPointerIdMap =
HashMap<IncomingId,
PointerId,
WTF::PairHash<int, int>,
WTF::PairHashTraits<WTF::UnsignedWithZeroKeyHashTraits<int>,
WTF::UnsignedWithZeroKeyHashTraits<int>>>;

typedef struct PointerAttributes {
IncomingId incoming_id;
bool is_active_buttons;
Expand Down Expand Up @@ -153,12 +164,7 @@ class CORE_EXPORT PointerEventFactory {
LocalDOMWindow* view);

PointerId current_id_;
HashMap<IncomingId,
PointerId,
WTF::PairHash<int, int>,
WTF::PairHashTraits<WTF::UnsignedWithZeroKeyHashTraits<int>,
WTF::UnsignedWithZeroKeyHashTraits<int>>>
pointer_incoming_id_mapping_;
IncomingIdToPointerIdMap pointer_incoming_id_mapping_;
PointerIdKeyMap<PointerAttributes> pointer_id_mapping_;
int primary_id_[static_cast<int>(
WebPointerProperties::PointerType::kMaxValue) +
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/input/event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ EventHandler::EventHandler(LocalFrame& frame)
*selection_controller_)),
active_interval_timer_(frame.GetTaskRunner(TaskType::kUserInteraction),
this,
&EventHandler::ActiveIntervalTimerFired) {}
&EventHandler::ActiveIntervalTimerFired) {
pointer_event_manager_->SetGestureManager(gesture_manager_.Get());
}

void EventHandler::Trace(Visitor* visitor) const {
visitor->Trace(frame_);
Expand Down
19 changes: 19 additions & 0 deletions third_party/blink/renderer/core/input/gesture_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/editing/selection_controller.h"
#include "third_party/blink/renderer/core/event_type_names.h"
#include "third_party/blink/renderer/core/events/gesture_event.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
Expand Down Expand Up @@ -302,6 +303,10 @@ WebInputEventResult GestureManager::HandleGestureTap(
*tapped_element, event_handling_util::ParentForClickEvent);
auto* click_target_element = DynamicTo<Element>(click_target_node);

// TODO(crbug.com/1206108): Find a better approach to associate
// pointerdown pointerId with gesture tap
fake_mouse_up.id = last_pointerdown_event_pointer_id_;
fake_mouse_up.pointer_type = gesture_event.primary_pointer_type;
click_event_result =
mouse_event_manager_->SetMousePositionAndDispatchMouseEvent(
click_target_element, String(), event_type_names::kClick,
Expand Down Expand Up @@ -532,4 +537,18 @@ void GestureManager::ShowUnhandledTapUIIfNeeded(
#endif // BUILDFLAG(ENABLE_UNHANDLED_TAP)
}

void GestureManager::NotifyCurrentPointerDownId(PointerId pointer_id) {
// TODO(crbug.com/1206108): Find a better approach to associate
// pointerdown pointerId with gesture tap
// Pointerdown, pointerup events and tapdown and tap gestures are not fully
// ordered. It is guaranteed that tapdown follows pointerdown and tap
// follows pointerup. However, because pointer events and gestures are
// asynchronous, we can have the situation in which 2
// pointerdown/pointerup pairs are executed before any of their
// corresponding tapdown/tap pairs are executed.
// In that case, we would use the pointerId of pointerdown/pointerup set 2
// for the tapdown/tap pair of set 1. Which is wrong.
last_pointerdown_event_pointer_id_ = pointer_id;
}

} // namespace blink
12 changes: 12 additions & 0 deletions third_party/blink/renderer/core/input/gesture_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_INPUT_GESTURE_MANAGER_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_INPUT_GESTURE_MANAGER_H_

#include "third_party/blink/public/common/input/pointer_id.h"
#include "third_party/blink/public/platform/web_input_event_result.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/layout/hit_test_request.h"
Expand Down Expand Up @@ -54,6 +55,13 @@ class CORE_EXPORT GestureManager final
// position.
void SendContextMenuEventTouchDragEnd(const WebMouseEvent&);

// Used by PointerEventManager to notify GestureManager when a pointerdown
// pointerevent is dispatched. GestureManager is interested in knowing
// the pointerId of pointerdown event because it uses this pointer id
// to populate the pointerId for click and auxclick pointer events it
// generates.
void NotifyCurrentPointerDownId(PointerId pointer_id);

private:
WebInputEventResult HandleGestureShowPress();
WebInputEventResult HandleGestureTapDown(
Expand Down Expand Up @@ -93,6 +101,10 @@ class CORE_EXPORT GestureManager final

gfx::PointF long_press_position_in_root_frame_;

// The pointerId remapped by PointerEventFactory for the pointer event
// stream that generated the last pointerdown event
PointerId last_pointerdown_event_pointer_id_;

const Member<SelectionController> selection_controller_;
};

Expand Down
8 changes: 6 additions & 2 deletions third_party/blink/renderer/core/input/mouse_event_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "third_party/blink/renderer/core/editing/visible_selection.h"
#include "third_party/blink/renderer/core/events/drag_event.h"
#include "third_party/blink/renderer/core/events/mouse_event.h"
#include "third_party/blink/renderer/core/events/pointer_event_factory.h"
#include "third_party/blink/renderer/core/events/web_input_event_conversion.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
Expand Down Expand Up @@ -347,8 +348,11 @@ WebInputEventResult MouseEventManager::SetMousePositionAndDispatchMouseEvent(
const AtomicString& event_type,
const WebMouseEvent& web_mouse_event) {
SetElementUnderMouse(target_element, canvas_region_id, web_mouse_event);
return DispatchMouseEvent(element_under_mouse_, event_type, web_mouse_event,
canvas_region_id, nullptr, nullptr);
return DispatchMouseEvent(
element_under_mouse_, event_type, web_mouse_event, canvas_region_id,
nullptr, nullptr, false, web_mouse_event.id,
PointerEventFactory::PointerTypeNameForWebPointPointerType(
web_mouse_event.pointer_type));
}

WebInputEventResult MouseEventManager::DispatchMouseClickIfNeeded(
Expand Down
21 changes: 12 additions & 9 deletions third_party/blink/renderer/core/input/mouse_event_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_INPUT_MOUSE_EVENT_MANAGER_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_INPUT_MOUSE_EVENT_MANAGER_H_

#include "third_party/blink/public/common/input/pointer_id.h"
#include "third_party/blink/public/common/input/web_mouse_event.h"
#include "third_party/blink/public/platform/web_input_event_result.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/dom/synchronous_mutation_observer.h"
#include "third_party/blink/renderer/core/events/pointer_event_factory.h"
#include "third_party/blink/renderer/core/input/boundary_event_dispatcher.h"
#include "third_party/blink/renderer/core/page/event_with_hit_test_results.h"
#include "third_party/blink/renderer/platform/timer.h"
Expand Down Expand Up @@ -40,15 +42,16 @@ class CORE_EXPORT MouseEventManager final
virtual ~MouseEventManager();
void Trace(Visitor*) const override;

WebInputEventResult DispatchMouseEvent(EventTarget*,
const AtomicString&,
const WebMouseEvent&,
const String& canvas_region_id,
const FloatPoint* last_position,
EventTarget* related_target,
bool check_for_listener = false,
const PointerId& pointer_id = 0,
const String& pointer_type = "");
WebInputEventResult DispatchMouseEvent(
EventTarget*,
const AtomicString&,
const WebMouseEvent&,
const String& canvas_region_id,
const FloatPoint* last_position,
EventTarget* related_target,
bool check_for_listener = false,
const PointerId& pointer_id = PointerEventFactory::kInvalidId,
const String& pointer_type = "");

WebInputEventResult SetMousePositionAndDispatchMouseEvent(
Element* target_element,
Expand Down
12 changes: 12 additions & 0 deletions third_party/blink/renderer/core/input/pointer_event_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "third_party/blink/renderer/core/html/canvas/html_canvas_element.h"
#include "third_party/blink/renderer/core/input/event_handler.h"
#include "third_party/blink/renderer/core/input/event_handling_util.h"
#include "third_party/blink/renderer/core/input/gesture_manager.h"
#include "third_party/blink/renderer/core/input/mouse_event_manager.h"
#include "third_party/blink/renderer/core/input/touch_action_util.h"
#include "third_party/blink/renderer/core/layout/hit_test_canvas_result.h"
Expand Down Expand Up @@ -104,6 +105,7 @@ void PointerEventManager::Trace(Visitor* visitor) const {
visitor->Trace(pending_pointer_capture_target_);
visitor->Trace(touch_event_manager_);
visitor->Trace(mouse_event_manager_);
visitor->Trace(gesture_manager_);
}

PointerEventManager::PointerEventBoundaryEventDispatcher::
Expand Down Expand Up @@ -465,6 +467,12 @@ WebInputEventResult PointerEventManager::DispatchTouchPointerEvent(
: nullptr);

if (pointer_event) {
if (pointer_event->type() == event_type_names::kPointerdown) {
if (gesture_manager_.Get()) {
gesture_manager_->NotifyCurrentPointerDownId(
pointer_event->pointerId());
}
}
result = SendTouchPointerEvent(pointer_event_target.target_element,
pointer_event, web_pointer_event.hovering);
} else {
Expand Down Expand Up @@ -1115,4 +1123,8 @@ void PointerEventManager::RemoveLastMousePosition() {
pointer_event_factory_.RemoveLastPosition(PointerEventFactory::kMouseId);
}

void PointerEventManager::SetGestureManager(GestureManager* gesture_manager) {
gesture_manager_ = Member<GestureManager>(gesture_manager);
}

} // namespace blink
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/input/pointer_event_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace blink {

class LocalFrame;
class MouseEventManager;
class GestureManager;

// This class takes care of dispatching all pointer events and keeps track of
// properties of active pointer events.
Expand Down Expand Up @@ -108,6 +109,8 @@ class CORE_EXPORT PointerEventManager final
// function.
WebInputEventResult FlushEvents();

void SetGestureManager(GestureManager* gesture_manager);

private:
class EventTargetAttributes : public GarbageCollected<EventTargetAttributes> {
public:
Expand Down Expand Up @@ -280,6 +283,8 @@ class CORE_EXPORT PointerEventManager final
// main thread, or all events (touch start/end/move).
bool skip_touch_filter_discrete_ = false;
bool skip_touch_filter_all_ = false;

Member<GestureManager> gesture_manager_;
};

} // namespace blink
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -6857,3 +6857,5 @@ crbug.com/1205669 [ Mac10.13 ] virtual/threaded/external/wpt/animation-worklet/i

# Sheriff 2021-05-07
crbug.com/1206734 [ Mac10.13 ] external/wpt/websockets/Send-binary-blob.any.worker.html?wpt_flags=h2 [ Timeout ]

crbug.com/1206108 external/wpt/pointerevents/pointerevent_click_is_a_pointerevent_multiple_clicks.html [ Pass Failure ]
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<title>click is a PointerEvent</title>
<meta name="variant" content="?mouse">
<meta name="variant" content="?pen">
<meta name="variant" content="?touch">
<link rel="help" href="https://github.com/w3c/pointerevents/pull/317">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
Expand Down Expand Up @@ -54,5 +55,4 @@
}

run_test(inputSource);
// TODO(crbug.com/1150593): Add wpt test for touch (reuse run_test)
</script>

0 comments on commit 5088421

Please sign in to comment.