1662090 - Crash in [@ mozilla::dom::Document::CloneDocHelper]

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Wayne Mery (:wsmwk)]

Most crashes are version 81

Crash report: https://crash-stats.mozilla.org/report/index/e258189d-fa1b-482d-9214-3c1330200831

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::Document::CloneDocHelper const dom/base/Document.cpp:11191
1 xul.dll nsHTMLDocument::Clone const dom/html/nsHTMLDocument.cpp:673
2 xul.dll static nsINode::CloneAndAdopt dom/base/nsINode.cpp:3103
3 xul.dll nsINode::CloneNode dom/base/nsINode.cpp:2981
4 xul.dll mozilla::dom::Document::CreateStaticClone dom/base/Document.cpp:12280
5 xul.dll nsPrintObject::InitAsRootObject layout/printing/nsPrintObject.cpp:111
6 xul.dll nsPrintJob::DoCommonPrint layout/printing/nsPrintJob.cpp:667
7 xul.dll nsPrintJob::CommonPrint layout/printing/nsPrintJob.cpp:587
8 xul.dll nsPrintJob::Print layout/printing/nsPrintJob.cpp:919
9 xul.dll nsDocumentViewer::Print layout/base/nsDocumentViewer.cpp:3168

Comment 1

[Wayne Mery (:wsmwk)]

81 crashes started with 20200828195907 build

Print related?

Comment 2

[Magnus Melin [:mkmelin]]

Yes seems to be from printing.

Looks like it's crashing due to null mTiming. Maybe the MOZ_ASSERT should be MOZ_CRASH instead? https://searchfox.org/mozilla-central/source/dom/base/Document.cpp#11192 but of course that wouldn't fix the problem...

Or add an MaybeInitTiming() call to https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/dom/html/nsHTMLDocument.cpp#672 ?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

This is my fault, kinda. Thunderbird is using nsIWebBrowserPrint.print() directly and that no longer works unless you do it on a pre-cloned document.

This line should instead go through nsGlobalWindowOuter::Print.

Magnus, can you fix? Or should I try to do it? I can probably take a bit of time later this week, just ni? me back.

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: bug1662090_print_outer.patch

Not sure if this was what you had in mind. But it doesn't really work. It doesn't pop up the print window. The "printing" dialog is shown but I don't see where that print would have gone either.

Thanks for any pointers.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Yeah I'd have expected something like that to work. I have a bit of time today so I can look into it.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: fix-print-engine.patch

Something like this works, though it needs a patch to printUtils.js because this throws.

The printing setup in nsMsgPrintEngine is kinda gross. We open a new window with msgPrintEngine.xhtml which actually does the printing... I have no idea why it has to be so complicated.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1662090 - Don't rely on gBrowser in PrintUtils.startPrintWindow. r=jwatt

This is all we really need and allows Thunderbird to use this rather
than rolling their own thing.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd85b98c3c80
Don't rely on gBrowser in PrintUtils.startPrintWindow. r=jwatt

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Magnus, ni? so that the patch above in comment 6 doesn't get lost, I don't know what the procedure to get stuff landed in comm-central is.

Comment 10

[Magnus Melin [:mkmelin]]

Thanks! Will get it landed once the mozilla-central part is merged from autoland.

Comment 11

[Razvan Maries]

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e443bcd3256b
Fix nsMsgPrintEngine. r=mkmelin

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 9174009 [details] [diff] [review]
fix-print-engine.patch

Review of attachment 9174009 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/imap/src/nsImapProtocol.cpp
@@ +9614,5 @@
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP nsImapMockChannel::Suspend() {
> +  // MOZ_ASSERT_UNREACHABLE("nsImapMockChannel::Suspend");

Removed these, that it triggers is another bug that turned up on trunk.

Comment 14

[Wayne Mery (:wsmwk)]

Would be wonderful to have this uplifted to 81 beta to fix #1 crash https://crash-stats.mozilla.org/topcrashers/?product=Thunderbird&version=81.0&_facets_size=50

Comment 15

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Do you need this uplifted Magnus?

Comment 17

[Magnus Melin [:mkmelin]]

With merge date so close, I think it's not necessary to uplift.

Comment 24

[Chris Andrichak]

print crash is fixed for me on 82.0b1, i'm able to print again

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8ba0e81e1821
followup - clang-format. rs=clang-format

Comment 26

[[e-mail]]

My selected events in my calendar are still not printing.

Comment 27

[Ray Valdes]

82.0B1 still not printing, the print preview just hangs up and the print does nothing

Comment 28

[Wayne Mery (:wsmwk)]

beta 82 users - if you still have a problem it may be better to post in your original bug report please

Comment 30

[Wayne Mery (:wsmwk)]

No crashes reported for thunderbird beta 83 in last 6 months