Closed
Bug 1017798
Opened 10 years ago
Closed 10 years ago
Assertion failure in AncestorFilter::AssertHasAllAncestors()
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: botond, Assigned: heycam)
References
Details
Attachments
(3 files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
STR: 1. Build B2G with today's gecko in debug mode. 2. Run the First Time Experience. 3. On the Geolocation screen, disable geolocation. The tap to disable geolocation triggers the following assertion failure: Program received signal SIGSEGV, Segmentation fault. 0xb57643cc in AncestorFilter::AssertHasAllAncestors (this=0xbedd41a0, aElement=<optimized out>) at ../../../../refactoring/layout/style/nsCSSRuleProcessor.cpp:3620 3620 MOZ_ASSERT(mElements.Contains(cur)); (gdb) i s #0 0xb57643cc in AncestorFilter::AssertHasAllAncestors (this=0xbedd41a0, aElement=<optimized out>) at ../../../../refactoring/layout/style/nsCSSRuleProcessor.cpp:3620 #1 0xb5766e2a in RuleHash::EnumerateAllRules (this=0xb2eda140, aElement=0xb210d2e0, aData=0xbedd3c34, aNodeContext=...) at ../../../../refactoring/layout/style/nsCSSRuleProcessor.cpp:770 #2 0xb5767dac in nsCSSRuleProcessor::RulesMatching (this=<optimized out>, aData=0xbedd3c34) at ../../../../refactoring/layout/style/nsCSSRuleProcessor.cpp:2480 #3 0xb57ce6e4 in EnumRulesMatching<ElementRuleProcessorData> (aProcessor=<optimized out>, aData=<optimized out>) at ../../../../refactoring/layout/style/nsStyleSet.cpp:671 #4 0xb57d1180 in nsStyleSet::FileRules (this=0xb2125180, aCollectorFunc=0xb57ce6dd <EnumRulesMatching<ElementRuleProcessorData>(nsIStyleRuleProcessor*, void*)>, aData=0xbedd3c34, aElement=0xb210d2e0, aRuleWalker=0xbedd3c28) at ../../../../refactoring/layout/style/nsStyleSet.cpp:976 #5 0xb57d1ae2 in nsStyleSet::ResolveStyleFor (this=0xb2125180, aElement=0xb210d2e0, aParentContext=0xb1741338, aTreeMatchContext=...) at ../../../../refactoring/layout/style/nsStyleSet.cpp:1206 #6 0xb5805552 in mozilla::ElementRestyler::RestyleSelf (this=0xbedd3da4, aSelf=0xb16c7928, aRestyleHint=eRestyle_Subtree) at ../../../../refactoring/layout/base/RestyleManager.cpp:2462 #7 0xb5805978 in mozilla::ElementRestyler::Restyle (this=0xbedd3da4, aRestyleHint=eRestyle_Subtree) at ../../../../refactoring/layout/base/RestyleManager.cpp:2306 #8 0xb5806098 in mozilla::ElementRestyler::RestyleContentChildren (this=0xbedd3eec, aParent=<optimized out>, aChildRestyleHint=eRestyle_Subtree) at ../../../../refactoring/layout/base/RestyleManager.cpp:2844 #9 0xb5806160 in mozilla::ElementRestyler::RestyleChildren (this=0xbedd3eec, aChildRestyleHint=eRestyle_Subtree) at ../../../../refactoring/layout/base/RestyleManager.cpp:2577 #10 0xb5805994 in mozilla::ElementRestyler::Restyle (this=0xbedd3eec, aRestyleHint=eRestyle_Subtree) at ../../../../refactoring/layout/base/RestyleManager.cpp:2310 #11 0xb5806064 in mozilla::ElementRestyler::RestyleContentChildren (this=0xbedd3ff8, aParent=<optimized out>, aChildRestyleHint=0) at ../../../../refactoring/layout/base/RestyleManager.cpp:2833 #12 0xb5806160 in mozilla::ElementRestyler::RestyleChildren (this=0xbedd3ff8, aChildRestyleHint=0) at ../../../../refactoring/layout/base/RestyleManager.cpp:2577 #13 0xb5805994 in mozilla::ElementRestyler::Restyle (this=0xbedd3ff8, aRestyleHint=eRestyle_Self) at ../../../../refactoring/layout/base/RestyleManager.cpp:2310 #14 0xb5805ada in mozilla::RestyleManager::ComputeStyleChangeFor (this=0xb2f14f00, aFrame=<optimized out>, aChangeList=0xbedd41fc, aMinChange=0, aRestyleTracker=..., aRestyleDescendants=false) at ../../../../refactoring/layout/base/RestyleManager.cpp:2958 #15 0xb5805efe in mozilla::RestyleManager::RestyleElement (this=0xb2f14f00, aElement=0xb1c941d0, aPrimaryFrame=0xb1c86eb8, aMinHint=0, aRestyleTracker=..., aRestyleDescendants=false) at ../../../../refactoring/layout/base/RestyleManager.cpp:866 #16 0xb58062b0 in mozilla::RestyleTracker::ProcessOneRestyle (this=0xb2f14f28, aElement=0xb1c941d0, aRestyleHint=eRestyle_Self, aChangeHint=0) at ../../../../refactoring/layout/base/RestyleTracker.cpp:123 #17 0xb5806916 in mozilla::RestyleTracker::DoProcessRestyles (this=0xb2f14f28) at ../../../../refactoring/layout/base/RestyleTracker.cpp:205 #18 0xb5805d66 in ProcessRestyles (this=0xb2f14f28) at ../../../../refactoring/layout/base/RestyleTracker.h:273 #19 mozilla::RestyleManager::ProcessPendingRestyles (this=0xb2f14f00) at ../../../../refactoring/layout/base/RestyleManager.cpp:1442 #20 0xb57ef9a4 in PresShell::FlushPendingNotifications (this=0xb2f5a680, aFlush=...) at ../../../../refactoring/layout/base/nsPresShell.cpp:4052 #21 0xb57f5ea4 in nsRefreshDriver::Tick (this=0xb2f150a0, aNowEpoch=<optimized out>, aNowTime=...) at ../../../../refactoring/layout/base/nsRefreshDriver.cpp:1162 #22 0xb57f65e0 in mozilla::RefreshDriverTimer::Tick (this=0xb28ad540) at ../../../../refactoring/layout/base/nsRefreshDriver.cpp:162 #23 0xb4a1a896 in nsTimerImpl::Fire (this=0xb2336f10) at ../../../../refactoring/xpcom/threads/nsTimerImpl.cpp:608 #24 0xb4a1aa20 in nsTimerEvent::Run (this=0xb28d6650) at ../../../../refactoring/xpcom/threads/nsTimerImpl.cpp:701 #25 0xb4a17e12 in ProcessNextEvent (aResult=0xbedd4d17, aMayWait=false, this=0xb3a47880) at ../../../../refactoring/xpcom/threads/nsThread.cpp:766 #26 nsThread::ProcessNextEvent (this=0xb3a47880, aMayWait=<optimized out>, aResult=0xbedd4d17) at ../../../../refactoring/xpcom/threads/nsThread.cpp:685 #27 0xb49cec9c in NS_ProcessNextEvent (thread=0xb3a47880, mayWait=<optimized out>) at ../../../../refactoring/xpcom/glue/nsThreadUtils.cpp:263 #28 0xb4bd3ce8 in mozilla::ipc::MessagePump::Run (this=0xb3a01b80, aDelegate=0xbedd4e70) at ../../../../refactoring/ipc/glue/MessagePump.cpp:95 #29 0xb4bc0aaa in MessageLoop::RunInternal (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:229 #30 0xb4bc0ac2 in RunHandler (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:222 #31 MessageLoop::Run (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:196 #32 0xb51f2cde in nsBaseAppShell::Run (this=0xb2e6c880) at ../../../../refactoring/widget/xpwidgets/nsBaseAppShell.cpp:164 #33 0xb5ac5eea in XRE_RunAppShell () at ../../../../refactoring/toolkit/xre/nsEmbedFunctions.cpp:690 #34 0xb4bd3dfe in mozilla::ipc::MessagePumpForChildProcess::Run (this=0xb3a01b80, aDelegate=0xbedd4e70) at ../../../../refactoring/ipc/glue/MessagePump.cpp:253 #35 0xb4bc0aaa in MessageLoop::RunInternal (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:229 #36 0xb4bc0ac2 in RunHandler (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:222 #37 MessageLoop::Run (this=0xbedd4e70) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:196 #38 0xb5ac5db0 in XRE_InitChildProcess (aArgc=<optimized out>, aArgv=<optimized out>, aProcess=<optimized out>) at ../../../../refactoring/toolkit/xre/nsEmbedFunctions.cpp:527 #39 0x00008862 in main (argc=7, argv=0xbedd5964) at ../../../../refactoring/ipc/app/MozillaRuntimeMain.cpp:149 (gdb)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #1) > This isn't by chance using web components, is it? CC'ing Garvan, he might know (or know who might know).
I don't know anything helpful (or who would be involved in that setting).
Reporter | ||
Comment 4•10 years ago
|
||
CC'ing Francisco, he might be able to answer Boris' question in comment 1.
Assignee | ||
Comment 5•10 years ago
|
||
There's a web component registered for the <gaia-switch> element which is the thing you tap.
Assignee | ||
Comment 7•10 years ago
|
||
In the FTU app's index.html, we have: <ul> <li> <gaia-switch id="geolocation-switch" name="geolocation.enabled" checked> <label data-l10n-id="geolocationCheckbox"></label> </gaia-switch> </li> </ul> We're restyling the <label> in the document. But the ancestor filter contains a <label> (and its child <span>) from a shadow tree (the ancestor filter's <label> element has a ShadowRoot parent).
![]() |
||
Comment 8•10 years ago
|
||
So TreeMatchContext::InitAncestors uses GetParentNode() to walk up the tree. So if we started the restyle somewhere in the anon content, we only put anon things in there (because for shadow DOM, unlike XBL, .parentNode doesn't cross back into the light DOM), but then we walked down the _frame_ tree during the restyle and into light content. All of which is to say we need to teach InitAncestors (and probably InitStyleScopes) about going from kids of a ShadowRoot to the shadow host.
Assignee | ||
Comment 9•10 years ago
|
||
But will putting those flattened tree ancestors in the AncestorFilter affect the correctness of selector matching? Normally you don't want to consider ancestors in the light tree when determining whether a selector matches a node in the shadow tree.
![]() |
||
Comment 10•10 years ago
|
||
AncestorFilter is an optimization. We ask it whether we're guaranteed that we have no ancestors with a given class/id/etc. So correctness requires that any actual ancestors be in the ancestor filter. But you could stick whatever else you want in there; it would just mean that sometimes we end up not being to optimize out the SelectorMatchesTree tree-walk when we actually could have.
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
QA Contact: cam
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → cam
QA Contact: cam
Assignee | ||
Comment 15•10 years ago
|
||
One thing that still doesn't work is using a <style scoped> as a child of a ShadowRoot, since we don't stick the ElementIsStyleScopeRoot flag on the ShadowRoot. Filed bug 1021572 for that.
![]() |
||
Comment 16•10 years ago
|
||
Comment on attachment 8435602 [details] [diff] [review] Part 1: Make AncestorFilter and the TreeMatchContext style scope list include ancestors from following ShadowRoots up to the light tree. >+ if (host && host->IsElement()) { It is possible to have a shadowroot with no host, or with a non-element host? r=me
Attachment #8435602 -
Flags: review?(bzbarsky) → review+
Flags: needinfo?(wchen)
![]() |
||
Comment 17•10 years ago
|
||
Comment on attachment 8435603 [details] [diff] [review] Part 2: Set content bits on elements in shadow trees indicating whether they are in a style scope from the light tree. Maybe make those methods protected class statics on Element so you don't have to duplicate them? r=me
Attachment #8435603 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 18•10 years ago
|
||
Comment on attachment 8435604 [details] [diff] [review] Part 3: Crashtest. r=me
Attachment #8435604 -
Flags: review?(bzbarsky) → review+
Comment 19•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #16) > It is possible to have a shadowroot with no host, or with a non-element host? ShadowRoot should always have a non-null element host.
Flags: needinfo?(wchen)
![]() |
||
Comment 20•10 years ago
|
||
Could we make the getter on ShadowRoot be called Host then (since it's never null) and have it return Element*?
Comment 21•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #20) > Could we make the getter on ShadowRoot be called Host then (since it's never > null) and have it return Element*? GetHost() lives on document fragment (it's defined in the DOM spec) and most of the time a DocumentFragment will not have a host. We certainly could add Host() to ShadowRoot if we wanted to.
![]() |
||
Comment 22•10 years ago
|
||
> GetHost() lives on document fragment (it's defined in the DOM spec)
Ah, I see. Can we at least make it return Element* if that's what it always has?
And yeah, a Host() on ShadowRoot might be nice.
Assignee | ||
Comment 23•10 years ago
|
||
wchen, does that mean it's possible for a non-ShadowRoot DocumentFragment to have a shadow tree attached to it? Can a shadow tree be attached to another ShadowRoot? At the moment I'm only looking at shadow trees on Elements, but given the getter lives on DocumentFragment...
![]() |
||
Comment 24•10 years ago
|
||
> does that mean it's possible for a non-ShadowRoot DocumentFragment to have a shadow tree
> attached to it
It's not possible.
Assignee | ||
Comment 25•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/33ce82daa572 https://hg.mozilla.org/integration/mozilla-inbound/rev/b6cd0caaf182 https://hg.mozilla.org/integration/mozilla-inbound/rev/a98e6f80344c
OS: Gonk (Firefox OS) → All
Hardware: ARM → All
Comment 26•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/33ce82daa572 https://hg.mozilla.org/mozilla-central/rev/b6cd0caaf182 https://hg.mozilla.org/mozilla-central/rev/a98e6f80344c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in
before you can comment on or make changes to this bug.
Description
•