Closed
Bug 1077746
Opened 10 years ago
Closed 10 years ago
crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css or MathML-fonts installed
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
firefox34 | --- | unaffected |
firefox35 | + | fixed |
People
(Reporter: alice0775, Assigned: jtd)
References
Details
(Keywords: crash, reproducible)
Crash Data
Attachments
(1 file)
5.28 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-b363b29b-1d7e-45fe-b853-d43982141003. ============================================================= Steps To Reproduce: 1. Update some of add-ons and restart Actual Results: Browser crashes w/ this crash sig.
Reporter | ||
Comment 1•10 years ago
|
||
[Tracking Requested - why for this release]: regression since Nightly35.0a1 There is no need to add-on to reproduce crash. Just need userContent.css and certain sites. Steps To Reproduce: 1. Create New Profile 2. Create following userContent.css in the profile/chrome folder @namespace url(http://www.w3.org/1999/xhtml); @-moz-document url-prefix(http://), url-prefix(ftp://), url-prefix(file://), url-prefix(https://) { @font-face { font-family: "Arial"; src: local("Times New Roman"); } } 3. Start Nightly with the profile 4. Open the following 2 url in new tabs https://bugzilla.mozilla.org/show_bug.cgi?id=1075194 https://developer.mozilla.org/en-US/search?q=fr 5. Exit Browser Actual Results: Browser craches Expected Results: Not crash
status-firefox34:
--- → unaffected
status-firefox35:
--- → affected
tracking-firefox35:
--- → ?
Summary: crash in nsCSSValue::nsCSSValue(nsCSSValue const&) → crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css
Reporter | ||
Comment 2•10 years ago
|
||
bp-c7e3876b-1660-4f20-975a-34b7b2141004
Reporter | ||
Updated•10 years ago
|
Component: General → Layout
Reporter | ||
Comment 3•10 years ago
|
||
Regression window(m-i) Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/0e9883d2467b Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20141001203029 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/109ae9a694cc Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20141001205424 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0e9883d2467b&tochange=109ae9a694cc Regressed by: Bug 1028497
Blocks: 1028497
Keywords: reproducible
Reporter | ||
Updated•10 years ago
|
OS: Windows NT → Windows 7
Comment 4•10 years ago
|
||
Thanks for the STR. I get this assertion in a debug build: Assertion failure: HasRule(), at /z/moz2/a/layout/style/FontFace.cpp:827 ... (gdb) bt #0 0x00007ffff076dd02 in mozilla::dom::FontFace::DisconnectFromRule (this=0x7fffb6f1ff70) at /z/moz2/a/layout/style/FontFace.cpp:827 #1 0x00007ffff073d169 in mozilla::dom::FontFaceSet::DestroyUserFontSet (this=0x7fffdc1d81a0) at /z/moz2/a/layout/style/FontFaceSet.cpp:364 #2 0x00007ffff09825b2 in nsPresContext::SetShell (this=0x7fffb54f3800, aShell=0x0) at /z/moz2/a/layout/base/nsPresContext.cpp:1098 #3 0x00007ffff08a244c in PresShell::Destroy (this=0x7fffb52bf800) at /z/moz2/a/layout/base/nsPresShell.cpp:1256 ...
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86 → All
Comment 5•10 years ago
|
||
I think the issue is that we share the style sheet across documents, and the nsCSSFontFaceRule only has a single pointer for its FontFace. But the FontFace has a pointer to the FontFaceSet that is managing it, and there is one of those per document. So we end up inserting the same FontFace into more than one FontFaceSet, and we run into trouble when we tear down the second FontFaceSet that has a hold of the one FontFace.
Comment 6•10 years ago
|
||
Two solutions come to mind: (1) Move the mapping of nsCSSFontFaceRule to FontFace into the FontFaceSet. (2) Have the nsCSSFontFaceRule store a table of (FontFaceSet, FontFace) entries or (nsPresContext, FontFace) entries. (2) doesn't quite feel right since I don't think we store any document/prescontext-specific information on rules at the moment.
Comment 7•10 years ago
|
||
Assignee: nobody → cam
Attachment #8499956 -
Flags: review?(jdaggett)
Comment 8•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=2cc860745274
Comment 9•10 years ago
|
||
This also crashes if you have the MathML-fonts extension installed.
Updated•10 years ago
|
Summary: crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css → crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css of MathML-fonts installed
Updated•10 years ago
|
Summary: crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css of MathML-fonts installed → crash in nsCSSValue::nsCSSValue(nsCSSValue const&) on shutdown with certain sites if @font-face specified in userContent.css or MathML-fonts installed
Comment 10•10 years ago
|
||
(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #7) > Created attachment 8499956 [details] [diff] [review] > patch I have verified that this patch fixes my issue from comment 9 that a crash occurs if you have the MathML-fonts extension installed.
Comment 11•10 years ago
|
||
(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #8) > https://tbpl.mozilla.org/?tree=Try&rev=2cc860745274 The M1 oranges here were a problem with the revision of inbound I was on, btw; there doesn't look to be any failures from my patch in the try run.
Comment 12•10 years ago
|
||
I am not but think maybe I should change the severity to blocker because I suspect it is blocking work on MathML because people working on that probably have the MathML-fonts extension installed.
Comment 13•10 years ago
|
||
I'm on Arch and Manjaro Linux and I get this every time I exit or restart fx. For me the cause is an add-on called Bamboo Feed Reader.
> https://addons.mozilla.org/en-US/firefox/addon/bamboo-feed-reader/?src=api
> https://crash-stats.mozilla.com/report/index/61581719-b2c9-4d19-b36d-d826c2141005
Comment 14•10 years ago
|
||
This is top crasher #1 now (>2.4k crashes since this came up). topcrash keyword?
Comment 15•10 years ago
|
||
Anyone who wants to keep up to date with nightly changes but can't live with this crasher are welcome to use my builds located at https://www.wg9s.com/mozilla/firefox/ which contain this fix as well as some pending MathML fixes.
Assignee | ||
Updated•10 years ago
|
Attachment #8499956 -
Flags: review?(jdaggett) → review+
Updated•10 years ago
|
Comment 18•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7fceb8bf84d2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Depends on: 1079199
Looks like I might have found another case -- should I reopen or file a new bug? STR: 1. Load http://blog.cloudflare.com/understanding-and-mitigating-ntp-based-ddos-attacks/ 2. Crash https://crash-stats.mozilla.com/report/index/bfac7bce-58e9-4ca3-893e-e32122141007
Comment 20•10 years ago
|
||
(In reply to Stephen Donner [:stephend] from comment #19) > Looks like I might have found another case -- should I reopen or file a new > bug? There's also bug 1079422, does it fit that one?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #20) > (In reply to Stephen Donner [:stephend] from comment #19) > > Looks like I might have found another case -- should I reopen or file a new > > bug? > > There's also bug 1079422, does it fit that one? Doesn't seem to (to my untrained eye, at least -- the top 5 frames aren't consistent, or sometimes only the first 2 are) - I also crash on my nightly build for my Fennec on HTC One (M8), here: https://crash-stats.mozilla.com/report/index/fcdfa9cc-7b2c-4965-a87d-fa40d2141007 I'll file a new bug and we can cross-link/dupe as we see fit.
(In reply to Stephen Donner [:stephend] from comment #21) > Doesn't seem to (to my untrained eye, at least -- the top 5 frames aren't > consistent, or sometimes only the first 2 are) - I also crash on my nightly > build for my Fennec on HTC One (M8), here: > https://crash-stats.mozilla.com/report/index/fcdfa9cc-7b2c-4965-a87d- > fa40d2141007 > > I'll file a new bug and we can cross-link/dupe as we see fit. Just filed bug 1079620.
Assignee | ||
Comment 23•10 years ago
|
||
Reopening this as it doesn't appear to be a good fix.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•10 years ago
|
Assignee: cam → jdaggett
Comment 24•10 years ago
|
||
(In reply to John Daggett (:jtd) from comment #23) > Reopening this as it doesn't appear to be a good fix. Well, you are too **** yourself. It is not a bad fix, just does not fix all the issues. Without this patch I can not run nighties at all, not even good enough to get them to run good enough to update to the latest nightly without crashing.
Assignee | ||
Comment 25•10 years ago
|
||
On closer inspection, the patch here doesn't address the fundamental problem, namely that since it's possible for @font-face rules to be shared across multiple documents, an individual nsCSSFontFaceRule object should *not* point to a FontFace. It looks like the methods that reference GetFontFace are only within FontFaceSet::UpdateRules, so it would be smarter I think to build a map on the fly within that routine and not try to maintain a link from nsCSSFontFaceRule ==> FontFace. With a user stylesheet present, logging calls to SetFontFace shows that multiple fontsets are sharing the same underlying @font-face rule: userfont FontFaceForRule rule: 0x1180930c0 fontset: 0x12350b660 fontface: 0x12350e500 userfont FontFaceForRule rule: 0x1180930c0 fontset: 0x12a6e5c20 fontface: 0x1294a7d30 So I think the better way is to eliminate the need for the FontFace ptr altogether and the map that's maintained across calls to UpdateRules. I think this should eliminate both the original crash and the crashes associated with the patch here.
Assignee | ||
Comment 26•10 years ago
|
||
Patches to back out this change and fix the underlying problem on bug 1079422.
Comment 27•10 years ago
|
||
See also bug 1080086 comment #5 for a similar crash on SeaMonkey (at closedown after updating add-ons, then at first startup when restarting the crashed session). Dupe?
Comment 28•10 years ago
|
||
Oh, and BTW I have a userChrome.css and a userContent.css; my userChrome.css includes several font rules (including a font-family for all chrome elements).
Assignee | ||
Comment 29•10 years ago
|
||
Fixed by changes made for bug 1079422.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•