1387942 - stylo: file_fullscreen-backdrop.html Assertion failure: backdropFrame->GetParent()->IsViewportFrame() for fullscreen test on Mac

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Xidorn Quan [:xidorn] UTC+11]

I have no idea what's happening there... This assumption should be hold :/ I probably need to investigate what's happening there.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

I tried locally on my mac, and this assertion doesn't show up... so it is somehow try server only :/

Comment 2

[Bobby Holley (:bholley)]

Is this still happening?

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Yes, it's still happening. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c09813a980f0bfc10770db97f57fa74cdf1ff30&selectedJob=130227561

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Oh, actually, this assertion is wrong. Backdrop frame can be parented by either the viewport frame or the canvas frame, depending on whether its position is "fixed" or "absolute". Related code can be found in nsCSSFrameConstructor.cpp [1].

The value of position is "fixed" by default, so this assertion doesn't show up on majority of tests, but in the specific file_fullscreen-backdrop.html test, we try to change the position property to "absolute" and test that it works as expected. In that case, the assertion could be triggered.

But it is not clear to me why this happens only in Mac, and why I cannot reproduce this crash locally.

emilio, do you have idea why it isn't always triggered, and what would go wrong if the assertion is violated?


[1] https://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/layout/base/nsCSSFrameConstructor.cpp#1052-1055

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

This makes me think that it probably should have a slightly higher priority.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> emilio, do you have idea why it isn't always triggered, and what would go
> wrong if the assertion is violated?

No idea off-hand about why this isn't always triggered, I guess it's hard to trigger a restyle of this frame given -moz-top-layer changes also reframe?

Re. what can go wrong, actually nothing in this case (if it was other kind of parent we could be doing unnecessary work, or not). I'd like to keep it given it makes it easier to reason about it, let's relax it.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1387942: Relax backdrop frame parenthood assertion, and re-enable test.

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

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8906918 [details]
Bug 1387942: Relax backdrop frame parenthood assertion, and re-enable test.

https://reviewboard.mozilla.org/r/178654/#review183646

r=me with the test re-enabled at the same time.

::: commit-message-e21a0:4
(Diff revision 1)
> +Bug 1387942: Relax backdrop frame parenthood assertion. r?xidorn
> +
> +MozReview-Commit-ID: 9VP6LgqUZ1F
> +

Please also include the change from https://hg.mozilla.org/try/rev/174fd17c84478d0e7918664da5b2749c99ac7ed8 to re-enable this test.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8906918 [details]
Bug 1387942: Relax backdrop frame parenthood assertion, and re-enable test.

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/874f4d1c9ba1
Relax backdrop frame parenthood assertion, and re-enable test. r=xidorn

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/874f4d1c9ba1