1655179 - Crash in [@ nsPrintJob::ReconstructAndReflow]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u608768]

This bug is for crash report bp-007b6781-d2de-46bd-ac33-c505f0200724.

Top 10 frames of crashing thread:

0 xul.dll nsPrintJob::ReconstructAndReflow layout/printing/nsPrintJob.cpp:1380
1 xul.dll nsPrintJob::SetupToPrintContent layout/printing/nsPrintJob.cpp:1469
2 xul.dll nsPrintJob::MaybeResumePrintAfterResourcesLoaded layout/printing/nsPrintJob.cpp:1733
3 xul.dll nsPrintJob::OnStateChange layout/printing/nsPrintJob.cpp:1756
4 xul.dll nsDocLoader::DoFireOnStateChange uriloader/base/nsDocLoader.cpp:1331
5 xul.dll nsDocLoader::FireOnStateChange uriloader/base/nsDocLoader.cpp:1294
6 xul.dll nsDocLoader::doStopURLLoad uriloader/base/nsDocLoader.cpp:898
7 xul.dll nsDocLoader::OnStopRequest uriloader/base/nsDocLoader.cpp:622
8 xul.dll mozilla::net::nsLoadGroup::NotifyRemovalObservers netwerk/base/nsLoadGroup.cpp:615
9 xul.dll mozilla::net::nsLoadGroup::RemoveRequest netwerk/base/nsLoadGroup.cpp:522

Started in build 20200723095657. Regression window: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=138e7b575614cbfc1e45576a15825f51cb6e6614&tochange=1415430432706ec6b8c7590cda9b8ca87397e7a7.

Erik, could this have been bug 1654406?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Emilio, did you land the resource loading changes already?

Comment 2

[Erik Nordin [:nordzilla]]

It's certainly possible that my patch could have caused this, but my changes should have been pretty self contained to the "Print Selection Only" option. If this is happening during any other use case then it may not be my patch.

I've been trying to reproduce this on my end, unsuccessfully so far. I will keep trying.

Does this happen only on Windows 7?

Comment 3

[u608768]

(In reply to Erik Nordin [:nordzilla] from comment #2)

Does this happen only on Windows 7?

Mostly Windows 7, but a few Windows 10 crashes too: https://crash-stats.mozilla.org/signature/?signature=nsPrintJob%3A%3AReconstructAndReflow&date=%3E%3D2020-07-17T21%3A50%3A00.000Z&date=%3C2020-07-24T21%3A50%3A00.000Z&_sort=-date

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah this smells a lot like something bug 1648064 could've made much more common. The code path is pre-existing, but before we were only using it for font loads and now we also use it for images.

Will poke.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I found a repro for bug 1652967, but not for this one ;(

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Ok, I can repro this consistently if I print https://mathiasbynens.be/demo/img-loading-lazy with print.always_print_silent=true. That works for me as a repro. Example crash: bp-a1760bf3-1456-4a65-824c-679e10200725

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1655179 - Don't attach a progress listener until we've started to set up the print presentation. r=smaug

This is afaict a pre-existing (conceptual) issue before my patch, but
it was only theoretical because @font-face loads are triggered from
styling / layout, so we wouldn't trigger them until we initialize the
document.

The print setup may be asynchronous
(via nsPrintJob::Observe -> InitPrintDocConstruction -> ReflowDocList).

If we get the load event sooner than that, then we'd try to start
printing before we've ever called ReflowDocList which is what sets
everything up.

Thus we'd assert in debug builds here:

https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/layout/printing/nsPrintJob.cpp#1453-1457

And crash in ReconstructAndReflow in release builds.

We don't care about what goes on before we start setting up the
presentation of the document, so defer setting up the
web-progress-listener until we do that.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f06512ba3db
Don't attach a progress listener until we've started to set up the print presentation. r=smaug

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/6f06512ba3db