Crash in nsXULWindow::SyncAttributesToWidget

RESOLVED FIXED in Firefox 61

Status

()

P1
critical
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: philipp, Assigned: emilio)

Tracking

({crash, csectype-nullptr, regression})

61 Branch
mozilla62
All
Windows 10
crash, csectype-nullptr, regression
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61 fixed, firefox62 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
This bug was filed from the Socorro interface and is
report bp-4da7cc5a-4ce3-4125-b6df-fed540180503.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsXULWindow::SyncAttributesToWidget xpfe/appshell/nsXULWindow.cpp:1572
1 xul.dll nsXULWindow::OnChromeLoaded xpfe/appshell/nsXULWindow.cpp:1126
2 xul.dll nsWebShellWindow::OnStateChange xpfe/appshell/nsWebShellWindow.cpp:656
3 xul.dll nsDocLoader::DoFireOnStateChange uriloader/base/nsDocLoader.cpp:1315
4 xul.dll nsDocLoader::doStopDocumentLoad uriloader/base/nsDocLoader.cpp:869
5 xul.dll nsDocLoader::DocLoaderIsEmpty uriloader/base/nsDocLoader.cpp:747
6 xul.dll nsDocLoader::OnStopRequest uriloader/base/nsDocLoader.cpp:632
7 xul.dll mozilla::net::nsLoadGroup::RemoveRequest netwerk/base/nsLoadGroup.cpp:629
8 xul.dll nsIDocument::DoUnblockOnload dom/base/nsDocument.cpp:8422
9 xul.dll nsUnblockOnloadEvent::Run dom/base/nsDocument.cpp:8373

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

this signature is regressing in volume during the firefox 61 cycle. so far it's win10 only. the first crash was reported from 61.0a1 build 20180322220118.

perhaps related to bug 1439875, 1447875 & 1446264?
status-firefox62: --- → affected
Find someone to own this?
Flags: needinfo?(enndeakin)
Keywords: csectype-nullptr
Neil, do we think we should fix this for 61?
Flags: needinfo?(enndeakin)
(oops)
Flags: needinfo?(enndeakin)
Flags: needinfo?(mconley)

Comment 4

9 months ago
This is very likely a crash revealed by 1439875 and friends.

I'm assuming that the call to SetNonClientMargins either in or inside the call to HideWindowChrome is deleting this or mWindow or somesuch, since it looks like it sends events and invalidates.
Flags: needinfo?(enndeakin)
Thanks, Neil.

Hey emilio, assuming this is fallout from bug 1439875, do you have some cycles to investigate this?
Flags: needinfo?(mconley) → needinfo?(emilio)
(Assignee)

Updated

9 months ago
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8979719 - Flags: review?(enndeakin)

Comment 7

9 months ago
mozreview-review
Comment on attachment 8979719 [details]
Bug 1458999: Nullcheck for release, assert elsewhere.

https://reviewboard.mozilla.org/r/245872/#review251964
Attachment #8979719 - Flags: review?(enndeakin) → review+

Comment 8

9 months ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdf1c7272fcc
Nullcheck for release, assert elsewhere. r=enn
Triage: regression
Priority: -- → P1

Comment 10

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bdf1c7272fcc
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox62: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1463819
(Reporter)

Comment 11

9 months ago
hi, could you request uplift to 61 beta in case you deem fit to do so?
Flags: needinfo?(emilio)
(Assignee)

Comment 12

9 months ago
Yes, this should be uplifted, but also with the patch in bug 1463819. I'll request once both patches land.
Flags: needinfo?(emilio)
(Assignee)

Comment 13

9 months ago
Comment on attachment 8979719 [details]
Bug 1458999: Nullcheck for release, assert elsewhere.

Note that this needs also bug 1463819 to remove the diagnostic assertions.

Approval Request Comment
[Feature/Bug causing the regression]: pre-existing bug exposed by bug 1439875 and friends.
[User impact if declined]: crashes, mostly on startup on windows.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: pretty sure it fixes the crashes seen in bug 1458999 and in this bug, though we can wait for a few days to see crash rates if you want.
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: bug 1463819
[Is the change risky?]: not risky
[Why is the change risky/not risky?]: only more null-checks to handle a very rare situation.
[String changes made/needed]:
Attachment #8979719 - Flags: approval-mozilla-beta?
Comment on attachment 8979719 [details]
Bug 1458999: Nullcheck for release, assert elsewhere.

Fixes a crash. Approved for 61.0b10.
Attachment #8979719 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 15

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/96eeb9f7e318
status-firefox61: affected → fixed
You need to log in before you can comment on or make changes to this bug.