virtual/android/rootscroller/gesture-scroll-document-not-root-scroller.html fails on WebKit Linux Trusty (dbg) |
|||
Issue descriptionStarted with https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20%28dbg%29/11910 and consistently failing in subsequent runs. Due to r553303. 15:34:30.875 32079 worker/3 virtual/android/rootscroller/gesture-scroll-document-not-root-scroller.html output stderr lines: 15:34:30.875 32079 15:34:30.875 32079 DevTools listening on ws://127.0.0.1:41116/devtools/browser/6d9e88bb-f7f6-45b1-ad99-172f9a82b352 15:34:30.877 31943 [7/3373] virtual/android/rootscroller/gesture-scroll-document-not-root-scroller.html failed unexpectedly (asserts failed) 15:34:30.876 32079 worker/3 virtual/android/rootscroller/gesture-scroll-document-not-root-scroller.html failed: 15:34:30.876 32079 worker/3 asserts failed
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/936338b61aab7784912216f8e7881498805f1025 commit 936338b61aab7784912216f8e7881498805f1025 Author: Max Morin <maxmorin@chromium.org> Date: Thu Apr 26 11:23:07 2018 Revert "Scroll documentElement using LayoutView natively" This reverts commit eebeab8487b2fb74cf048e5e24cf5678856cf44e. Reason for revert: Reverting due to failing test ( crbug.com/836623 ). Original change's description: > Scroll documentElement using LayoutView natively > > When computing the scroll-chain for a gesture, only Elements can be > added to the chain. However, the document element (i.e. <html>) doesn't > have a ScrollableArea itself. Its scrolling is delegated to the > document *node* (i.e. the LayoutView) which cannot be added to the > chain. Thus, we use the documentElement to represent scrolling the > LayoutView. See ScrollManager::RecomputeScrollChain to see this > explicitly in code. > > When actually performing the scroll we need to reverse the above > substitution but we currently have a bug: in Element::NativeApplyScroll > we apply this substitution if we're scrolling the effective root > scroller rather than the document element. If the root scroller API is > enabled and the root scroller is changed to a different element, the > document can no longer be scrolled since it will fail to reverse the > documentElement<->LayoutView substitution and try to use the document > element's ScrollableArea which doesn't exist. > > A similar fix is needed in LogicalScroll for keyboard scrolls. This > complicated logic is encoded in the scroll-chain calculation so I've > rewritten that method to use the scroll chain for consistency. > > An additional change is needed since LayoutView::Scroll uses > LocalFrameView::GetScrollableArea to scroll. In the main frame, this > will be the RootFrameViewport which uses the global root scroller as > the layout viewport. Thus, if the root scroller is set to another > element, scrolling the LayoutView would actually scroll that element. > The global root scroller will always be scrolled using the correct > ScrollableArea via the ViewportScrollCallback so LayoutView shouldn't > be treated specially in this case. In fact, I've removed > LayoutBox::Scroll and its overrides altogether and replaced its > call sites with ScrollableArea directly. > > Bug: 813747 > Change-Id: I9e9a27ff684abf969c27ac152c860778a7e00e39 > Reviewed-on: https://chromium-review.googlesource.com/1018880 > Reviewed-by: Steve Kobes <skobes@chromium.org> > Commit-Queue: David Bokan <bokan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#553303} TBR=bokan@chromium.org,skobes@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. No-Try: true Bug: 813747 , 836623 Change-Id: Iaa92fdb566bec60fcdc2f03e9990fc4aacab0be1 Reviewed-on: https://chromium-review.googlesource.com/1029971 Commit-Queue: Max Morin <maxmorin@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#553981} [delete] https://crrev.com/83be57d40f5c53b09fe79186d1e01eb608759a62/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html [delete] https://crrev.com/83be57d40f5c53b09fe79186d1e01eb608759a62/third_party/WebKit/LayoutTests/rootscroller/keyboard-scroll-document-not-root-scroller.html [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_embedded_object.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_embedded_object.h [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_view.h
,
Apr 26 2018
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9829873bac0d85ef9c4bbe26999b3f989e37ba5 commit d9829873bac0d85ef9c4bbe26999b3f989e37ba5 Author: David Bokan <bokan@chromium.org> Date: Thu Apr 26 16:37:15 2018 [Reland] Scroll documentElement using LayoutView natively When computing the scroll-chain for a gesture, only Elements can be added to the chain. However, the document element (i.e. <html>) doesn't have a ScrollableArea itself. Its scrolling is delegated to the document *node* (i.e. the LayoutView) which cannot be added to the chain. Thus, we use the documentElement to represent scrolling the LayoutView. See ScrollManager::RecomputeScrollChain to see this explicitly in code. When actually performing the scroll we need to reverse the above substitution but we currently have a bug: in Element::NativeApplyScroll we apply this substitution if we're scrolling the effective root scroller rather than the document element. If the root scroller API is enabled and the root scroller is changed to a different element, the document can no longer be scrolled since it will fail to reverse the documentElement<->LayoutView substitution and try to use the document element's ScrollableArea which doesn't exist. A similar fix is needed in LogicalScroll for keyboard scrolls. This complicated logic is encoded in the scroll-chain calculation so I've rewritten that method to use the scroll chain for consistency. An additional change is needed since LayoutView::Scroll uses LocalFrameView::GetScrollableArea to scroll. In the main frame, this will be the RootFrameViewport which uses the global root scroller as the layout viewport. Thus, if the root scroller is set to another element, scrolling the LayoutView would actually scroll that element. The global root scroller will always be scrolled using the correct ScrollableArea via the ViewportScrollCallback so LayoutView shouldn't be treated specially in this case. In fact, I've removed LayoutBox::Scroll and its overrides altogether and replaced its call sites with ScrollableArea directly. Reland Note: Typo in test caused incorrect async call chain in promises. Previously landed patch is PS1 for easy comparison. TBR=skobes@chromium.org Bug: 813747 , 836623 Change-Id: Ie46e6f9142b4a4d815cb1e3cb8a99a338f96796e Reviewed-on: https://chromium-review.googlesource.com/1030055 Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#554049} [add] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html [add] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/WebKit/LayoutTests/rootscroller/keyboard-scroll-document-not-root-scroller.html [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_embedded_object.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_embedded_object.h [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_view.h
,
Apr 26 2018
Arg, still failing...disabling and looking.
,
Apr 26 2018
TestExpectation change is in CQ: https://chromium-review.googlesource.com/c/chromium/src/+/1030954
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17037cda956612a8903ee31cdc4e3e0e7b26620d commit 17037cda956612a8903ee31cdc4e3e0e7b26620d Author: David Bokan <bokan@chromium.org> Date: Fri Apr 27 15:37:44 2018 Disable rootscroller layout test on Linux Debug Bug: 836623 Change-Id: Ice1ff8b15775541812ee6c4d246659e676c3cb5f Reviewed-on: https://chromium-review.googlesource.com/1030954 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#554394} [modify] https://crrev.com/17037cda956612a8903ee31cdc4e3e0e7b26620d/third_party/WebKit/LayoutTests/TestExpectations
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca5d3397f540504dd841bafb9f49290bf9a12d96 commit ca5d3397f540504dd841bafb9f49290bf9a12d96 Author: David Bokan <bokan@chromium.org> Date: Mon Apr 30 15:39:19 2018 Fix rootscroller layout test on slow bots Add a rAF before starting the test. My guess is that on slow bots the the gesture starts before the layers are initialized and pushed to the compositor's acive tree so the entire gesture gets ignored. Bug: 836623 Change-Id: I1c54e31b4fb064926d8ad94da4bb8d9751332067 Reviewed-on: https://chromium-review.googlesource.com/1034010 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#554761} [modify] https://crrev.com/ca5d3397f540504dd841bafb9f49290bf9a12d96/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/ca5d3397f540504dd841bafb9f49290bf9a12d96/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html
,
Apr 30 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by bokan@chromium.org
, Apr 25 2018Status: Assigned (was: Untriaged)