1450307 - Crash in mozilla::layers::WebRenderCommandBuilder::GenerateFallbackData

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-f95e124e-af50-48cf-b7aa-5149f0180330.
=============================================================

Seen while looking at Mac crash stats: https://bit.ly/2GZmsEC

Top 10 frames of crashing thread:

0 XUL mozilla::layers::WebRenderCommandBuilder::GenerateFallbackData gfx/layers/wr/IpcResourceUpdateQueue.cpp:267
1 XUL mozilla::layers::WebRenderCommandBuilder::BuildWrMaskImage gfx/layers/wr/WebRenderCommandBuilder.cpp:1758
2 XUL nsDisplayMask::CreateWebRenderCommands layout/painting/nsDisplayList.cpp:9646
3 XUL mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList gfx/layers/wr/WebRenderCommandBuilder.cpp:1298
4 XUL mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands gfx/layers/wr/WebRenderCommandBuilder.cpp:1127
5 XUL mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer gfx/layers/wr/WebRenderLayerManager.cpp:271
6 XUL nsDisplayList::PaintRoot layout/painting/nsDisplayList.cpp:2654
7 XUL nsLayoutUtils::PaintFrame layout/base/nsLayoutUtils.cpp:4011
8 XUL mozilla::PresShell::Paint layout/base/PresShell.cpp:6336
9 XUL nsViewManager::ProcessPendingUpdatesPaint view/nsViewManager.cpp:480

=============================================================

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1450307 - Round scaled paint size instead of paint bounds in GenerateFallbackData.

A component of dtSize could be zero because RoundedToInt(rect).Size() is not the same as
RoundedToInt(rect.Size()). The size after rounding might be zero, even if the rounded size
is not zero. For example with x0=0.5, x1=1.1.

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

Comment 2

[Ryan Hunt [:rhunt]]

Funny, I just started getting this crash and was able to track down the cause I was seeing. I'm not super familiar with this code, so if the variable name isn't right I can rename it.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8964417 [details]
Bug 1450307 - Round scaled paint size instead of paint bounds in GenerateFallbackData.

https://reviewboard.mozilla.org/r/233148/#review238950

Comment 4

[Markus Stange [:mstange]]

Is that actually doing the correct thing? Or should we snap the edges and then check the snapped rect again for emptiness?

Comment 5

[Ryan Hunt [:rhunt]]

(In reply to Markus Stange [:mstange] from comment #4)
> Is that actually doing the correct thing? Or should we snap the edges and
> then check the snapped rect again for emptiness?

That sounds reasonable to me, but I don't know exactly how the resulting blog image is rendered or our pixel snapping requirements.

Also for reference the animation at the bottom of this page will trigger this assertion when the box's size is zero. [1]

[1] https://developer.mozilla.org/en-US/docs/Web/SVG/Element/animate

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I don't have a good idea what's actually correct. Rounding the size feels more correct to me, but I don't have any kind of argument.

Comment 7

[Markus Stange [:mstange]]

Sounds like somebody needs to create a testcase that would show different behavior with the two solutions, and then check what non-webrender Firefox does in that testcase.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

FYI I can always reproduce this crash with a recent Nightly build when selecting a pinned tab which has https://github.com/notifications as location.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa8bcc37d888
Round scaled paint size instead of paint bounds in GenerateFallbackData. r=jrmuizel

Comment 17

[Darkspirit]

I grabbed bad and good from https://hg.mozilla.org/integration/mozilla-inbound/graph/cc6cd0a77a35:
bad  @ 372811caffc6 "Bug 1444392 - Part 1 - Add test-only helpers to open and close the main menu. r=Gijs"
good @ cc6cd0a77a35 "Bug 1446503 - Place 3 pane toggle button next to sidebar tabs in the inspector. r=Honza"

Checking if the crash from bug 1447076 comment 13 is gone.
mozregression --find-fix --repo mozilla-inbound --bad 372811caffc6 --good cc6cd0a77a35 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://www.qualcomm.com/products/server-processors"
> 4:53.24 INFO: First good revision: aa8bcc37d8884c02d34d547308c355f4a7f72624
> 4:53.24 INFO: Last bad revision: 864320860e94d0dd3a8f05976601ed6a7dc5a79d
> 4:53.24 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=864320860e94d0dd3a8f05976601ed6a7dc5a79d&tochange=aa8bcc37d8884c02d34d547308c355f4a7f72624

> aa8bcc37d888	Ryan Hunt — Bug 1450307 - Round scaled paint size instead of paint bounds in GenerateFallbackData. r=jrmuizel

Thanks! :)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9436102afb18
Mark tests as fuzzy. r=me on a CLOSED TREE

Comment 19

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbee7ebc2fce
Backed out 2 changesets for reftest failures on fieldset-columns-001.html. CLOSED TREE

Comment 20

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

Can we just add a check for dtSize being zero and bail out at https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/gfx/layers/wr/WebRenderCommandBuilder.cpp#1615 if that's the case? That should fix the crashing without changing any behaviour, and then we can figure out what we want the behaviour to be.

Comment 21

[Cosmin Sabou [:CosminS]]

Backed out for reftest failures on fieldset-columns-001.html

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/bbee7ebc2fce7e19391f09a6eaf2fd1430c00133

Failure log: 
https://treeherder.mozilla.org/logviewer.html#?job_id=171858565&repo=mozilla-inbound&lineNumber=2626
https://treeherder.mozilla.org/logviewer.html#?job_id=171866461&repo=mozilla-inbound&lineNumber=11821
https://treeherder.mozilla.org/logviewer.html#?job_id=171866515&repo=mozilla-inbound&lineNumber=11624

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=aa8bcc37d8884c02d34d547308c355f4a7f72624&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-failure_classification_id=2&selectedJob=171858565

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac4c6867aede
Paper over a crash in WebRenderCommandBuilder::GenerateFallbackData. r=kats

Comment 24

[Ryan Hunt [:rhunt]]

I filed a bug for figuring out the proper rounding behavior and landed a patch which should paper over the crash.

Comment 32

[Noemi Erli[:noemi_erli]]

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

Comment 33

[Intermittent Failures Robot]

5 failures in 765 pushes (0.007 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-central: 3
* autoland: 2

Platform breakdown:
* linux64-qr: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1450307&startday=2018-04-02&endday=2018-04-08&tree=trunk