Crash in nsXULWindow::SyncAttributesToWidget

RESOLVED FIXED in Firefox 61

Status

()

defect
P1
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philipp, Assigned: emilio)

Tracking

({crash, csectype-nullptr, regression})

61 Branch
mozilla62
All
Windows 10
Points:
---
Dependency tree / graph

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

a year 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?
Find someone to own this?
Flags: needinfo?(enndeakin)
Neil, do we think we should fix this for 61?
Flags: needinfo?(enndeakin)
(oops)
Flags: needinfo?(enndeakin)
Flags: needinfo?(mconley)

Comment 4

a year 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

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

Updated

a year ago
Attachment #8979719 - Flags: review?(enndeakin)

Comment 7

a year 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

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdf1c7272fcc
Nullcheck for release, assert elsewhere. r=enn

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bdf1c7272fcc
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1463819
Reporter

Comment 11

a year ago
hi, could you request uplift to 61 beta in case you deem fit to do so?
Flags: needinfo?(emilio)
Yes, this should be uplifted, but also with the patch in bug 1463819. I'll request once both patches land.
Flags: needinfo?(emilio)
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+
You need to log in before you can comment on or make changes to this bug.