1712491 - Crash on a codepen demo [@ wr_moz2d_render_cb ]

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Mayank Bansal]

Go to https://codepen.io/mandymichael/pen/yLabZPr

drag-and-resize the css/html/JS code pane
ER: no crash
AR: crash

https://crash-stats.mozilla.org/report/index/5716586d-d94e-45b6-9b66-4bce20210523
https://crash-stats.mozilla.org/report/index/bf5f5e1a-bef4-4e8a-b5e5-1b8590210523
https://crash-stats.mozilla.org/report/index/c28a17be-16fb-4afc-8581-965a10210523
https://crash-stats.mozilla.org/report/index/0f5c5ac5-bfaa-4ab6-83fb-5777e0210523

Comment 1

[Mayank Bansal]

2021-05-23T12:56:53.452000: INFO : platform_repository: https://hg.mozilla.org/integration/autoland
2021-05-23T12:56:53.452000: INFO : platform_version: 90.0a1
2021-05-23T12:57:21.486000: INFO : Narrowed integration regression window from [3d958d70, c1a29b26] (3 builds) to [d3b02e2c, c1a29b26] (2 builds) (~1 steps left)
2021-05-23T12:57:21.499000: DEBUG : Starting merge handling...
2021-05-23T12:57:21.499000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=c1a29b263709319c149950a118448ad084a00120&full=1
2021-05-23T12:57:21.499000: DEBUG : redo: attempt 1/3
2021-05-23T12:57:21.499000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=c1a29b263709319c149950a118448ad084a00120&full=1',), kwargs: {}, attempt #1
2021-05-23T12:57:21.504000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2021-05-23T12:57:23.621000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=c1a29b263709319c149950a118448ad084a00120&full=1 HTTP/1.1" 200 None
2021-05-23T12:57:23.700000: DEBUG : Found commit message:
Bug 1548056 - Use nsDisplayItem::Paint and DLBI to paint fallback filters instead of FrameLayerBuilder. r=jrmuizel

Differential Revision: https://phabricator.services.mozilla.com/D115321

2021-05-23T12:57:23.700000: DEBUG : Did not find a branch, checking all integration branches
2021-05-23T12:57:23.700000: INFO : The bisection is done.
2021-05-23T12:57:23.700000: INFO : Stopped

Comment 2

[Mayank Bansal]

Attachment: aboutsupport

Comment 3

[Matt Woodrow (:mattwoodrow)]

Andrew, do you have any ideas what would cause this?

The failure is "Replay failure: ExternalSourceSurfaceCreation PLAY". Looks like we had instances of this happening before my changes, but does look like it might be more common (and we now have a testcase for this specific case).

The testcase is using an image inside of an SVG filter that we paint using a fallback blob (and my changes are to the way that blob is recorded).

This looks rather similar to bug 1516011, but as far as I can tell we are still using TakeExternalSurfaces in the fallback blob path to hang on to any shared image references long enough.

Also, should we consider not crashing the whole browser for these errors (on platforms without a GPU process)?

Comment 4

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

Set release status flags based on info from the regressing bug 1548056

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

I'm writing a patch to provide better guarantees about the lifetimes of the surfaces.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

With my patch in bug 1712803, the problem is still not solved for the test case. It appears we are still referencing images during rasterization that the blob supposedly no longer needs. Disabling partial invalidation of the blobs makes the issue go away, so it may be related to the partial recordings missing out on dependencies, or the merging itself is wrong.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

I think we hit:

https://searchfox.org/mozilla-central/rev/0adc9c8175bcc7cbc1da6adbfe40f9fe18836ac5/gfx/layers/wr/WebRenderCommandBuilder.cpp#786

where the invalid rect doesn't contain the container, but we still repaint it because we want to add all of the item bounds to the recording index to be able to merge the blobs:

https://searchfox.org/mozilla-central/rev/0adc9c8175bcc7cbc1da6adbfe40f9fe18836ac5/gfx/layers/wr/WebRenderCommandBuilder.cpp#799

Then we try to paint the filter:

https://searchfox.org/mozilla-central/rev/0adc9c8175bcc7cbc1da6adbfe40f9fe18836ac5/gfx/layers/wr/WebRenderCommandBuilder.cpp#996

And we get a different external image on the subsequent paint. This changes the recording, but it isn't in the invalid rect, so we don't actually merge the change in:

https://searchfox.org/mozilla-central/rev/0adc9c8175bcc7cbc1da6adbfe40f9fe18836ac5/gfx/webrender_bindings/src/moz2d_renderer.rs#466

As a result, we still try to rasterize with the old images which we may have already released.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1712491 - Avoid re-recording parts of blob images which haven't changed.

When we update a blob image recording because of a visibility change,
the invalid rect may be empty. We still need to iterate over all of the
items in the blob for merging purposes, to include their metadata (i.e.
the bounds) in the recording. We should not attempt to re-record any
elements that are not within the invalid rect because that may change
our dependencies on external images and/or fonts. This patch fixes how
transform and filter display items would be re-recorded without being
invalidated.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3fdb5b163962
Avoid re-recording parts of blob images which haven't changed. r=jrmuizel

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3fdb5b163962

Comment 11

[Mayank Bansal]

This is fixed in latest nightly

Comment 12

[Maria Berlinger [:mberlinger], Services QA]

Hello,
I’ve managed to reproduce this issue using Firefox 90.0a1 (2021-05-23) on Ubuntu 20.04x64 by following the STR from comment 0.
This is verified fixed using Firefox 90.0b4 on the following OSes: Windows 10x64, Ubuntu 20.04x64 and macOS 11.3.

Comment 13

[Wayne Mery (:wsmwk)]

#4 crash for Thunderbird 90.0b1. 99% Mac. For example bp-9e64f88d-efad-47a9-acc2-b2d590210610. Does it match to this bug?