Description
Frédéric Wang (:fredw)
2017-06-05 01:23:49 PDT
Created attachment 312000 [details]
Patch
Comment on attachment 312000 [details] Patch Attachment 312000 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3875178 New failing tests: tiled-drawing/scrolling/frames/coordinated-frame-gain-scrolling-ancestor.html tiled-drawing/scrolling/frames/fixed-inside-frame.html tiled-drawing/scrolling/frames/coordinated-frame.html tiled-drawing/scrolling/frames/coordinated-frame-lose-scrolling-ancestor.html tiled-drawing/scrolling/frames/coordinated-frame-in-fixed.html Created attachment 312001 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 312000 [details] Patch Attachment 312000 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3875166 New failing tests: fast/scrolling/ios/remove-scrolling-role.html Created attachment 312002 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Created attachment 312004 [details]
Patch
@Simon: Can you please review that patch? Created attachment 313395 [details] Patch (test for iframes boxes) > 2017/06/14 > 20:18:52 - smfr : so let’s go forward with the parent offset thing in the scrolling tree > 20:19:05 - smfr : we’ll also need to be careful that we’re handling events in teh correct rectangle for each scroller > 20:19:16 - smfr : not sure if the scroller bounds that we have now are the right rect > 20:19:27 - smfr : e.g. test with iframes which have margin,border and padding OK, I finally had time to check that. Currently, iframe padding box (including scroll bars) is the area that can receive mouse wheel event. However, in attachment 312321 [details] I'm using scrollableAreaSize which is only the iframe padding box (excluding scroll bars). Moreover, the offset calculated here does not take into account the margin+border of the iframe. Other values like (visible) contents size do not seem to be what we want. Hence it seems we'll have to add more parameters or to adjust the calculation. I'm attaching a simple HTML test with its dumped scrolling tree. Comment on attachment 312004 [details]
Patch
I think we should store a rect for each node (to handle borders/padding correctly), and I guess each rect could be relative to its parent node?
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 312004 [details] > Patch > > I think we should store a rect for each node (to handle borders/padding > correctly), and I guess each rect could be relative to its parent node? So you are suggesting to replace this "offset in parent" property with a rect, right? Right, yes. Created attachment 320018 [details]
Patch (WIP)
Comment on attachment 320018 [details] Patch (WIP) Attachment 320018 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4464558 New failing tests: fast/scrolling/scrolling-tree-iframe-boxes.html Created attachment 320029 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 320018 [details] Patch (WIP) Attachment 320018 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4464607 New failing tests: fast/scrolling/scrolling-tree-iframe-boxes.html Created attachment 320031 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 320018 [details] Patch (WIP) Attachment 320018 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4464661 New failing tests: fast/scrolling/scrolling-tree-iframe-boxes.html Created attachment 320033 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 320018 [details] Patch (WIP) Attachment 320018 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4464726 New failing tests: fast/scrolling/scrolling-tree-includes-frame.html fast/scrolling/scrolling-tree-iframe-boxes.html fast/scrolling/ios/remove-scrolling-role.html Created attachment 320036 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Created attachment 320273 [details]
Patch (WIP)
Created attachment 320541 [details]
Patch
Created attachment 320626 [details]
Patch
Comment on attachment 320626 [details] Patch Attachment 320626 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4532433 New failing tests: tiled-drawing/scrolling/frames/coordinated-frame-gain-scrolling-ancestor.html Created attachment 320630 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 320626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320626&action=review > Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:45 > + RectInParent, Maybe we call this ParentRelativeScrollableRect, since it's only the part that should respond to scroll events. > Source/WebCore/rendering/RenderLayerCompositor.cpp:3776 > + result.rectInParent.moveBy(currLayer->location()); I think this is too naive. It should use something like RenderLayer::convertToLayerCoords(), which may need to change to say if there were any intermediate transforms (we may have existing code like this somewhere). (In reply to Simon Fraser (smfr) from comment #27) > > Source/WebCore/rendering/RenderLayerCompositor.cpp:3776 > > + result.rectInParent.moveBy(currLayer->location()); > > I think this is too naive. It should use something like > RenderLayer::convertToLayerCoords(), which may need to change to say if > there were any intermediate transforms (we may have existing code like this > somewhere). I thought we wanted to ignore transforms in a first step, but IIUC at the end we would need to calculate the matrix (product of transforms) and rect (maybe just its size)? (In reply to Frédéric Wang (:fredw) from comment #28) > (In reply to Simon Fraser (smfr) from comment #27) > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:3776 > > > + result.rectInParent.moveBy(currLayer->location()); > > > > I think this is too naive. It should use something like > > RenderLayer::convertToLayerCoords(), which may need to change to say if > > there were any intermediate transforms (we may have existing code like this > > somewhere). > > I thought we wanted to ignore transforms in a first step, but IIUC at the > end we would need to calculate the matrix (product of transforms) and rect > (maybe just its size)? I think we should handle translations and scales (i.e. axis-aligned transforms). If any frame is transformed in other ways, we need to throw the entire page into slow scrolling (or do something smarter with bounding rects). (In reply to Simon Fraser (smfr) from comment #29) > I think we should handle translations and scales (i.e. axis-aligned > transforms). Ah, right I remember now. OK, we should handle those here. > If any frame is transformed in other ways, we need to throw the > entire page into slow scrolling (or do something smarter with bounding > rects). I had opened bug 173354 for that. Created attachment 320918 [details]
Patch
Created attachment 321814 [details]
Patch
Small modification to use RenderLayer::convertToLayerCoords
Created attachment 321815 [details]
Patch
Created attachment 325353 [details]
Patch
Rebasing...
Created attachment 355484 [details]
Patch
Rebasing. Some tests are flacky, with "reachable contents size" being randomly output or not. Need to debug this...
Created attachment 355485 [details]
Patch
Comment on attachment 355485 [details] Patch Attachment 355485 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10118906 New failing tests: tiled-drawing/scrolling/frames/fixed-inside-frame.html Created attachment 355512 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 355485 [details] Patch Attachment 355485 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/10119003 New failing tests: webanimations/leak-document-with-web-animation.html fast/scrolling/scrolling-tree-iframe-parent-relative-scrollable-rect.html Created attachment 355513 [details]
Archive of layout-test-results from ews201 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Created attachment 355514 [details]
Patch
Created attachment 355517 [details]
Patch
Comment on attachment 355517 [details] Patch Attachment 355517 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/10120465 New failing tests: webanimations/leak-document-with-web-animation.html fast/scrolling/scrolling-tree-iframe-parent-relative-scrollable-rect.html Created attachment 355526 [details]
Archive of layout-test-results from ews201 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 355517 [details] Patch Attachment 355517 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10120384 New failing tests: fast/scrolling/ios/change-scrollability-on-content-resize-nested.html fast/scrolling/ios/remove-scrolling-role.html media/no-fullscreen-when-hidden.html fast/scrolling/ios/change-scrollability-on-content-resize.html Created attachment 355527 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 355633 [details]
Patch
Comment on attachment 355633 [details] Patch Attachment 355633 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10157233 New failing tests: media/no-fullscreen-when-hidden.html Created attachment 355669 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
I've been working on some code to update the scrolling tree during a new GraphicsLayer tree walk post-flush in RenderLayerCompositor (though I'm also toying with the idea of doing it during RenderLayerCompositor::updateBackingAndHierarchy()). The advantage is that we'll be able to keep track of the scrolling-tree ancestor, and maybe compute geometry during that existing walk. So it would be great if you could split this patch and upload a patch with just the scrolling tree changes to include the parent-relative scrollable rect. (In reply to Simon Fraser (smfr) from comment #50) > I've been working on some code to update the scrolling tree during a new > GraphicsLayer tree walk post-flush in RenderLayerCompositor (though I'm also > toying with the idea of doing it during > RenderLayerCompositor::updateBackingAndHierarchy()). The advantage is that > we'll be able to keep track of the scrolling-tree ancestor, and maybe > compute geometry during that existing walk. > Great to hear that. > So it would be great if you could split this patch and upload a patch with > just the scrolling tree changes to include the parent-relative scrollable > rect. Sure. So IIUC, you mean removing the changes to inRenderLayerCompositor* as well as the associated LayoutTests update? (In reply to Frédéric Wang (:fredw) from comment #51) > (In reply to Simon Fraser (smfr) from comment #50) > > I've been working on some code to update the scrolling tree during a new > > GraphicsLayer tree walk post-flush in RenderLayerCompositor (though I'm also > > toying with the idea of doing it during > > RenderLayerCompositor::updateBackingAndHierarchy()). The advantage is that > > we'll be able to keep track of the scrolling-tree ancestor, and maybe > > compute geometry during that existing walk. > > > > Great to hear that. > > > So it would be great if you could split this patch and upload a patch with > > just the scrolling tree changes to include the parent-relative scrollable > > rect. > > Sure. So IIUC, you mean removing the changes to inRenderLayerCompositor* as > well as the associated LayoutTests update? Right. Created attachment 355872 [details]
Patch
Extracting only the set/get/dump APIs.
Created attachment 355873 [details]
Patch (tentative layout tests adjustments)
@smfr: I've extracted relevant changes in attachment 355872 [details] (code) and attachment 355873 [details] (layout tests). The test failure on iOS seems unrelated, see bug 192088 Comment on attachment 355872 [details] Patch Rejecting attachment 355872 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 355872, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=355872&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=172914&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 355872 from bug 172914. Fetching: https://bugs.webkit.org/attachment.cgi?id=355872 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... M Source/WebCore/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/Source/WebCore/ChangeLog' is out of date W: 73100f3605242fdc3c69d247b9ab0bbf1b71ca01 and refs/remotes/origin/master differ, using rebase: :040000 040000 065eee271eac953bd2a6b0d881d0a2e1a1b94ac8 0e9cf8475f83a4372cc2a8703579faac5683073a M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... M Source/WebCore/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/Source/WebCore/ChangeLog' is out of date W: 73100f3605242fdc3c69d247b9ab0bbf1b71ca01 and refs/remotes/origin/master differ, using rebase: :040000 040000 065eee271eac953bd2a6b0d881d0a2e1a1b94ac8 0e9cf8475f83a4372cc2a8703579faac5683073a M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource From https://git.webkit.org/git/WebKit 25552c067c9..cb3f7e0b42d master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 238638 = 25552c067c9607b197375e5b239990c1cbc021c2 r238639 = c0ebda636f1d838e44ce2d48ed4a5774d2053adf r238640 = cb3f7e0b42d592cea3ba7e26e2a895fd0098aacc Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc M Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm M Tools/ChangeLog r238641 = 0dfb93794b73475ceee7a825335cd99ba8b1ab6a (refs/remotes/origin/master) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: https://webkit-queues.webkit.org/results/10186730 Created attachment 355997 [details]
Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 355997 [details]: inspector/model/remote-object-api.html bug 192144 (author: bburg@apple.com) media/modern-media-controls/media-documents/media-document-invalid.html bug 192145 (author: graouts@apple.com) The commit-queue is continuing to process your patch. The commit-queue encountered the following flaky tests while processing attachment 355997 [details]: http/tests/webgl/1.0.2/texSubImage2DHTML.html bug 192146 (author: roger_fong@apple.com) The commit-queue is continuing to process your patch. Comment on attachment 355997 [details] Patch for landing Clearing flags on attachment: 355997 Committed r238665: <https://trac.webkit.org/changeset/238665> All reviewed patches have been landed. Closing bug. |