Crash in geckoservo::glue::Servo_Element_GetPrimaryComputedValues

RESOLVED FIXED in Firefox 62

Status

()

defect
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: marcia, Assigned: emilio)

Tracking

({crash, regression, reproducible})

Trunk
mozilla62
Unspecified
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

(crash signature, )

Attachments

(2 attachments)

This bug was filed from the Socorro interface and is
report bp-d3160d40-b0ef-4ee0-bebd-dae9c0180529.
=============================================================

Primarily Mac and Fennec crash spotted on nightly: https://bit.ly/2kF1WP3. Mac crashes are 16 crashes/5 installs, although it appears the same person crashed several times on a localhost test. 

I can produce the crash on Mac nightly using https://www.vanmoof.com/en_us.

Top 10 frames of crashing thread:

0 XUL geckoservo::glue::Servo_Element_GetPrimaryComputedValues src/libcore/cell.rs:240
1 XUL nsCSSFrameConstructor::ResolveComputedStyle layout/base/nsCSSFrameConstructor.cpp:5012
2 XUL nsCSSFrameConstructor::AddFrameConstructionItems layout/base/nsCSSFrameConstructor.cpp:5573
3 XUL nsCSSFrameConstructor::ContentRangeInserted layout/base/nsCSSFrameConstructor.cpp:6753
4 XUL mozilla::RestyleManager::ProcessRestyledFrames layout/base/RestyleManager.cpp:1510
5 XUL mozilla::RestyleManager::DoProcessPendingRestyles layout/base/RestyleManager.cpp:2994
6 XUL mozilla::PresShell::DoFlushPendingNotifications layout/base/RestyleManager.cpp:3071
7 XUL nsIDocument::FlushPendingNotifications layout/base/nsIPresShell.h:576
8 XUL mozilla::dom::Element::GetScrollFrame dom/base/Element.cpp:2291
9 XUL mozilla::dom::Element::GetClientAreaRect dom/base/Element.cpp:1049

=============================================================
Crash Signature: [@ geckoservo::glue::Servo_Element_GetPrimaryComputedValues] → [@ geckoservo::glue::Servo_Element_GetPrimaryComputedValues] [@ struct style::gecko_bindings::sugar::ownership::Strong<T> geckoservo::glue::Servo_Element_GetPrimaryComputedValues]

Comment 1

a year ago
FWIW, I submitted one of the crash reports (probably the localhost ones mentioned above).  We're getting ready to release our application which uses Web Components.  We were testing how our product works after ShadowDOM was enabled in the nightly: https://bugzilla.mozilla.org/show_bug.cgi?id=1460069.  It looks like Vanmoof's site also uses Web Components.  

In our application we have a `div` with `display:flex;` styling and a HTMLSlotElement child.  Changing that style from `flex` to `block` stops the crashing.  I've tried building out a simple reproduction of this setup but haven't been able to get it to crash.  Let me know if there is more information I can provide.  Thanks!
(In reply to Michael England from comment #1)
> FWIW, I submitted one of the crash reports (probably the localhost ones
> mentioned above).  We're getting ready to release our application which uses
> Web Components.  We were testing how our product works after ShadowDOM was
> enabled in the nightly:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1460069.  It looks like
> Vanmoof's site also uses Web Components.  
> 
> In our application we have a `div` with `display:flex;` styling and a
> HTMLSlotElement child.  Changing that style from `flex` to `block` stops the
> crashing.  I've tried building out a simple reproduction of this setup but
> haven't been able to get it to crash.  Let me know if there is more
> information I can provide.  Thanks!

That's really helpful! I managed to repro on Vanmoof, though no clear STR (switching tabs back and forth seems to help).

I'll take a look. Of course a more reduced test-case would be appreciated, but of course that's really helpful still!
Flags: needinfo?(emilio)
Assignee

Updated

a year ago
Assignee: nobody → emilio
Assignee

Updated

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

Comment 5

a year ago
mozreview-review
Comment on attachment 8982776 [details]
Bug 1465572: Changing the slot name should properly invalidate layout.

https://reviewboard.mozilla.org/r/248664/#review254886
Attachment #8982776 - Flags: review?(bugs) → review+

Comment 6

a year ago
mozreview-review
Comment on attachment 8982777 [details]
Bug 1465572: Ensure <slot> disables whitespace optimizations.

https://reviewboard.mozilla.org/r/248666/#review255606
Attachment #8982777 - Flags: review?(bzbarsky) → review+

Comment 7

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/297e6634c0a6
Changing the slot name should properly invalidate layout. r=smaug
https://hg.mozilla.org/integration/autoland/rev/d81a759c7d95
Ensure <slot> disables whitespace optimizations. r=bz

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/297e6634c0a6
https://hg.mozilla.org/mozilla-central/rev/d81a759c7d95
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11377 for changes under testing/web-platform/tests
Is this something that should ride the trains or should we consider it for Beta backport?
Flags: needinfo?(emilio)
Flags: in-testsuite+
This is only a Shadow DOM issue which is not enabled in Beta.
Flags: needinfo?(emilio)
Huh, we see a couple Beta reports, but maybe they're from people manually turning it on.
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.