[root layer scrolls] fast/scrolling failures |
||||
Issue descriptionsataya.m@samsung.com has volunteered to look into the following test failures: virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-overflow-hidden.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-overried-inherited-visibility-hidden.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-no-overried-inherited-visibility-hidden.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-no-visibility-hidden-child.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-no.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-yes.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-visibility-hidden-child.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-zero-size-and-border.html
,
Apr 15 2017
These cases are passing !!! virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-overflow-hidden.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-overried-inherited-visibility-hidden.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-yes.html virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-visibility-hidden-child.html i shall remove these test cases from the TestExpectations !!!
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f057f40c594d751bc68e5808c66525d0f195c72 commit 6f057f40c594d751bc68e5808c66525d0f195c72 Author: sataya.m <sataya.m@samsung.com> Date: Mon Apr 17 08:26:32 2017 Remove RLS Passing Test Cases from TestExceptions. Removing RLS passing test cases from the test exceptions. BUG= 711474 Review-Url: https://codereview.chromium.org/2820023002 Cr-Commit-Position: refs/heads/master@{#464904} [modify] https://crrev.com/6f057f40c594d751bc68e5808c66525d0f195c72/third_party/WebKit/LayoutTests/TestExpectations
,
Apr 18 2017
Hi Skobes,
Here is todays findings.
started working on this virtual/rootlayerscrolls/fast/scrolling/scrollable-area-frame-scrolling-no-overried-inherited-visibility-hidden.html
Found few bug's with in this test case.
#Bug 1
<iframe width=120 scrolling=no style="visibility: visible;" src="resources/generic-scrollable-content.html"></iframe>
Means iframe scrolling mode is no, means no scrollbars shouldnt be created but.
with RLS : IFrame creates horizontal scrollbar. => Reason.
void PaintLayerScrollableArea::ComputeScrollbarExistence(){
....
....
needs_horizontal_scrollbar = Box().ScrollsOverflowX(); // This condition makes it true.
....
if (Box().IsLayoutView()) {
if (LocalFrame* frame = Box().GetFrame()) {
if (FrameView* frame_view = frame->View()) {
ScrollbarMode h_mode;
ScrollbarMode v_mode;
frame_view->CalculateScrollbarModes(h_mode, v_mode);
/* Bug is here. CalculateScrollbarModes returns h_mode & v_mode
with kScrollbarAlwaysOff as frame owner scrolling mode is off.
but we are not setting needs_horizontal_scrollbar to false,
as h_mode is kScrollbarAlwaysOff, due to which Horizontal Scrollbar
is created.
*/
if (h_mode == kScrollbarAlwaysOn)
needs_horizontal_scrollbar = true;
if (v_mode == kScrollbarAlwaysOn)
needs_vertical_scrollbar = true;
}
}
}
}
Without RLS: IFrame doesnt create horizontal scrollbar => Reason
void FrameView::Init(){
....
....
....
if (frame_->Owner() &&
frame_->Owner()->ScrollingMode() == kScrollbarAlwaysOff)
SetCanHaveScrollbars(false);
}
# Bug 2 .
<iframe width=120 scrolling=no style="visibility: visible;" src="resources/generic-scrollable-content.html"></iframe>
where generic-scrollable-content.html content is just "content content content content .... " without any textarea.
with RLS : Vertical scrollbar is created and we are able to scroll.
without RLS: No Scrollbar is created and we are not able to scroll.
For Bug#2 we can stop creating scrollbars with below fix, setting needs_vertical_scrollbar = false in kScrollbarAlwaysOff mode.
Need pointers on how to deregister from scrolling.
void PaintLayerScrollableArea::ComputeScrollbarExistence(){
....
....
....
if (Box().IsLayoutView()) {
if (LocalFrame* frame = Box().GetFrame()) {
if (FrameView* frame_view = frame->View()) {
ScrollbarMode h_mode;
ScrollbarMode v_mode;
frame_view->CalculateScrollbarModes(h_mode, v_mode);
if (h_mode == kScrollbarAlwaysOn)
needs_horizontal_scrollbar = true;
else if(h_mode == kScrollbarAlwaysOff)
needs_horizontal_scrollbar = false;
if (v_mode == kScrollbarAlwaysOn)
needs_vertical_scrollbar = true;
else if(v_mode == kScrollbarAlwaysOff)
needs_vertical_scrollbar = false;
}
}
}
}
Share your inputs.
Thanks,
MuVen
,
Apr 20 2017
Thanks, I agree with your analysis. PLSA::ComputeScrollbarExistence should be updated to handle the case where FrameView::CalculateScrollbarModes returns kScrollbarAlwaysOff. It looks like "scrolling=no" disables user scrolls by making FrameView::UserInputScrollable return false. I think PLSA::UserInputScrollable should be updated to return false if FrameView::CalculateScrollbarModes returns kScrollbarAlwaysOff (if it is the root layer).
,
Apr 20 2017
I'll be on leave for next 10 days. @srirama, please make the necessary changes along with the test cases and submit it for review (to Skobes).
,
Apr 20 2017
@srirama, Please verify if this patch fixes the above failing test cases !!! If it doesn't fix, debug and update your findings !!!
,
Apr 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31eb41c176f57dbdbf8d5ce4edd45d21a1c9a45c commit 31eb41c176f57dbdbf8d5ce4edd45d21a1c9a45c Author: srirama.m <srirama.m@samsung.com> Date: Wed Apr 26 02:25:48 2017 [RLS] Update iframe scrollbar behaviour based on scrolling attribute Currently in RLS path, scrollbar existence is not updated when "scrolling" attribute is "no". Updated PLSA::ComputeScrollbarExistence and PLSA::UserInputScrollable to consider scrollbar modes to turn of scrolling/scrollbars for root layers. BUG= 711474 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2831313002 Cr-Commit-Position: refs/heads/master@{#467207} [modify] https://crrev.com/31eb41c176f57dbdbf8d5ce4edd45d21a1c9a45c/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/31eb41c176f57dbdbf8d5ce4edd45d21a1c9a45c/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03a08a02a028bdc0042b34cf36d229f02f9ce5fc commit 03a08a02a028bdc0042b34cf36d229f02f9ce5fc Author: sataya.m <sataya.m@samsung.com> Date: Tue Jul 18 18:13:19 2017 [RLS] Don't add to ScrollableAreaSet if the size is zero Add ScrollableArea only if the box size is not zero and there is a horizontal or vertical overflow scroll. BUG= 711474 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2857583007 Cr-Commit-Position: refs/heads/master@{#487525} [modify] https://crrev.com/03a08a02a028bdc0042b34cf36d229f02f9ce5fc/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/03a08a02a028bdc0042b34cf36d229f02f9ce5fc/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp [modify] https://crrev.com/03a08a02a028bdc0042b34cf36d229f02f9ce5fc/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
,
Jul 18 2017
Can we mark this fixed now? Please let us know if you would like us to work on any other issues.
,
Jul 19 2017
Thanks for the patches so far! Here are the remaining failures in fast/scrolling that I think we should look at. (Or if they are passing, we should remove them from TestExpectations.) virtual/rootlayerscrolls/fast/scrolling/scrollbar-tickmarks-styled.html virtual/rootlayerscrolls/fast/scrolling/scrollbar-tickmarks-styled-after-onload.html virtual/rootlayerscrolls/fast/scrolling/scrollbar-prevent-default.html
,
Jul 19 2017
I have debugged first two test cases. Reason looks to be somewhat similar for both cases. These test cases are trying to paint textmatchmarker on top of scrollbar using internals API (internals.addTextMatchMarker). This function is adding the marker and then calling InvalidatePaintForTickmarks() in LocalFrameView.cpp which is trying to invalidate vertical scrollbar using SetNeedsPaintInvalidation(). In case of RLS, this scrollbar is coming NULL. So i changed the code to get the correct scrollbar using LayoutViewportScrollableArea(). Now it is reaching till ScrollableArea::SetScrollbarNeedsPaintInvalidation() and the flags are set properly on graphics layer but still the marker is not displayed. Any inputs for further debugging?
,
Jul 19 2017
I think the tickmarks are painted by ScrollbarTheme::PaintTickmarks, you could try looking there.
,
Jul 20 2017
Thanks for the quick suggestion. I have fixed these test cases now at https://chromium-review.googlesource.com/c/578831 But there is a new test failure text-match-highlight.html It is crashing with the below callstack. Couldn't understand what exactly is this paintunderinvalidationcheck. Probably that also needs to be fixed incase of RLS flow? *************************************************** STDERR: [1:1:0720/013635.711825:48546148475:FATAL:PaintController.cpp(464)] Check failed: false. Can't find cached display item STDERR: #0 0x000001a57617 base::debug::StackTrace::StackTrace() STDERR: #1 0x000001a6e601 logging::LogMessage::~LogMessage() STDERR: #2 0x0000030a0158 blink::PaintController::FindOutOfOrderCachedItemForward() STDERR: #3 0x00000309d6d7 blink::PaintController::FindCachedItem() STDERR: #4 0x00000309d455 blink::PaintController::UseCachedDrawingIfPossible() STDERR: #5 0x0000030e601b blink::ScrollbarTheme::PaintTickmarks() STDERR: #6 0x0000030e5aa3 blink::ScrollbarTheme::Paint() STDERR: #7 0x0000030e3fe3 blink::Scrollbar::Paint() STDERR: #8 0x000003ad6253 blink::ScrollableAreaPainter::PaintOverflowControls() STDERR: #9 0x000003a6a9ba blink::BlockPainter::PaintOverflowControlsIfNeeded() **************************************************************
,
Jul 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8af4ef8b433ae41baae4cea697ad2ce66c530da2 commit 8af4ef8b433ae41baae4cea697ad2ce66c530da2 Author: Munukutla Subrahmanya Praveen <sataya.m@samsung.com> Date: Sat Jul 22 19:09:53 2017 [RLS] Remove scrollbar-prevent-default.html from TestExpectations Removing scrollbar-prevent-default.html from TestExpectations. BUG= 711474 Change-Id: Ia38c40d7cfb3ddb49d607a53a84094f2dd1b4055 Reviewed-on: https://chromium-review.googlesource.com/581327 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: Mu Ven <sataya.m@samsung.com> Cr-Commit-Position: refs/heads/master@{#488864} [modify] https://crrev.com/8af4ef8b433ae41baae4cea697ad2ce66c530da2/third_party/WebKit/LayoutTests/TestExpectations
,
Jul 25 2017
@skobes, Can you share next set of testcases which we can look in to. Thanks,
,
Jul 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/652a75d70e8f275bc795e2bbed301a42820af1b8 commit 652a75d70e8f275bc795e2bbed301a42820af1b8 Author: Sriram <srirama.m@samsung.com> Date: Mon Jul 31 07:47:35 2017 [RLS] Implement GetTickmarks for RootLayerScrolling Currently GetTickmarks is not implemented in PLSA so the tickmarks are not getting painted for RLS flow. Implemented this function now and updated testExpectations. BUG= 711474 Change-Id: I150dfd413d2ae015e7e9e2dad94d1116df049fb3 Reviewed-on: https://chromium-review.googlesource.com/578831 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com> Cr-Commit-Position: refs/heads/master@{#490724} [modify] https://crrev.com/652a75d70e8f275bc795e2bbed301a42820af1b8/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/652a75d70e8f275bc795e2bbed301a42820af1b8/third_party/WebKit/Source/core/frame/LocalFrameView.cpp [modify] https://crrev.com/652a75d70e8f275bc795e2bbed301a42820af1b8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp [modify] https://crrev.com/652a75d70e8f275bc795e2bbed301a42820af1b8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
,
Jul 31 2017
@Skobes, we will not be able to check virtual/rootlayerscrolls/fast/scrolling/scrollbar-prevent-default.html as it is working fine in Linux and fails only in mac and windows. Right now we have only Linux setup. Can you please share other test cases if any?
,
Oct 10 2017
,
Feb 9 2018
,
Feb 9 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by satay...@samsung.com
, Apr 15 2017