0

Fix touch resize control on Scroll Unif. path

With Scroll Unification turned on, resize controls on touch screens do
not work and this seems to be because the code that handles the resize
(ScrollManager::HandleScrollGestureOnResizer) is triggered on the
main thread in response to GestureScroll events and when scroll
Unification is enabled, the main thread will not receive GestureScroll
events.

This patch tries to fix this by adding logic to PointerEventManager so
that resizing can be triggered based on touch events and not
GestureScroll events.
In paint_chunker.cc we draw a touch action rect over the resize control
corner. To ensure that the resize is handled as expected,
we need to make sure that if a touchmove is over a region which doesn't
have an event listener, we should still forward it from the compositor
to the main thread if it is in a touch sequence that began with a
touchstart that *was* in a region with a touch event listener. This
change captured in input_proxy_handler.
In crbug.com/729031, we wanted to allow blocking touchmoves that follow non-blocking touchstarts so this patch only forwards touchmoves that follow blocking touchstarts.

The change to event_handler.cc is to make sure that similar to touch
events on scrollbars, touch events on resizers are not adjusted.

This patch splits the web_test resize-corner-tracking-touch.html into
two test files: one with the original filename and another with a new
filename, resize-iframe-corner-tracking-touch.html.
The new file tests resizing on an iframe itself and on contents of an
iframe. This patch does not cause these currently failing
iframe-related test cases to pass and it seems to be because the
HitTest done by PointerEventManager::HandlePointerEvent "passes
through" the outer document and into the iframe document, but the
resizer exists in the outer document.
A follow-up patch will try to fix the iframe-related tests.
This patch causes the currently failing non-iframe test cases to pass.

Bug: 1248581
Change-Id: I2496af643c5cf124e8c2dbde78a2c47f63340745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3946605
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065517}
This commit is contained in:
David Awogbemila
2022-10-31 16:14:14 +00:00
committed by Chromium LUCI CQ
parent 88e7c6731e
commit fe61d39448
12 changed files with 222 additions and 44 deletions

@@ -1713,7 +1713,7 @@ bool EventHandler::BestClickableNodeForHitTestResult(
// adjustment only takes into account DOM nodes so a touch over a scrollbar
// will be adjusted towards nearby nodes. This leads to things like textarea
// scrollbars being untouchable.
if (result.GetScrollbar()) {
if (result.GetScrollbar() || result.IsOverResizer()) {
target_node = nullptr;
return false;
}

@@ -30,10 +30,12 @@
#include "third_party/blink/renderer/core/page/chrome_client.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/page/pointer_lock_controller.h"
#include "third_party/blink/renderer/core/paint/paint_layer.h"
#include "third_party/blink/renderer/core/scroll/scrollbar.h"
#include "third_party/blink/renderer/core/timing/dom_window_performance.h"
#include "third_party/blink/renderer/core/timing/event_timing.h"
#include "third_party/blink/renderer/core/timing/window_performance.h"
#include "third_party/blink/renderer/platform/geometry/layout_size.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -105,6 +107,8 @@ void PointerEventManager::Clear() {
pointer_capture_target_.clear();
pending_pointer_capture_target_.clear();
dispatching_pointer_id_ = 0;
resize_scrollable_area_.Clear();
offset_from_resize_corner_ = LayoutSize();
}
void PointerEventManager::Trace(Visitor* visitor) const {
@@ -116,6 +120,7 @@ void PointerEventManager::Trace(Visitor* visitor) const {
visitor->Trace(mouse_event_manager_);
visitor->Trace(gesture_manager_);
visitor->Trace(captured_scrollbar_);
visitor->Trace(resize_scrollable_area_);
}
PointerEventManager::PointerEventBoundaryEventDispatcher::
@@ -667,6 +672,9 @@ WebInputEventResult PointerEventManager::HandlePointerEvent(
if (HandleScrollbarTouchDrag(event, pointer_event_target.scrollbar))
return WebInputEventResult::kHandledSuppressed;
if (HandleResizerDrag(pointer_event, pointer_event_target))
return WebInputEventResult::kHandledSuppressed;
// Any finger lifting is a user gesture only when it wasn't associated with a
// scroll.
// https://docs.google.com/document/d/1oF1T3O7_E4t1PYHV6gyCwHxOi3ystm0eSL5xZu7nvOg/edit#
@@ -744,6 +752,58 @@ bool PointerEventManager::HandleScrollbarTouchDrag(const WebPointerEvent& event,
return handled;
}
bool PointerEventManager::HandleResizerDrag(
const WebPointerEvent& event,
const event_handling_util::PointerEventTarget& pointer_event_target) {
switch (event.GetType()) {
case WebPointerEvent::Type::kPointerDown: {
Node* node = pointer_event_target.target_element;
if (!node || !node->GetLayoutObject() ||
!node->GetLayoutObject()->EnclosingLayer())
return false;
PaintLayer* layer = node->GetLayoutObject()->EnclosingLayer();
if (!layer->GetScrollableArea())
return false;
gfx::Point p = frame_->View()->ConvertFromRootFrame(
gfx::ToFlooredPoint(event.PositionInWidget()));
if (layer->GetScrollableArea()->IsAbsolutePointInResizeControl(
p, kResizerForTouch)) {
resize_scrollable_area_ = layer->GetScrollableArea();
resize_scrollable_area_->SetInResizeMode(true);
frame_->GetPage()->GetChromeClient().SetTouchAction(frame_,
TouchAction::kNone);
offset_from_resize_corner_ =
LayoutSize(resize_scrollable_area_->OffsetFromResizeCorner(p));
return true;
}
break;
}
case WebInputEvent::Type::kPointerMove: {
if (resize_scrollable_area_ && resize_scrollable_area_->InResizeMode()) {
gfx::Point pos = gfx::ToRoundedPoint(event.PositionInWidget());
resize_scrollable_area_->Resize(pos, offset_from_resize_corner_);
return true;
}
break;
}
case WebInputEvent::Type::kPointerUp: {
if (resize_scrollable_area_ && resize_scrollable_area_->InResizeMode()) {
resize_scrollable_area_->SetInResizeMode(false);
resize_scrollable_area_.Clear();
offset_from_resize_corner_ = LayoutSize();
return true;
}
break;
}
default:
return false;
}
return false;
}
WebInputEventResult PointerEventManager::CreateAndDispatchPointerEvent(
Element* target,
const AtomicString& mouse_event_name,

@@ -9,6 +9,7 @@
#include "third_party/blink/renderer/core/input/boundary_event_dispatcher.h"
#include "third_party/blink/renderer/core/input/touch_event_manager.h"
#include "third_party/blink/renderer/core/page/touch_adjustment.h"
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
#include "third_party/blink/renderer/platform/wtf/deque.h"
@@ -254,11 +255,17 @@ class CORE_EXPORT PointerEventManager final
bool HandleScrollbarTouchDrag(const WebPointerEvent&, Scrollbar*);
bool HandleResizerDrag(const WebPointerEvent&,
const event_handling_util::PointerEventTarget&);
// NOTE: If adding a new field to this class please ensure that it is
// cleared in |PointerEventManager::clear()|.
const Member<LocalFrame> frame_;
WeakMember<PaintLayerScrollableArea> resize_scrollable_area_;
LayoutSize offset_from_resize_corner_;
// Prevents firing mousedown, mousemove & mouseup in-between a canceled
// pointerdown and next pointerup/pointercancel.
// See "PREVENT MOUSE EVENT flag" in the spec:

@@ -81,6 +81,7 @@ HitTestResult::HitTestResult(const HitTestResult& other)
inner_url_element_(other.URLElement()),
scrollbar_(other.GetScrollbar()),
is_over_embedded_content_view_(other.IsOverEmbeddedContentView()),
is_over_resizer_(other.is_over_resizer_),
canvas_region_id_(other.CanvasRegionId()) {
// Only copy the NodeSet in case of list hit test.
list_based_test_result_ =
@@ -127,6 +128,7 @@ void HitTestResult::PopulateFromCachedResult(const HitTestResult& other) {
is_over_embedded_content_view_ = other.IsOverEmbeddedContentView();
cacheable_ = other.cacheable_;
canvas_region_id_ = other.CanvasRegionId();
is_over_resizer_ = other.IsOverResizer();
// Only copy the NodeSet in case of list hit test.
list_based_test_result_ =
@@ -593,6 +595,7 @@ void HitTestResult::Append(const HitTestResult& other) {
inner_url_element_ = other.URLElement();
is_over_embedded_content_view_ = other.IsOverEmbeddedContentView();
canvas_region_id_ = other.CanvasRegionId();
is_over_resizer_ = other.IsOverResizer();
}
if (other.list_based_test_result_) {

@@ -151,6 +151,10 @@ class CORE_EXPORT HitTestResult {
void SetIsOverEmbeddedContentView(bool b) {
is_over_embedded_content_view_ = b;
}
void SetIsOverResizer(bool is_over_resizer) {
is_over_resizer_ = is_over_resizer;
}
bool IsOverResizer() const { return is_over_resizer_; }
bool IsSelected(const HitTestLocation& location) const;
String Title(TextDirection&) const;
@@ -238,6 +242,11 @@ class CORE_EXPORT HitTestResult {
// Returns true if we are over a EmbeddedContentView (and not in the
// border/padding area of a LayoutEmbeddedContent for example).
bool is_over_embedded_content_view_;
// This is true if the location is over the bottom right of a resizable
// object, where resize controls are located. See
// PaintLayerScrollableArea::IsAbsolutePointInResizeControl for how that is
// tested.
bool is_over_resizer_ = false;
mutable Member<NodeSet> list_based_test_result_;
String canvas_region_id_;

@@ -1963,8 +1963,10 @@ bool PaintLayerScrollableArea::HitTestOverflowControls(
gfx::Rect resize_control_rect;
if (GetLayoutBox()->CanResize()) {
resize_control_rect = ResizerCornerRect(kResizerForPointer);
if (resize_control_rect.Contains(local_point))
if (resize_control_rect.Contains(local_point)) {
result.SetIsOverResizer(true);
return true;
}
}
int resize_control_size = max(resize_control_rect.height(), 0);

@@ -277,7 +277,8 @@ void PaintChunker::CreateScrollHitTestChunk(
auto& hit_test_data = chunk.EnsureHitTestData();
hit_test_data.scroll_translation = scroll_translation;
hit_test_data.scroll_hit_test_rect = rect;
if (id.type == DisplayItem::Type::kScrollbarHitTest) {
if (id.type == DisplayItem::Type::kScrollbarHitTest ||
id.type == DisplayItem::Type::kResizerScrollHitTest) {
hit_test_data.touch_action_rects.push_back(
TouchActionRect{rect, TouchAction::kNone});
}

@@ -1315,6 +1315,15 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleTouchStart(
}
}
// main_thread_touch_sequence_start_disposition_ will indicate that touchmoves
// in the sequence should be sent to the main thread if the touchstart event
// was blocking. We may change |result| from DROP_EVENT if there is a touchend
// listener so we need to update main_thread_touch_sequence_start_disposition_
// here (before the check for a touchend listener) so that we send touchmoves
// only IF we send the (blocking) touchstart.
if (!(result == DID_HANDLE || result == DROP_EVENT))
main_thread_touch_sequence_start_disposition_ = result;
// If |result| is still DROP_EVENT look at the touch end handler as we may
// not want to discard the entire touch sequence. Note this code is
// explicitly after the assignment of the |touch_result_| in
@@ -1374,8 +1383,20 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleTouchMove(
touch_event.touch_start_or_first_touch_move) {
bool is_touching_scrolling_layer;
cc::TouchAction allowed_touch_action = cc::TouchAction::kAuto;
EventDisposition result = HitTestTouchEvent(
touch_event, &is_touching_scrolling_layer, &allowed_touch_action);
EventDisposition result;
bool is_main_thread_touch_sequence_with_blocking_start =
main_thread_touch_sequence_start_disposition_.has_value() &&
main_thread_touch_sequence_start_disposition_.value() == DID_NOT_HANDLE;
// If the touchmove occurs in a touch sequence that's being forwarded to
// the main thread, we can avoid the hit test since we want to also forward
// touchmoves in the sequence to the main thread.
if (is_main_thread_touch_sequence_with_blocking_start) {
touch_result_ = main_thread_touch_sequence_start_disposition_.value();
result = touch_result_.value();
} else {
result = HitTestTouchEvent(touch_event, &is_touching_scrolling_layer,
&allowed_touch_action);
}
TRACE_EVENT_INSTANT2(
"input", "Allowed TouchAction", TRACE_EVENT_SCOPE_THREAD, "TouchAction",
cc::TouchActionToString(allowed_touch_action), "disposition", result);
@@ -1400,6 +1421,10 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleTouchEnd(
}
if (touch_event.touches_length == 1)
touch_result_.reset();
if (main_thread_touch_sequence_start_disposition_.has_value())
main_thread_touch_sequence_start_disposition_.reset();
return DID_NOT_HANDLE;
}

@@ -342,6 +342,8 @@ class PLATFORM_EXPORT InputHandlerProxy : public cc::InputHandlerClient,
bool gesture_pinch_in_progress_ = false;
bool in_inertial_scrolling_ = false;
bool scroll_sequence_ignored_;
absl::optional<EventDisposition>
main_thread_touch_sequence_start_disposition_;
// Used to animate rubber-band/bounce over-scroll effect.
std::unique_ptr<ElasticOverscrollController> elastic_overscroll_controller_;

@@ -6753,7 +6753,7 @@ crbug.com/1311128 external/wpt/paint-timing/fcp-only/fcp-document-opacity-text.h
# Scroll Unification known issues (go/su-web-tests) for enabling in test:
crbug.com/1311431 [ Mac ] fast/scroll-behavior/overscroll-behavior.html [ Failure Pass ]
crbug.com/1248581 fast/scrolling/resize-corner-tracking-touch.html [ Failure Pass ]
crbug.com/1248581 fast/scrolling/resize-iframe-corner-tracking-touch.html [ Failure Pass ]
crbug.com/1312036 virtual/hidpi/fast/scrolling/scrollbars/dsf-ready/mouse-interactions-dsf-2.html [ Failure Pass ]
# These tests fail on SwiftShader FEMU due to a change in log/exp implementation.

@@ -24,8 +24,6 @@ we have enough elements to scroll the page</div>
<div id="div" style="width: 150px; height: 100px;"></div>
<textarea id="textarea1" style="width: 150px; height: 100px;"></textarea>
<br>
<iframe id="iframe1" src="resources/resize-corner-tracking-touch-iframe.html"
style="resize:both; width: 200px; height: 200px"></iframe>
<script type="text/javascript">
async function touchDrag(target, offset, delta) {
const rect = target.getBoundingClientRect();
@@ -74,7 +72,6 @@ we have enough elements to scroll the page</div>
async function setupTest(scrollTop = 50) {
resetStyle('div', 'width: 150px; height: 100px;');
resetStyle('textarea1', 'width: 150px; height: 100px;');
resetStyle('iframe1', 'resize:both; width: 200px; height: 200px;');
// Scroll the page first to test that resize works with a scrolled page.
document.scrollingElement.scrollTop = scrollTop;
@@ -133,40 +130,5 @@ we have enough elements to scroll the page</div>
assert_no_resize(target, oldBounds);
}, 'Touchpad scroll should not resize the textarea.');
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const oldBounds = iframe.getBoundingClientRect();
await touchDrag(iframe, {x: -6, y: -7}, {dx: -20, dy: -10});
assert_resize(iframe, oldBounds);
}, 'Touch drag inside the resizer area of iframe resizes the iframe.');
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const oldBounds = iframe.getBoundingClientRect();
// Do not require an exact hit on the resizer due to touch fuzzing.
await touchDrag(iframe, {x: -20, y: -20}, {dx: -20, dy: -10});
assert_resize(iframe, oldBounds);
}, 'Touch drag a little off the resizer area of an iframe will resize ' +
' the iframe.');
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const rect = iframe.getBoundingClientRect();
const target = iframe.contentDocument.getElementById("textarea2");
const oldBounds = target.getBoundingClientRect();
await touchDrag(target, {x: rect.left - 6, y: rect.top - 7},
{dx: 20, dy: 10});
assert_resize(target, oldBounds);
}, 'Touch drag inside the resizer area of a textarea inside an iframe ' +
'will resize the textarea.');
};
</script>

@@ -0,0 +1,107 @@
<!DOCTYPE html>
<script src='../../resources/gesture-util.js'></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<style>
iframe {
border: blue 2px solid;
}
</style>
<iframe id="iframe1" src="resources/resize-corner-tracking-touch-iframe.html"
style="resize:both; width: 200px; height: 200px"></iframe>
<script type="text/javascript">
async function touchDrag(target, offset, delta) {
const rect = target.getBoundingClientRect();
const x0 = rect.right + offset.x;
const y0 = rect.bottom + offset.y;
drag = {
start_x: x0,
start_y: y0,
end_x: x0 + delta.dx,
end_y: y0 + delta.dy
};
await touchDragTo(drag);
return waitForCompositorCommit();
}
async function touchpadScroll(target, offset, delta, device) {
var rect = target.getBoundingClientRect();
const precise_scroll_delta = true;
await smoothScrollWithXY(-delta.dx, -delta.dy,
rect.right + offset.x, rect.bottom + offset.y,
GestureSourceType.TOUCHPAD_INPUT, SPEED_INSTANT,
precise_scroll_delta);
return waitForCompositorCommit();
}
function assert_resize(target, oldBounds) {
const newBounds = target.getBoundingClientRect();
assert_true(newBounds.width != oldBounds.width &&
newBounds.height != oldBounds.height,
'Element failed to resize');
}
function assert_no_resize(target, oldBounds) {
const newBounds = target.getBoundingClientRect();
assert_true(newBounds.width == oldBounds.width &&
newBounds.height == oldBounds.height,
'Element unexpectedly resized');
}
function resetStyle(element_id, style) {
const element = document.getElementById(element_id);
element.style = style;
}
async function setupTest(scrollTop = 50) {
resetStyle('iframe1', 'resize:both; width: 200px; height: 200px;');
// Scroll the page first to test that resize works with a scrolled page.
document.scrollingElement.scrollTop = scrollTop;
return waitForCompositorCommit();
}
window.onload = () => {
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const oldBounds = iframe.getBoundingClientRect();
await touchDrag(iframe, {x: -6, y: -7}, {dx: -20, dy: -10});
assert_resize(iframe, oldBounds);
}, 'Touch drag inside the resizer area of iframe resizes the iframe.');
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const oldBounds = iframe.getBoundingClientRect();
// Do not require an exact hit on the resizer due to touch fuzzing.
await touchDrag(iframe, {x: -20, y: -20}, {dx: -20, dy: -10});
assert_resize(iframe, oldBounds);
}, 'Touch drag a little off the resizer area of an iframe will resize ' +
' the iframe.');
promise_test(async () => {
await setupTest(/* scrollTop */ 150);
const iframe = document.getElementById("iframe1");
const rect = iframe.getBoundingClientRect();
const target = iframe.contentDocument.getElementById("textarea2");
const oldBounds = target.getBoundingClientRect();
await touchDrag(target, {x: rect.left - 6, y: rect.top - 7},
{dx: 20, dy: 10});
assert_resize(target, oldBounds);
}, 'Touch drag inside the resizer area of a textarea inside an iframe ' +
'will resize the textarea.');
};
</script>