Bug 79951 – [chromium] Mark root layer scrollbars as always opaque to disable blending

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

[Adrienne Walker]

[chromium] Mark root layer scrollbars as always opaque to disable blending

[Adrienne Walker]

Created attachment 129537 [details]
Patch

[James Robinson]

Comment on attachment 129537 [details]
Patch

I do not think overlay root layer scrollbars are always opaque (custom or non-custom). This needs some more checks, methinks.

[James Robinson]

Comment on attachment 129537 [details]
Patch

I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque.  We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens.

[Adrienne Walker]

(In reply to comment #3)
> (From update of attachment 129537 [details])
> I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque.  We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens.

Oh, quite right.  Scrollbar::isOverlayScrollbar is pretty far away from LayerChromium at that point, and it seems like a layering violation to pipe it in.

I suppose the other option is to wait and do this at the ScrollbarLayer level after https://bugs.webkit.org/show_bug.cgi?id=78872 lands.

[James Robinson]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 129537 [details] [details])
> > I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque.  We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens.
> 
> Oh, quite right.  Scrollbar::isOverlayScrollbar is pretty far away from LayerChromium at that point, and it seems like a layering violation to pipe it in.
> 

Ah yeah, that is farther than I thought (I was thinking we could just reach in to the ScrollableArea but we've helpfully abstracted NonCompositedContentHost away from being able to do things like that).

> I suppose the other option is to wait and do this at the ScrollbarLayer level after https://bugs.webkit.org/show_bug.cgi?id=78872 lands.

Coincidentally I've been looking at that patch. That would suck hardcore to have to merge if we need to do that.

Other gross option: pass isOverlay bits in the setViewport call from WebViewImpl.

[Adrienne Walker]

Created attachment 131126 [details]
Patch

[Adrienne Walker]

Created attachment 131127 [details]
Whoops, remove printf

[James Robinson]

Comment on attachment 131127 [details]
Whoops, remove printf

If we pass FrameView* to the scrollbarLayerDidChange function then we can also move the scrollLayerForFrameView() code into that helper too, instead of duplicating it across both of the frameView*ScrollbarLayerDidChange() functions

[Adrienne Walker]

Created attachment 131130 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 131130 [details]
Patch for landing

Rejecting attachment 131130 [details] from commit-queue.

New failing tests:
platform/chromium/virtual/gpu/fast/canvas/canvas-text-alignment.html
Full output: http://queues.webkit.org/results/11892959

[Adrienne Walker]

Comment on attachment 131130 [details]
Patch for landing

This passes locally for me.  Let me try this again.

[WebKit Review Bot]

Comment on attachment 131130 [details]
Patch for landing

Rejecting attachment 131130 [details] from commit-queue.

New failing tests:
platform/chromium/virtual/gpu/fast/canvas/canvas-text-alignment.html
Full output: http://queues.webkit.org/results/11949021

[Adrienne Walker]

Committed r110620: <http://trac.webkit.org/changeset/110620>

[Adrienne Walker]

(In reply to comment #13)
> Committed r110620: <http://trac.webkit.org/changeset/110620>

Failing test was due to https://bugs.webkit.org/show_bug.cgi?id=78529 (and test being disabled on Linux debug).  This patch should make debug and release equivalent and I will rebaseline after this goes through the bots.