Closed
Bug 946312
Opened 11 years ago
Closed 11 years ago
Crashes in StateSelectorMatches
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 945048
People
(Reporter: smichaud, Assigned: heycam)
References
Details
(Keywords: steps-wanted, topcrash-mac)
Crash Data
https://crash-stats.mozilla.com/query/?product=Firefox&version=ALL%3AALL&range_value=4&range_unit=weeks&date=12%2F04%2F2013+17%3A00%3A00&query_search=signature&query_type=contains&query=StateSelectorMatches&reason=&release_channels=&build_id=&process_type=any&hang_type=any These start with the 2013-11-28-03-02-01-mozilla-central build, and are in code introduced by: http://hg.mozilla.org/mozilla-central/rev/8e580ff4d7ca Bug 922669 - Part 6: Split out user action pseudo-class matching from SelectorMatches. r=bz author Cameron McCormack <cam@mcc.id.au> Thu Nov 28 17:46:39 2013 +1100 (at Thu Nov 28 17:46:39 2013 +1100) They occur on Windows and Linux in low numbers, but are currently the #6 topcrasher on the Mac.
Reporter | ||
Updated•11 years ago
|
Crash Signature: [@ StateSelectorMatches ]
Reporter | ||
Comment 1•11 years ago
|
||
> but are currently the #6 topcrasher on the Mac.
They're the #6 Mac topcrasher on the 28 branch.
Updated•11 years ago
|
tracking-firefox28:
--- → ?
Updated•11 years ago
|
tracking-firefox29:
--- → ?
Comment 2•11 years ago
|
||
Passing this on to :cam for help with investigation as this may be a fallout from Bug 922669. Also this seems to be across platform.
Assignee: nobody → cdawson
Updated•11 years ago
|
Keywords: steps-wanted
Reporter | ||
Comment 3•11 years ago
|
||
Bhavana, did you mean to pass this to Cameron McCormack <cam@mcc.id.au>? :-)
Reporter | ||
Comment 4•11 years ago
|
||
All these crashes happen here: http://hg.mozilla.org/mozilla-central/annotate/84a5a5800bd3/layout/style/nsCSSRuleProcessor.cpp#l1671 They're probably null dereferences (aElement seems to be null).
Assignee | ||
Comment 6•11 years ago
|
||
I can't reproduce locally yet, but: Boris, in bug 945048 I removed the check for null pdata->mPseudoElement thinking that it wasn't needed now that bogus selectors like ::-moz-selection:hover don't parse. But I think we're crashing here because have a set of rules like: ::-moz-progress-bar:hover { } ::-moz-selection { } and when we are calling ProbePseudoElementStyle for the ::-moz-selection, we're passing in null as the element (as expected), but we end up finding the ::moz-progress-bar:hover rule and checking to see if it should contribute to the style returned by ProbePseudoElementStyle. So I think it makes sense to restore those checks. Does that sound right to you?
Flags: needinfo?(bzbarsky)
Comment 7•11 years ago
|
||
> but we end up finding the ::moz-progress-bar:hover rule That shouldn't happen. Those should be in separate rulehashes, no? The stacks in breakpad with this signature seem to be pretty confused (like those methods don't actually call each other, mostly), but here's my question: is this just a duplicate of bug 945048, maybe? I see no instances of this crash with a 2013-12-03 build or newer.... In particular, bug 945048 was a crash in StateSelectorMatches when we get to the GetContentStateForVisitedHandling call, but the rule there used :active. If you had a testcase as for that bug but using :hover, then you'd crash on the line where this bug says we're crashing, right?
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #7) > That shouldn't happen. Those should be in separate rulehashes, no? Yeah, they should be. > The stacks in breakpad with this signature seem to be pretty confused (like > those methods don't actually call each other, mostly), but here's my > question: is this just a duplicate of bug 945048, maybe? I see no instances > of this crash with a 2013-12-03 build or newer.... Oh, I misread the most recent build ID on crash-stats and looked locally at 2013-12-03 to see whether the bug 945048 fix is in there. > In particular, bug 945048 was a crash in StateSelectorMatches when we get to > the GetContentStateForVisitedHandling call, but the rule there used :active. > If you had a testcase as for that bug but using :hover, then you'd crash on > the line where this bug says we're crashing, right? Yes :hover and :active should have exactly the same crashing-or-not behaviour. So yeah, if there are no more crashes by tomorrow, let's dupe it to bug 945048.
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Updated•10 years ago
|
tracking-firefox28:
? → ---
tracking-firefox29:
? → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•