Bug 219547

Summary: REGRESSION(r270425) [GTK] wheel scrolling stopped working
Product: WebKit Reporter: Lauro Moura <lmoura>
Component: ScrollingAssignee: Lauro Moura <lmoura>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, alicem, bugs-noreply, cgarcia, clord, fujii.hironori, jan.brummer, jmason, mcatanzaro, pnormand, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218764
https://bugs.webkit.org/show_bug.cgi?id=219726
Bug Depends on: 218764    
Bug Blocks:    
Attachments:
Description Flags
Tentative patch none

Lauro Moura
Reported 2020-12-04 11:37:53 PST
# Timeouts fast/events/remove-child-onscroll.html fast/events/wheel/wheel-event-destroys-frame.html fast/events/wheel/wheel-event-destroys-overflow.html fast/events/wheel/wheel-event-outside-body.html fast/events/wheel/wheelevent-basic.html fast/events/wheel/wheelevent-in-text-node.html fast/events/wheel/wheelevent-mousewheel-interaction.html fast/scrolling/iframe-scrollable-after-back.html fast/scrolling/overflow-scrollable-after-back.html pointer-lock/mouse-event-delivery.html scrollbars/scroll-rtl-or-bt-layer.html # Failures fast/events/wheel/continuous-platform-wheelevent-in-scrolling-div.html fast/frames/flattening/scrolling-in-object.html fast/scrolling/rtl-scrollbars-listbox-scroll.html Using the MiniBrowser, I can't scroll anything when async scrolling is enabled. Could not test disabling it yet, though.
Attachments
Tentative patch (4.44 KB, patch)
2020-12-09 22:21 PST, Lauro Moura 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Lauro Moura
Comment 1 2020-12-04 11:45:31 PST
Gardened in r270443.
Lauro Moura
Comment 2 2020-12-08 09:58:14 PST
*** Bug 219646 has been marked as a duplicate of this bug. ***
Lauro Moura
Comment 3 2020-12-09 22:21:01 PST
Created attachment 415835 [details] Tentative patch
Lauro Moura
Comment 4 2020-12-09 22:23:01 PST
Comment on attachment 415835 [details] Tentative patch View in context: https://bugs.webkit.org/attachment.cgi?id=415835&action=review > LayoutTests/platform/gtk/fast/scrolling/overflow-scrollable-after-back-expected.txt:4 > +PASS: mouseWheel caused scrolling Not sure if this extra scroll is right, though. The test sends 4 scroll phases (begin, changed, changed, ended), and the 3 first ones happen to trigger the callbacks.
Simon Fraser (smfr)
Comment 5 2020-12-09 23:04:48 PST
Comment on attachment 415835 [details] Tentative patch View in context: https://bugs.webkit.org/attachment.cgi?id=415835&action=review > Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp:168 > + auto scrollingTree = m_scrollingTrees.get(pageID); > + if (!scrollingTree) > + dispatchWheelEventViaMainThread(pageID, wheelEvent, processingSteps); How does GTK work with ENABLE(SCROLLING_THREAD) but no scrolling tree? That combination makes no sense.
Carlos Garcia Campos
Comment 6 2020-12-11 06:45:47 PST
Async scrolling depends on accelerated compositing mode being enabled. In the GTK port, we have an accelerate compositing policy setting, that is ondemand by default, which means we enable AC mode when the page requires to enter in AC mode. Async scrolling requires AC mode to be always enabled, so we only enable it when the AC policy is set to always.
Philippe Normand
Comment 7 2021-01-04 02:16:51 PST
Ping reviewers
Michael Catanzaro
Comment 8 2021-01-04 08:56:29 PST
Hm, I think I'm not the right person to review this. But I understand that all scrolling is totally broken for a month now, so would be good to land. I guess we have not been testing trunk very much in December. :P
Alice Mikhaylenko
Comment 9 2021-01-05 01:37:42 PST
All non-async scrolling* Which means that at the very least it never happens with GTK4, so scrolling worked there the whole time. So if you switch hardware acceleration policy to always and then open a new tab in minibrowser, it will work.
EWS
Comment 10 2021-01-07 07:58:43 PST
Committed r271235: <https://trac.webkit.org/changeset/271235> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415835 [details].
Radar WebKit Bug Importer
Comment 11 2021-01-07 07:59:37 PST
<rdar://problem/72889749>
Fujii Hironori
Comment 12 2021-02-07 18:38:47 PST
This fix isn't perfect. Filed: Bug 221067 – [GTK] Page scrolling by wheel events doesn't work in non-AC mode pages if Hardware Acceleration Policy is changed dynamically from "always" to "never"
Fujii Hironori
Comment 13 2021-02-07 19:30:02 PST
Comment on attachment 415835 [details] Tentative patch View in context: https://bugs.webkit.org/attachment.cgi?id=415835&action=review > Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp:166 > + auto scrollingTree = m_scrollingTrees.get(pageID); You are accessing m_scrollingTrees without locking m_scrollingTreesMutex. Is this OK?
Fujii Hironori
Comment 14 2021-02-15 11:17:28 PST
(In reply to Fujii Hironori from comment #13) > Comment on attachment 415835 [details] > Tentative patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=415835&action=review > > > Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp:166 > > + auto scrollingTree = m_scrollingTrees.get(pageID); > > You are accessing m_scrollingTrees without locking m_scrollingTreesMutex. Is > this OK? r272867 removed the suspicious code.
Philippe Normand
Comment 15 2021-02-15 12:32:41 PST
Thanks! I'll test this, I noticed wheel scrolling wasn't working anymore in the WPE MiniBrowser (and Cog). I'll open a new bug if the issue is still present.
Fujii Hironori
Comment 16 2021-02-15 12:46:10 PST
(In reply to Philippe Normand from comment #15) > Thanks! I'll test this, I noticed wheel scrolling wasn't working anymore in > the WPE MiniBrowser (and Cog). I'll open a new bug if the issue is still > present. r271235 and r272867 are fixes for non-AC mode (the case of no scrolling tree). WPE seems to have a different issue.
Note You need to log in before you can comment on or make changes to this bug.