Bug 194433

Created attachment 361500 [details] patch

Created attachment 361501 [details] test case with busy main thread

Created attachment 361502 [details] test case with busy main thread

Created attachment 361503 [details] patch

Created attachment 361504 [details] patch

Attachment 361504 [details] did not pass style-queue: ERROR: LayoutTests/ChangeLog:12: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 1 in 26 files If any of these errors are false positives, please file a bug against check-webkit-style.

Does this fix <rdar://problem/47755552>?

Created attachment 361515 [details] patch

(In reply to Simon Fraser (smfr) from comment #7) > Does this fix <rdar://problem/47755552>? No, this patch doesn't affect the main frame. That still only updates the scroll position on layer tree commit (not synchronizing to UIScrollView) and may use stale position. It is bit of a separate issue and probably better handled separately.

Comment on attachment 361515 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=361515&action=review > Source/WebCore/ChangeLog:12 > + This patch removes code that that passes the last committed scroll position via the state tree and modifies its only > + client (in ScrollingTreeScrollingNodeDelegateIOS) to use the UIScrollView scrolling position. How does this work with FixedPositionViewportConstraints::layerPositionForViewportRect((), which uses viewportRectAtLastLayout and layerPositionAtLastLayout, which you haven't changed? ScrollingTreeFrameScrollingNode::fixedPositionRect() uses the committed scroll position, which should match viewportRectAtLastLayout. Or maybe these are never used in the same code path?

Comment on attachment 361515 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=361515&action=review > Source/WebCore/ChangeLog:14 > + Test by Frederic Wang. @Antii: Now that you fixed bug 194886, please don't forget to import the three remaining tests from programmatic-scroll-iframe.html, attachment 359723 [details]: // This checks scrolling behavior for position "fixed" and "sticky". document.getElementById("positionFixed").contentWindow.window.scrollTo(100, 100); document.getElementById("positionStickyBegin").contentWindow.window.scrollTo(0, 50); document.getElementById("positionStickyEnd").contentWindow.window.scrollTo(0, 150);

Created attachment 362606 [details] Patch (tests) Just moving my old tests here, so that they are not lost.

Created attachment 362990 [details] Patch (tests) Tentative to split new tests into independent files.

Comment on attachment 362990 [details] Patch (tests) These all pass other than fast/scrolling/ios/programmatic-scroll-iframe-012.html which times out.

(In reply to Simon Fraser (smfr) from comment #14) > Comment on attachment 362990 [details] > Patch (tests) > > These all pass other than > fast/scrolling/ios/programmatic-scroll-iframe-012.html which times out. Do you know if it's a bug in our code (e.g. bug 196290) or a bug in the test? Also, do we still want to take these tests now that we have new APIs?

Comment on attachment 362990 [details] Patch (tests) View in context: https://bugs.webkit.org/attachment.cgi?id=362990&action=review > LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe-012.html:38 > + </body>" onload="newFrameLoaded()"> OK, this is the silly mistake: it should be runTest()

(In reply to Frédéric Wang (:fredw) from comment #15) > (In reply to Simon Fraser (smfr) from comment #14) > > Comment on attachment 362990 [details] > > Patch (tests) > > > > These all pass other than > > fast/scrolling/ios/programmatic-scroll-iframe-012.html which times out. > > Do you know if it's a bug in our code (e.g. bug 196290) or a bug in the test? > Also, do we still want to take these tests now that we have new APIs? I'll move the tests to bug 196394 as it seems we no longer need Antti's patch to make them pass.