Closed Bug 1458999 Opened 2 years ago Closed 2 years ago

Crash in nsXULWindow::SyncAttributesToWidget

Categories

(Core :: XUL, defect, P1)

61 Branch
All
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed
firefox62 --- fixed

People

(Reporter: philipp, Assigned: emilio)

References

Details

(Keywords: crash, csectype-nullptr, regression)

Crash Data

Attachments

(1 file)

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)
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: nobody → emilio
Flags: needinfo?(emilio)
Attachment #8979719 - Flags: review?(enndeakin)
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+
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
https://hg.mozilla.org/mozilla-central/rev/bdf1c7272fcc
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1463819
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.