Existing tests use UI scripting, which is clumsy and slow.
Created attachment 363516 [details] patch
Comment on attachment 363516 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=363516&action=review This looks good to me and would definitely allow to write more reliable tests. The only thing I wonder is whether new code should be behind a compilation flag so that it is only enabled for developer builds? > Source/WebCore/testing/Internals.cpp:1713 > + auto& scrollingCoordinator = downcast<AsyncScrollingCoordinator>(*document->page()->scrollingCoordinator()); It seems that the Windows bots don't like this. No idea why... Maybe you need to protect some stuff with ENABLE(ASYNC_SCROLLING)?
Comment on attachment 363516 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=363516&action=review > Source/WebCore/ChangeLog:8 > + It would be useful to be able to test delegated user scrolling without using clumsy and slow UI scripting. slow!? > Source/WebCore/ChangeLog:14 > + This mechanism currently supports subframes and overflow scrolling on iOS. Will it support main frame scrolling, and macOS too? > Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp:221 > +void ScrollingStateScrollingNode::setRequestedDelegatedScrollPositionForTesting(const FloatPoint& requestedDelegatedScrollPositionForTesting) > +{ > + m_requestedDelegatedScrollPositionForTesting = requestedDelegatedScrollPositionForTesting; > + setPropertyChanged(RequestedDelegatedScrollPositionForTesting); > +} I don't think doing this via a scrolling tree commit is the right approach. This will trigger a commit of any dirty state in the scrolling state tree, which may mask bugs. I think we want to simulate a user scroll more directly. On iOS, this should actually scroll the UIScrollVIew, which would trigger the wasScrolledByDelegatedScrolling() code path. On macOS, it would enter the code path that is called form handling wheel events. I think we also want this test mode to disable messaging back to the main thread/UI process. So I think we do need UIScriptController on iOS (and maybe macOS too), in conjunction with something that cuts of the dispatch in ThreadedScrollingTree::scrollingTreeNodeDidScroll() (macOS) and calling the scrolling coordinator in RemoteScrollingTree::scrollingTreeNodeDidScroll (iOS).
> slow!? Yes, because swipe based scrolling needs long wait times stabilize. See the existing tests.
*to stabilize
> I think we want to simulate a user scroll more directly. On iOS, this should > actually scroll the UIScrollVIew, which would trigger the > wasScrolledByDelegatedScrolling() code path. On macOS, it would enter the > code path that is called form handling wheel events. This patch does scroll UIScrollView on iOS triggering wasScrolledByDelegatedScrolling. Mac support would be a separate patch. > I think we also want > this test mode to disable messaging back to the main thread/UI process. Separate API and a separate patch. > So I think we do need UIScriptController on iOS (and maybe macOS too), in > conjunction with something that cuts of the dispatch in > ThreadedScrollingTree::scrollingTreeNodeDidScroll() (macOS) and calling the > scrolling coordinator in RemoteScrollingTree::scrollingTreeNodeDidScroll > (iOS).
(In reply to Antti Koivisto from comment #4) > > slow!? > > Yes, because swipe based scrolling needs long wait times stabilize. See the > existing tests. It could be an instantaneous "scroll to"
UIScriptController already has immediateScrollToOffset(). It just needs a Frame or Document parameter, and some "don't sync" smarts.
(In reply to Simon Fraser (smfr) from comment #8) > UIScriptController already has immediateScrollToOffset(). It just needs a > Frame or Document parameter, and some "don't sync" smarts. Interesting. Does it work for overflow scroll? Does it allow to test UI => Web process sync?
(In reply to Simon Fraser (smfr) from comment #8) > UIScriptController already has immediateScrollToOffset(). It just needs a > Frame or Document parameter, and some "don't sync" smarts. Any UIScriptController based API should probably take a point instead of Frame or Document so it also covers hit testing (can Frames and Documents even be reasonably passed to UIScriptController?).
I'd rather the hit testing be a separate entry point. It's much easier to write a test that says "scroll element E" rathe than figuring out coords for it.
(In reply to Simon Fraser (smfr) from comment #11) > I'd rather the hit testing be a separate entry point. It's much easier to > write a test that says "scroll element E" rathe than figuring out coords for > it. Sure, thus this patch. It is unclear to me how to make DOM objects like Element available to UIScriptController. There are no existing examples. Since it is your baby I think you'll get the honours of implementing this.
Created attachment 364271 [details] Patch
Comment on attachment 364271 [details] Patch Clearing flags on attachment: 364271 Committed r242773: <https://trac.webkit.org/changeset/242773>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48797504>