1576514 - Fission assertion failure: false (Two layers that scroll together have different ancestor transforms), at gfx/layers/apz/src/APZCTreeManager.cpp:1173

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: parent.html

Similar to bug 1491000 and bug 1450686, but this assertion happens only with enabling fission that means a test case that causes the assertion has an out-of-process iframe. I am going to attach the parent html here and will attach the child html file in a later comment.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: child.html

To reproduce the assertion you need to specify fission related preferences something like this;

./mach run --setpref "fission.autostart=true" --setpref "fission.oopif.attribute=true" --setpref="gfx.webrender.all=true" parent.html

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)

To reproduce the assertion you need to specify fission related preferences something like this;

./mach run --setpref "fission.autostart=true" --setpref "fission.oopif.attribute=true" --setpref="gfx.webrender.all=true" parent.html

Forgot to say that you also need to scroll by mouse wheel or some such.

Comment 3

[Botond Ballo [:botond]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)

./mach run --setpref "fission.autostart=true" --setpref "fission.oopif.attribute=true" --setpref="gfx.webrender.all=true" parent.html

Are you including the WebRender pref here because it's required for Fission to work? Or just required for this bug to reproduce?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

It's for reproducing this assertion.

Comment 5

[Botond Ballo [:botond]]

Attachment: APZC tree dump

Since we don't have a layers dump with WebRender, I hacked the APZC tree dump to print the scroll nodes' transforms (attached).

The scroll nodes on line 5 and line 11 of the dump share the same APZC (APZC (0x100000002, 2, 2)), but have different transforms ([ 1 0; 0 1; 14 767; ] vs. [ I ]), thus triggering the assertion.

Unfortunately I have no quick ideas on why this might be happening. It will take some digging through the WebRenderCommandBuilder code that produces the WebRenderScrollData and related objects to understand how we end up in this situation.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #5)

Since we don't have a layers dump with WebRender, I hacked the APZC tree dump to print the scroll nodes' transforms (attached).

That sounds like something that might be good to land for debugging future issues.

Comment 7

[Botond Ballo [:botond]]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

(In reply to Botond Ballo [:botond] from comment #5)

Since we don't have a layers dump with WebRender, I hacked the APZC tree dump to print the scroll nodes' transforms (attached).

That sounds like something that might be good to land for debugging future issues.

Yeah, I'm thinking of adding a "WebRender layer dump" of sorts that dumps the scroll nodes in UpdateHitTestingTree and prints many of the same properties we'd print for layers.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:botond, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Chris Peterson [:cpeterson]]

Tentatively tracking APZ Fission bugs for Fission Beta milestone (M7).

Comment 10

[Neha Kochar [:neha]]

Botond, please re-triage this for Fission.

Comment 11

[Botond Ballo [:botond]]

We have recently (in bug 1673492) downgraded this assertion to an NS_ASSERTION as there have been several reports of it firing (in various circumstances) but we are not aware of any user-noticeable effects on the relevant pages.

It's still something we'd like to fix but I don't think it needs to be fixed for M7; I suggest tracking for MVP instead.

Comment 12

[Neha Kochar [:neha]]

M8 is when Fission starts rolling out to Release, so this should get fixed in M8.

Comment 13

[Neha Kochar [:neha]]

Since there are no user-noticeable effect on relevant pages, this can wait till MVP.

Comment 14

[Chris Peterson [:cpeterson]]

After this bug is fixed, we can try to re-enable the browser_pdfjs_saveas.js test in bug 1713911. The test is currently skipped for Fission because it hits this assertion failure:

[browser_pdfjs_saveas.js]
skip-if =
  fission && debug && (os == "win" || os == "mac")  # Bug 1713911 - new Fission platform triage

https://searchfox.org/mozilla-central/rev/a9851cca236e03d088d7528dd27445080475a68e/toolkit/components/pdfjs/test/browser.ini#34,37-38

We might also be able to close these other "Two layers that scroll together have different ancestor transforms" bugs as fixed by this bug:

Bug 1450686
Bug 1491000
Bug 1587369
Bug 1652619

Comment 16

[Chris Peterson [:cpeterson]]

In bug 1713911 comment 9, Hiro suggested:

If this test causes only the APZ's assertion, then we can simply allow it with using SimpleTest.expectAssertions.

Nika recommends that we implement Hiro's suggestion so we can re-enable the browser_pdfjs_saveas.js test for Fission debug tests on Windows and macOS until we fix this assertion failure.

In the meantime, this test bug doesn't need to block shipping Fission, so I will move this bug from Fission MVP to Future.

[browser_pdfjs_saveas.js]
support-files =
  !/toolkit/content/tests/browser/common/mockTransfer.js
skip-if =
  fission && debug && (os == "win" || os == "mac")  # Bug 1713911 - new Fission platform triage

https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test/browser.ini#34-38

Comment 17

[Botond Ballo [:botond]]

Local testing shows that my patch for bug 1719913 fixes the assertion in the original testcase (comment 0 and comment 1).

Comment 18

[Botond Ballo [:botond]]

(In reply to Chris Peterson [:cpeterson] from comment #14)

After this bug is fixed, we can try to re-enable the browser_pdfjs_saveas.js test in bug 1713911.

I did a couple of Try pushes to verify this:

• This push has browser_pdfjs_saveas.js re-enabled, but not the fix for bug 1719913. In the logs of the Windows bc5 test chunk (which includes this test), I see the assertion (although it does not actually cause the test job to fail...)
• This push has the test re-enabled and also the fix for bug 1719913, and I no longer see the assertion in the logs.

So, I think that after bug 1719913 lands, we can land a patch here to re-enable that test.

Comment 19

[Botond Ballo [:botond]]

Attachment: Bug 1576514 - Re-enable browser_pdfjs_saveas.js for Fission. r=hiro

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a99c3db316ce
Re-enable browser_pdfjs_saveas.js for Fission. r=hiro

Comment 21

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/a99c3db316ce

Comment 22

[Chris Peterson [:cpeterson]]

Setting status-firefox92=wontfix because we don't need to uplift this Fission test fix to Beta.