Implement RenderLayer::requestScrollPositionUpdate so that we can call requestScrollPositionUpdate more convinient. There's a recursive code path, if we implement it directly. AsyncScrollingCoordinator::requestScrollPositionUpdate AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll ScrollableArea::scrollToOffsetWithoutAnimation ScrollAnimator::scrollToOffsetWithoutAnimation ScrollAnimator::notifyPositionChanged ScrollableArea::setScrollOffsetFromAnimation RenderLayer::requestScrollPositionUpdate AsyncScrollingCoordinator::requestScrollPositionUpdate In order to break the circle, I'll try to call notifyScrollPositionChanged in AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll instead.
Created attachment 389014 [details] Patch
Comment on attachment 389014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389014&action=review > Source/WebCore/ChangeLog:8 > + Test: fast/scrolling/ios/programmativ-scroll-element.html programmatic*
The title of this bug doesn't parse.
Created attachment 389138 [details] Patch
(In reply to Simon Fraser (smfr) from comment #3) > The title of this bug doesn't parse. Done, sorry for the confusion.
Comment on attachment 389138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389138&action=review > Source/WebCore/ChangeLog:9 > + while perform asynchronous programmatic scrolling for overflow elements. In order to break performing* > LayoutTests/fast/scrolling/ios/programmatic-scroll-element-crash.html:35 > + }, `Element async scroll by ${scrollFunction}() shouldn't crash`); What is the "crash" exactly? I'm not sure we have a notion of "crashtests" in WPT (yet). Is the goal of this test to only reproduce the issue rather than to do conformance testing? If so, maybe we should just have a traditional webkit test without assert/promise_test, with the minimal repro code and with the text "This test passes if it does not crash". > LayoutTests/fast/scrolling/ios/resources/scroll-behavior.js:1 > +function observeScrolling(elements, callback) { I guess it's ok, but only resetNode/scrollNode seem to be used? I wonder if we really want all of these or just inline the needed code in the test? Especially if you put only the minimal code in the html file...
Comment on attachment 389138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389138&action=review Hi Fred, Thanks for the review:) >> Source/WebCore/ChangeLog:9 >> + while perform asynchronous programmatic scrolling for overflow elements. In order to break > > performing* Done >> LayoutTests/fast/scrolling/ios/programmatic-scroll-element-crash.html:35 >> + }, `Element async scroll by ${scrollFunction}() shouldn't crash`); > > What is the "crash" exactly? > > I'm not sure we have a notion of "crashtests" in WPT (yet). Is the goal of this test to only reproduce the issue rather than to do conformance testing? If so, maybe we should just have a traditional webkit test without assert/promise_test, with the minimal repro code and with the text "This test passes if it does not crash". Sorry for the confusion. Any programmatic scroll of overflow element will do. This test was from scroll-behavior and reused. I'll write a new for this. >> LayoutTests/fast/scrolling/ios/resources/scroll-behavior.js:1 >> +function observeScrolling(elements, callback) { > > I guess it's ok, but only resetNode/scrollNode seem to be used? I wonder if we really want all of these or just inline the needed code in the test? Especially if you put only the minimal code in the html file... Got it. This file is also from scroll-behavior. We don't need it in the new test.
Created attachment 389342 [details] Patch
The commit-queue encountered the following flaky tests while processing attachment 389342 [details]: imported/w3c/web-platform-tests/IndexedDB/interleaved-cursors-small.html bug 203433 The commit-queue is continuing to process your patch.
Comment on attachment 389342 [details] Patch Clearing flags on attachment: 389342 Committed r255501: <https://trac.webkit.org/changeset/255501>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59063923>