Squashed fixed position layers shouldn't flicker while scrolling

Squashed fixed position layers shouldn't flicker while scrolling

We were correctly squashing fixed position layers together, but we weren't
setting the correct position constraint on the squashing layer. Instead, we
were setting the constraint on the main graphics layer directly, which meant
the compositor incorrectly scrolled the squashed fixed position layer instead
of keeping it stuck to the viewport.

R=vollick@chromium.org
BUG=402819
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180225

[abarth-chromium, 2014-08-13 22:06:27]

+chrishtr because this touches on squashing

[Ian Vollick, 2014-08-13 22:29:20]

Cool. LGTM.

https://codereview.chromium.org/475553002/diff/1/Source/core/page/scrolling/S...
File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):

https://codereview.chromium.org/475553002/diff/1/Source/core/page/scrolling/S...
Source/core/page/scrolling/ScrollingCoordinator.cpp:215: GraphicsLayer*
mainLayer = compositedLayerMapping->childForSuperlayers();
I really like this. I think that localRootForOwningLayer came from a time when
we'd thought that the squashed layers could scroll wrt to the owning layer and
that's now impossible.

[abarth-chromium, 2014-08-13 22:32:49]

I think there's a further improvement we can make in
FrameView::scrollContentsFastPath after this CL lands, but I'll do that
separately:

        if (layer->compositingState() == PaintsIntoOwnBacking)
            continue;

That should probably understand that grouped backings don't need repaints
either.

That code is also busted because it assumes that all the
ViewportConstrainedObjectSet scroll with the viewport, which is false.  :)

[abarth-chromium, 2014-08-13 22:33:01]

The CQ bit was checked by abarth@chromium.org

[commit-bot: I haz the power, 2014-08-13 22:34:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/abarth@chromium.org/475553002/1

[abarth-chromium, 2014-08-13 23:07:27]

The CQ bit was checked by abarth@chromium.org

[commit-bot: I haz the power, 2014-08-13 23:08:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/abarth@chromium.org/475553002/20001

[commit-bot: I haz the power, 2014-08-14 03:33:44]

Committed patchset #2 (20001) as 180225

[chrishtr, 2014-08-18 23:09:52]

Did a test come in separately?

[abarth-chromium, 2014-08-18 23:27:35]

On 2014/08/18 at 23:09:52, chrishtr wrote:
> Did a test come in separately?

I couldn't figure out how to construct a test for this case.  The problem is
that the main thread comes in and fixes things, so you need to somehow see the
state of the world after the compositor moves things around but before the main
thread fixes the problem.

Do you have any thoughts on an approach for a test?  I'm happy to write one if
you have some ideas.

[chrishtr, 2014-08-19 21:28:42]

On 2014/08/18 at 23:27:35, abarth wrote:
> On 2014/08/18 at 23:09:52, chrishtr wrote:
> > Did a test come in separately?
> 
> I couldn't figure out how to construct a test for this case.  The problem is
that the main thread comes in and fixes things, so you need to somehow see the
state of the world after the compositor moves things around but before the main
thread fixes the problem.
> 
> Do you have any thoughts on an approach for a test?  I'm happy to write one if
you have some ideas.

I agree it's hard. No great ideas from me.