0

Fix unsafe use of weak ptr in WidgetInputHandlerManager

WidgetInputHandlerManager was using AsWeakPtr for a callback on the
compositor thread and another on the main thread.  This creates a
problem if both call sites create a weak reference with overlapping
lifetime since they must be dereferenced on the same thread.  Since
WidgetInputHandlerManager extends RefCountedThreadSafe, we should
use the 'this' pointer when binding a callback instead of a WeakPtr
if the callback will run on the compositor thread. We can still use
WeakPtr for callbacks that will run on the main thread.  WidgetBase,
which created the WidgetInputHandlerManager instance lives on the main
thread.

Bug: 476553, 1221290
Change-Id: Ie9bdf278b4a6c454fb8003cc04f567469e39f460
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2969643
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#894428}
This commit is contained in:
Kevin Ellis
2021-06-21 22:13:30 +00:00
committed by Chromium LUCI CQ
parent a82ddb5477
commit 2ce392c696
3 changed files with 12 additions and 12 deletions

@ -788,7 +788,7 @@ void WidgetInputHandlerManager::DidHandleInputEventSentToCompositor(
static_cast<const WebGestureEvent&>(event->Event()).PositionInWidget();
ElementAtPointCallback result_callback = base::BindOnce(
&WidgetInputHandlerManager::FindScrollTargetReply, AsWeakPtr(),
&WidgetInputHandlerManager::FindScrollTargetReply, this,
std::move(event), std::move(metrics), std::move(callback));
main_thread_task_runner_->PostTask(

@ -5557,10 +5557,6 @@ crbug.com/1054577 [ Mac ] external/wpt/storage-access-api/requestStorageAccess.s
crbug.com/476553 virtual/scroll-unification/external/wpt/dom/events/scrolling/overscroll-event-fired-to-scrolled-element.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/external/wpt/dom/events/scrolling/scrollend-event-for-user-scroll.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/hit-test-counts.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/middleClickAutoscroll-click-hyperlink.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/middleClickAutoscroll-click.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/middleClickAutoscroll-in-iframe.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/middleClickAutoscroll-nested-divs.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/mouse-cursor-change.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/platform-wheelevent-paging-xy-in-scrolling-div.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/remove-child-onscroll.html [ Pass Failure Timeout Crash ]
@ -5576,7 +5572,6 @@ crbug.com/476553 virtual/scroll-unification/fast/events/touch/gesture/gesture-ta
crbug.com/476553 virtual/scroll-unification/fast/events/touch/gesture/touch-gesture-scroll-input-field.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/touch/gesture/touch-gesture-scroll-listbox.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/events/wheel/wheel-latched-scroll-node-removed.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/scrolling/autoscroll-latch-clicked-node-if-parent-unscrollable.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/scrolling/resize-corner-tracking-touch.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/fast/scrolling/subpixel-overflow-mouse-drag.html [ Pass Failure Timeout Crash ]
crbug.com/476553 virtual/scroll-unification/http/tests/misc/destroy-middle-click-locked-target-crash.html [ Pass Failure Timeout Crash ]

@ -36,14 +36,19 @@ window.onload = async function()
promise_test (async (t) => {
await waitForCompositorCommit();
await mouseClickOn(container.offsetLeft + 10, container.offsetTop + 10, middleButton);
//Move mouse in unavailable direction first.
// Move mouse in unavailable direction first.
await mouseMoveTo(container.offsetLeft + (container.offsetWidth/2), container.offsetTop + 10);
//Move mouse in available direction to scroll the content.
await mouseMoveTo(container.offsetLeft + (container.offsetWidth/2), container.offsetTop + container.offsetHeight);
// Move mouse in available direction to scroll the content.
const scrollPromise = waitForScrollEvent(container);
// TODO(crbug.com/1221392): Autoscroll can prevent a synthetic movemove
// from completing. All GSUs are acked, but the queue is never empty.
/* await */ mouseMoveTo(container.offsetLeft + (container.offsetWidth/2),
container.offsetTop + container.offsetHeight);
await scrollPromise;
await waitFor(() => {
return container.scrollTop === container.scrollHeight - container.clientHeight;
return container.scrollTop ===
container.scrollHeight - container.clientHeight;
}, "Failed to scroll container with middle-click autoscroll.");
}, "Container scrolled in available direction.");
}
</script>
</script>