1458598 - Make layout/reftests/async-scrolling/fixed-pos-scrolled-clip-3.html pass on linux64-qr with APZ enabled

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

+++ This bug was initially created as a clone of Bug #1377187 +++

With WR+APZ enabled, the layout/reftests/async-scrolling/fixed-pos-scrolled-clip-3.html reftest fails. This bug is for the patches that make it pass, without regressing any "earlier" tests.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Part of the problem here is the scrollbar isn't showing up properly, but that's fixed by bug 1451168. The other part of the problem is that we have a situation like so:

DefineScrollFrame (the root scrollframe)
  PushStackingContext with transform (10, 10)
    DefineScrollFrame (the overflow:scroll item)
      <fixed item>

Right now (with the patches in bug 1377187) we ending up using the root scrollframe as the scrollid for the fixed item, which means it gets hoisted out of the transform, and so position of the fixed item is off by 10,10. I verified this by injecting a dummy infinite-rect clip inside the stacking context like so:

DefineScrollFrame (the root scrollframe)
  PushStackingContext with transform (10, 10)
    Dummy Clip
    DefineScrollFrame (the overflow:scroll item)
      <fixed item>

and then using the dummy clip as the scrollid for the fixed item. That fixed the problem but isn't really a great solution because we're going to introduce a ton of dummy clips. Instead we should expose the reference frame clip id that is created by the stacking context and use that as the scrollid for the fixed item.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b67e8ac2a240cbe4a37e11265af08a66fd6090bb

Comment 3

[Kartikaya Gupta (email:[e-mail])]

The three failures don't seem to be specific to the patches that I wrote for this bug; instead they seem to be a result of combining the patches on bug 1377187 and bug 1451168. So I'll have to fuzz them when I land bug 1377187.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

The patches here depend on servo/webrender#2720 which should be in the next WR update.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1458598 - Expose the generated reference frame id for stacking contexts.

This is just plumbing, no functional changes yet.

Review commit: https://reviewboard.mozilla.org/r/241410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241410/

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1458598 - Override scrollframes with their descendant reference frames.

The test case has a fixed item A inside a scrollframe B which is inside
a reference frame C which is inside the root scrollframe D. The
ClipManager code currently uses D's scrollid as the scrolling ancestor
for A, because the gecko display list's ASR is set up that way. However,
we really want to set C as the scrolling ancestor, because otherwise the
item A gets hoisted out of C and the transform from C doesn't get
applied to it. This patch ensures that when we enter C, we install an
override so that anything that would have used D's scrollid ends up
using C's, which results in the correct behaviour.

Review commit: https://reviewboard.mozilla.org/r/241412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241412/

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8972854 [details]
Bug 1458598 - Expose the generated reference frame id for stacking contexts.

https://reviewboard.mozilla.org/r/241410/#review248064

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8972855 [details]
Bug 1458598 - Override scrollframes with their descendant reference frames.

https://reviewboard.mozilla.org/r/241412/#review248068

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8972854 [details]
Bug 1458598 - Expose the generated reference frame id for stacking contexts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241410/diff/1-2/

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8972855 [details]
Bug 1458598 - Override scrollframes with their descendant reference frames.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241412/diff/1-2/

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Rebased on autoland tip: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1f5c47adee1f6e218e16bff86931e4b5d518299

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06170521ac0a
Expose the generated reference frame id for stacking contexts. r=mstange
https://hg.mozilla.org/integration/autoland/rev/0bb3414bd6af
Override scrollframes with their descendant reference frames. r=mstange

Comment 13

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/06170521ac0a
https://hg.mozilla.org/mozilla-central/rev/0bb3414bd6af