1663623 - Assertion failure: !chan (Why is there a document channel?), at toolkit/components/windowwatcher/nsWindowWatcher.cpp:1016

Status: NEW

(Core :: Printing: Setup, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

STR;

1. Disable print.tab_modal.enabled
2. Open about:preferences
3. Ctrl+P

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Another STR;

1. Enable print.tab_modal.enabled
2. Open about:preferences
3. Change the destination to "Save to PDF"
4. Print

I am not sure whether this happens on beta or not.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

This doesn't happen on 81beta (on ec231f93093f6b5a587c6591e435efebab47f11e).

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(I am totally unsure this assertion is problematic or not)

Comment 4

[Daniel Holbert [:dholbert]]

I just ran into this fatal assertion. My STR (fresh profile) are:

1. Visit about:config
2. Set print.always_print_silent to true (if it's not already true) [EDIT: I'm belatedly adding this step after realizing it's needed; see comment 8]
3. Ctrl+P (while viewing about:config). (or about:preferences; that's similarly affected)

This repro's the fatal assertion-failure for me 100% of the time (attempted twice, reproduced both times) in any build with --enable-debug (i.e. any build where the assertion is active).

I captured in pernosco and I'll post the trace here when I've got it.

Comment 5

[Daniel Holbert [:dholbert]]

Pernosco recording: https://pernos.co/debug/jf70L8Ja3XFl6J4DoP-nLQ/index.html#f{m[AyXi,JzPg_,t[Kw,A0Og_,f{e[AyWS,JADr_,s{af5Ivt/AA,bCuk,uFz0hMA,oF0Fvag___/

Comment 6

[Daniel Holbert [:dholbert]]

It looks like the first version of this assertion was added by bz, back in 2006:
https://searchfox.org/mozilla-central/diff/e9379f36796a621ed9bef7cb1f8483b4606d084a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp#1671

though hg blame shows that kmag and nika have refactored this section of code several times, moving the assertion around a bit. In particular, it was upgraded from a nonfatal assertion to a fatal assertion in bug 1562292 part 2d.

kmag, maybe you could take a look at the pernosco trace and see if anything is amiss? It's a bit unfortunate to have this as a Ctrl+P insta-abort footgun for about:config in debug builds (as it currently seems to be for me, at least). [edit: it's not quite this bad, it's only an insta-abort if you have silent printing turned on]

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: prefs.js to reliably trigger this bug in debug builds (for dholbert at least) (EDIT: obsoleting because `print.always_print_silent` is the only part of this that matters)

(In reply to Daniel Holbert [:dholbert] from comment #4)

I just ran into this fatal assertion. My STR (fresh profile) are:
[...]
it's a bit unfortunate to have this as a Ctrl+P insta-abort footgun for about:config

Correction: I'd forgotten, I wasn't actually working with a fresh profile. I had a prefs.js file that I tool from a fuzzing bug, and that prefs.js seems to be required to trigger this fatal assertion. I'm attaching it here.

If I clobber my profile and start fresh (without this prefs.js file), I don't hit this issue.

Comment 8

[Daniel Holbert [:dholbert]]

Turns out there's only one pref from comment 7 that's needed to trigger the bug: print.always_print_silent = true.

I've updated my STR in comment 4.

If it ends up mattering for reproducibility: I'm on Ubuntu 22.04, and I have no printers configured for this machine, so my Firefox default print target (the one automatically used for silent printing) is our Save To PDF backend.

Comment 9

[Andrew McCreight [:mccr8]]

Maybe Nika can take a look before Kris.

Comment 10

[Nika Layzell [:nika] (ni? for response)]

As far as I can tell, this is caused by an issue with how we create the static clone browser in the case of silent printing. The window opening code ends up calling all the way through to https://searchfox.org/mozilla-central/rev/e66cff951667dacd0faa95dfde830564a58a8a3f/toolkit/components/printing/content/printUtils.js#418-428, creating the browser using the PrintPreview.createPreviewBrowser method and returning it from createContentWindow.

I think the issue here is that the browser which is created in PrintPreview.createPreviewBrowser only has nodefaultsrc set if this.settingsBrowser is non-null (https://searchfox.org/mozilla-central/rev/e66cff951667dacd0faa95dfde830564a58a8a3f/toolkit/components/printing/content/printUtils.js#791,800), but the browser returned from createContentWindow can never be navigating when it is returned, so must have been created with nodefaultsrc if it is a new window (which is effectively what that assertion is testing). This could actually potentially cause issues where the static clone document is clobbered when the implicit about:blank load finishes.

I'm guessing we need to move the code which sets nodefaultsrc out of the condition. I think this means it was regressed by bug 1666247 (which changed the code, making it possible to forget to set nodefaultsrc), though it appears silent printing was actually broken until it was fixed in bug 1741698, so perhaps that codepath was unreachable until then.

Redirecting the ni? to :jwatt