Bug 78008 – Investigate rubberbanding/transform-overhang-[e|se|s].html failures

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

These are the remaining rubberbanding / compositing test failures not fixed by https://bugs.webkit.org/show_bug.cgi?id=78007

The code in ScrollView::calculateOverhangAreasForPainting() is not producing any overhang areas because  contentsWidth() and contentsHeight() are both 0.

Those checks is there to prevent a "full overhang" bug happening when the page is loading. It does make me think these are related to how the layout tests are setup for the composited case, since we're not seeing the same effect for the non-composited versions of these tests.

The issue is that in the Composited path, the overhang layer was only getting enabled/disabled in ScrollView::scrollContents().

However, the order of things was such that the scrolling took place before the size had been set, at which time overhang was rightfully calculated to not exist. However, there was no update to this when the size was finally set.

The solution is to re-calculate overhang bounds on size change in addition to scrolling.

Created attachment 127583 [details]
Patch

James, can you review?

Ping.

[James Robinson]

Comment on attachment 127583 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127583&action=review

Seems OK, but this weird mix of ScrollView/FrameView/RenderLayerCompositor for overhang areas is really annoying to try to understand and debug.

On mac !CHROMIUM, the ScrollbarTheme sets up the overhang area layer to just be a tiled pattern and the shadow layer to be some sort of magic path-based layer. we don't support either of those in our compositor today, but I think we could with not too much effort. I think we'd be much better off using an approach like that rather than manually repainting + reuploading this overhang layer all the time.  I suspect you won't want to work on improving this, but would you mind filing some bugs to improve this?  I'm not super confident in our ability to maintain the current code.

> Source/WebCore/platform/ScrollView.h:285
> +    void updateOverhangAreas();

this should be private

calculate...() probably should be private as well

Created attachment 128228 [details]
Patch

(In reply to comment #6)
> (From update of attachment 127583 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127583&action=review
> 
> Seems OK, but this weird mix of ScrollView/FrameView/RenderLayerCompositor for overhang areas is really annoying to try to understand and debug.
> 
> On mac !CHROMIUM, the ScrollbarTheme sets up the overhang area layer to just be a tiled pattern and the shadow layer to be some sort of magic path-based layer. we don't support either of those in our compositor today, but I think we could with not too much effort. I think we'd be much better off using an approach like that rather than manually repainting + reuploading this overhang layer all the time.  I suspect you won't want to work on improving this, but would you mind filing some bugs to improve this?  

Filed: https://bugs.webkit.org/show_bug.cgi?id=79251

> > Source/WebCore/platform/ScrollView.h:285
> > +    void updateOverhangAreas();
> 
> this should be private
> 
> calculate...() probably should be private as well

Done.

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 128228 [details]:

storage/indexeddb/index-count.html bug 79266 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

Comment on attachment 128228 [details]
Patch

Clearing flags on attachment: 128228

Committed r108536: <http://trac.webkit.org/changeset/108536>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.