Closed
Bug 1382019
Opened 7 years ago
Closed 7 years ago
stylo: Startup Crash in nsStyleContext::AsServo
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | blocking | fixed |
People
(Reporter: marcia, Assigned: manishearth)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
This bug was filed from the Socorro interface and is
report bp-167610b3-bdb9-4fa1-a9bc-d94050170718.
=============================================================
Today's respun nightly is crashing on startup with this stack with stylo enabled: http://bit.ly/2uxdYRb. Running Windows 10.
Without stylo there is no crash.
Flags: needinfo?(bobbyholley)
Comment 4•7 years ago
|
||
This is a regression that happened to be picked up by the respin. :-(
Manish has a patch. We probably want to land it directly to central and then respin again.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
Patch moved to this bug. try @ https://treeherder.mozilla.org/#/jobs?repo=try&author=manishearth@gmail.com&selectedJob=115335600
Assignee: nobody → manishearth
Assignee | ||
Comment 7•7 years ago
|
||
Stylo try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=da13071104f4ba881580ae242dfbc36c0ba14d60 , forgot to include it in the previous stuff
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8887685 [details]
Bug 1382019 - stylo: Don't crash on null parent contexts in nsCSSFrameConstructor::AddFrameConstructionItemsInternal ;
https://reviewboard.mozilla.org/r/158584/#review163870
::: layout/base/nsCSSFrameConstructor.cpp:5892
(Diff revision 1)
> // first time. Thus call StyleNewChildren() again.
> styleSet->StyleNewChildren(element);
>
> + auto parent = styleContext->GetParentAllowServo();
> + ServoStyleContext* parentServo = parent ? parent->AsServo() : nullptr;
> +
This works, but I'd rather just pass `nullptr` there. There's no way that can create an style context given it's using `LazyComputeBehavior::Assert`.
That way you also get rid of a `GetParentAllowServo`, which is nicer.
Attachment #8887685 -
Flags: review+
Comment 9•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8887685 [details]
Bug 1382019 - stylo: Don't crash on null parent contexts in nsCSSFrameConstructor::AddFrameConstructionItemsInternal ;
https://reviewboard.mozilla.org/r/158584/#review163870
Also, it'd be nice to have a test for this, btw.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Crash Signature: [@ nsStyleContext::AsServo] → [@ nsStyleContext::AsServo]
[@ nsStyleContext::IsServo ]
Assignee | ||
Comment 11•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=818f0461e6e0b9dc39b0281a1ba0764aeee0f69f
Seems to be fine, might want to wait a bit more for tests to fill out.
How can we uplift this?
(AIUI getting this landed is more urgent, so i'm not including a test yet)
Flags: needinfo?(bobbyholley)
Comment 13•7 years ago
|
||
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/1b065ffd8a53
stylo: Don't crash on null parent contexts in nsCSSFrameConstructor::AddFrameConstructionItemsInternal ; r=emilio a=crashfix CLOSED TREE
Can you resolve this once you confirm it actually fixes things?
Flags: needinfo?(manishearth)
Comment 15•7 years ago
|
||
Do you have a way to confirm it/reproduce the crash?
Updated•7 years ago
|
tracking-firefox56:
--- → blocking
Comment 16•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> Do you have a way to confirm it/reproduce the crash?
just enable stylo (layout.css.servo.enabled=true) and start firefox.
Comment 17•7 years ago
|
||
I'm running with this patch on inbound and no longer get the crashes as specified.
Comment 18•7 years ago
|
||
FWIW, I think the confusion here is that this is a platform-specific startup crash that doesn't happen on linux, which is the only platform we're running on CI right now for stylo. We're fixing this in bug 1380053.
Flags: needinfo?(bobbyholley)
Comment 19•7 years ago
|
||
I'm running On Windows 10 btw...
Blocks: 1380053
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(manishearth)
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Target Milestone: --- → mozilla56
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•