Closed
Bug 1403694
Opened 7 years ago
Closed 7 years ago
stylo: Crash in mozilla::ServoStyleSet::AppendSheetOfType
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | verified |
firefox58 | --- | verified |
People
(Reporter: philipp, Assigned: bradwerth)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
xidorn
:
review+
ritu
:
approval-mozilla-beta+
|
Details |
This bug was filed from the Socorro interface and is report bp-d9d9e6a3-3f70-48b9-acd0-5ff540170927. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll mozilla::ServoStyleSet::AppendSheetOfType(mozilla::SheetType, mozilla::ServoStyleSheet*) layout/style/ServoStyleSet.cpp:1466 1 xul.dll mozilla::ServoStyleSet::AppendStyleSheet(mozilla::SheetType, mozilla::ServoStyleSheet*) layout/style/ServoStyleSet.cpp:715 2 xul.dll nsDocumentViewer::CreateStyleSet(nsIDocument*) layout/base/nsDocumentViewer.cpp:2482 3 xul.dll nsDocumentViewer::InitPresentationStuff(bool) layout/base/nsDocumentViewer.cpp:698 4 xul.dll nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) layout/base/nsDocumentViewer.cpp:959 5 xul.dll nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) layout/base/nsDocumentViewer.cpp:676 6 xul.dll nsDocShell::SetupNewViewer(nsIContentViewer*) docshell/base/nsDocShell.cpp:9547 7 xul.dll nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) docshell/base/nsDocShell.cpp:7366 8 xul.dll nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, bool, bool) docshell/base/nsDocShell.cpp:8240 9 xul.dll nsDocShell::EnsureContentViewer() docshell/base/nsDocShell.cpp:8098 10 xul.dll nsDocShell::GetDocument() docshell/base/nsDocShell.cpp:4629 11 xul.dll nsPIDOMWindow<mozIDOMWindowProxy>::MaybeCreateDoc() dom/base/nsGlobalWindow.cpp:4127 12 xul.dll nsPIDOMWindow<mozIDOMWindowProxy>::GetDoc() dom/base/nsPIDOMWindow.h:213 13 xul.dll nsGlobalWindow::WrapObject(JSContext*, JS::Handle<JSObject*>) dom/base/nsGlobalWindow.h:319 14 xul.dll XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, bool, nsresult*) js/xpconnect/src/XPCConvert.cpp:778 15 xul.dll XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, nsresult*) js/xpconnect/src/XPCConvert.cpp:345 16 xul.dll nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1267 17 xul.dll nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:615 18 xul.dll PrepareAndDispatch xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:85 19 xul.dll SharedStub xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:112 20 xul.dll nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) xpcom/ds/nsObserverList.cpp:112 this is showing up as a new signature in low volume after 57.0b was rolled-out to beta users in the last hours.
Comment 1•7 years ago
|
||
Brad: it appears we've got a null StyleSheet in the list here. Can you have a look and see how that can happen and how we should plug it? Thx!
Flags: needinfo?(bwerth)
Assignee | ||
Comment 2•7 years ago
|
||
It's coming from the nsStyleSheetService singleton, from the AGENT_SHEET entry for the Servo backend. I'm still searching for what sets that entry to NULL.
Assignee | ||
Comment 3•7 years ago
|
||
I've found at least one place where a NULL sheet could be put into that array: http://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/layout/base/nsStyleSheetService.cpp#269. This will happen if nsLayoutUtils::StyloSupportedInCurrentProcess() returns false, which is dependent on process state in a way I don't understand well. I'll change the nsStyleSheetService to not push a NULL sheet in that case.
Flags: needinfo?(bwerth)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8913485 -
Flags: review?(xidorn+moz)
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=718039ac8020900b21341462d645a9da7c219650
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8913485 [details] Bug 1403694 Part 1: Prevent nsStyleSheetService from storing null servo stylesheets. https://reviewboard.mozilla.org/r/184828/#review189978 ::: layout/base/nsStyleSheetService.cpp:254 (Diff revision 1) > } > > nsresult rv; > > RefPtr<StyleSheet> geckoSheet; > RefPtr<StyleSheet> servoSheet; In this case, I think you can move the variable `servoSheet` into the `if` block as well.
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8913485 [details] Bug 1403694 Part 1: Prevent nsStyleSheetService from storing null servo stylesheets. https://reviewboard.mozilla.org/r/184828/#review189982 I think this change itself is fine, but I suspect whether this would fix anything... When `StyloSupportedInCurrentProcess()` returns false, `UpdateStyleBackendType` should never set the backend to Servo, so `nsStyleSheetService` shouldn't ever return array from `mServoSheets`. And given `StyloSupportedInCurrentProcess()` doesn't change its return value in the lifetime of the process (because it simply depends on the process type), this invariant should never change. `nsIDocument::SetStyleBackendType` can be used to set the backend to something explicitly, but majority of its uses are just for inheriting backend from parent document, so they shouldn't be able to escape from the restriction above. There is one case, though, that the backend can be set to Servo regardless of the process restriction is `gfxSVGGlyphsDocument::ParseDocument` [1]. That can be a problem, but it doesn't seem to be related to this case anyway. [1] https://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/gfx/thebes/gfxSVGGlyphs.cpp#365-376
Attachment #8913485 -
Flags: review?(xidorn+moz) → review+
Comment 8•7 years ago
|
||
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7) > There is one case, though, that the backend can be set to > Servo regardless of the process restriction is > `gfxSVGGlyphsDocument::ParseDocument` [1]. That can be a problem, but it > doesn't seem to be related to this case anyway. Oh, wait, this is wrong. It checks `nsLayoutUtils::StyloEnabled()` which is the same check as `UpdateStyleBackendType`, so it is restricted by the process type as well.
Comment 9•7 years ago
|
||
That says, I have no idea how this crash can happen at all...
Assignee | ||
Comment 10•7 years ago
|
||
Thanks for the more detailed analysis in comment 7. I don't know that we actively check the invariant that "backend == servo -> StyloSupportedInCurrentProcess() returns true". Since the patch corrects something that is arguably wrong (even if never called due to an invariant), I'm going to land it.
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f713977f1629 Fix a case where a NULL stylesheet would be added to the nsStyleSheetService if nsLayoutUtils::StyloSupportedInCurrentProcess() returns false. r=xidorn
Comment 13•7 years ago
|
||
Backed out Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f713977f16299641ecd8d01ae9393d7ba4541a9c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Push with more failures (ignore the xpcshell netwerk ones): https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3c4afc844a6aacc68f1ee2c325143e2a7f569b17&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=134078376&repo=autoland [task 2017-09-29T15:33:51.372Z] 15:33:51 INFO - 87 INFO None88 INFO TEST-START | layout/style/test/test_additional_sheets.html [task 2017-09-29T15:34:13.655Z] 15:34:13 INFO - INFO | automation.py | Application ran for: 0:07:54.541903 [task 2017-09-29T15:34:13.656Z] 15:34:13 INFO - INFO | zombiecheck | Reading PID log: /tmp/tmpO7hvojpidlog [task 2017-09-29T15:34:14.297Z] 15:34:14 INFO - /data/tombstones does not exist; tombstone check skipped [task 2017-09-29T15:34:15.156Z] 15:34:15 INFO - mozcrash Copy/paste: /builds/worker/workspace/build/linux64-minidump_stackwalk /tmp/tmpP5HY4m/3212b427-7ffa-556b-5a33-f26039b70428.dmp /builds/worker/workspace/build/symbols [task 2017-09-29T15:34:23.231Z] 15:34:23 INFO - mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/3212b427-7ffa-556b-5a33-f26039b70428.dmp [task 2017-09-29T15:34:23.231Z] 15:34:23 INFO - mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/3212b427-7ffa-556b-5a33-f26039b70428.extra [task 2017-09-29T15:34:23.235Z] 15:34:23 WARNING - PROCESS-CRASH | layout/style/test/test_additional_sheets.html | application crashed [@ nsStyleSheetService::LoadAndRegisterSheet] [task 2017-09-29T15:34:23.236Z] 15:34:23 INFO - Crash dump filename: /tmp/tmpP5HY4m/3212b427-7ffa-556b-5a33-f26039b70428.dmp [task 2017-09-29T15:34:23.236Z] 15:34:23 INFO - Operating system: Android [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - 0.0.0 Linux 2.6.29-gea477bb #1 Wed Sep 26 11:04:45 PDT 2012 armv7l [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - CPU: arm [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - ARMv7 ARM Cortex-A8 features: swp,half,thumb,fastmult,vfpv2,edsp,neon,vfpv3 [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - 1 CPU [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - GPU: UNKNOWN [task 2017-09-29T15:34:23.242Z] 15:34:23 INFO - Crash reason: SIGSEGV [task 2017-09-29T15:34:23.243Z] 15:34:23 INFO - Crash address: 0x0 [task 2017-09-29T15:34:23.243Z] 15:34:23 INFO - Process uptime: not available [task 2017-09-29T15:34:23.243Z] 15:34:23 INFO - Thread 11 (crashed) [task 2017-09-29T15:34:23.244Z] 15:34:23 INFO - 0 libxul.so!nsStyleSheetService::LoadAndRegisterSheet [nsStyleSheetService.cpp:f713977f1629 : 181 + 0x0] [task 2017-09-29T15:34:23.244Z] 15:34:23 INFO - r0 = 0x000000b6 r1 = 0x67db4702 r2 = 0x67db4702 r3 = 0x67db4702 [task 2017-09-29T15:34:23.245Z] 15:34:23 INFO - r4 = 0x00000000 r5 = 0x5424e0a0 r6 = 0x00000000 r7 = 0x5299f448 [task 2017-09-29T15:34:23.245Z] 15:34:23 INFO - r8 = 0x5556f460 r9 = 0x00000002 r10 = 0x00000000 r12 = 0x00000003 [task 2017-09-29T15:34:23.245Z] 15:34:23 INFO - fp = 0x5299f748 sp = 0x5299f350 lr = 0x5bc05663 pc = 0x5bc0b156 [task 2017-09-29T15:34:23.245Z] 15:34:23 INFO - Found by: given as instruction pointer in context [task 2017-09-29T15:34:23.245Z] 15:34:23 INFO - 1 libxul.so!NS_InvokeByIndex [xptcinvoke_arm.cpp:f713977f1629 : 169 + 0x13] [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - r4 = 0x5299f440 r5 = 0x5bc0b04d r6 = 0x00000003 r7 = 0x5299f448 [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - r8 = 0x00000000 r9 = 0x00000002 r10 = 0x00000000 fp = 0x5299f748 [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - sp = 0x5299f430 pc = 0x5a799267 [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - 2 libxul.so!CallMethodHelper::Call [XPCWrappedNative.cpp:f713977f1629 : 1996 + 0xd] [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - r3 = 0x5299f4c0 r4 = 0x5299f490 r5 = 0x00000002 r6 = 0x5299f468 [task 2017-09-29T15:34:23.246Z] 15:34:23 INFO - r7 = 0x00000002 r8 = 0x00000000 r9 = 0x00000002 r10 = 0x00000000 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - fp = 0x5299f748 sp = 0x5299f460 pc = 0x5ac57a81 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - 3 libxul.so!XPCWrappedNative::CallMethod [XPCWrappedNative.cpp:f713977f1629 : 1282 + 0x5] [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r4 = 0x5299f5b0 r5 = 0x00000000 r6 = 0x5299f584 r7 = 0x5299f4b8 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r8 = 0x5299f4b0 r9 = 0x5299f588 r10 = 0x00000000 fp = 0x5299f748 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - sp = 0x5299f490 pc = 0x5ac57b6f [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - 4 libxul.so!XPC_WN_CallMethod [XPCWrappedNativeJSOps.cpp:f713977f1629 : 929 + 0x7] [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r4 = 0x00000001 r5 = 0x5299f5b0 r6 = 0x5299f584 r7 = 0x5299f598 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r8 = 0x5299f5a4 r9 = 0x5299f588 r10 = 0x00000000 fp = 0x5299f748 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - sp = 0x5299f568 pc = 0x5ac5c51b [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - 5 libxul.so!js::CallJSNative [jscntxtinlines.h:f713977f1629 : 293 + 0x3] [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r4 = 0x5299f6f0 r5 = 0x52c2f800 r6 = 0x5299f750 r7 = 0x00000000 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - r8 = 0x00000001 r9 = 0x5ac5c419 r10 = 0x5820f000 fp = 0x5299f748 [task 2017-09-29T15:34:23.247Z] 15:34:23 INFO - sp = 0x5299f628 pc = 0x5c51bfc1 [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - 6 libxul.so!js::InternalCallOrConstruct [Interpreter.cpp:f713977f1629 : 495 + 0x7] [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - r4 = 0x52c2f800 r5 = 0x5299f6f0 r6 = 0x00000000 r7 = 0x00000000 [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - r8 = 0x5299f66c r9 = 0x52cd8000 r10 = 0x5299f700 fp = 0x00000000 [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - sp = 0x5299f660 pc = 0x5c536185 [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - 7 libxul.so!js::Call [Interpreter.cpp:f713977f1629 : 559 + 0x3] [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - r4 = 0x5299f6f0 r5 = 0x00000000 r6 = 0x6ac47200 r7 = 0xffffff8c [task 2017-09-29T15:34:23.248Z] 15:34:23 INFO - r8 = 0x5299f770 r9 = 0x5299f6e0 r10 = 0x5299f700 fp = 0x00000000 [task 2017-09-29T15:34:23.249Z] 15:34:23 INFO - sp = 0x5299f6b8 pc = 0x5c536537 [task 2017-09-29T15:34:23.249Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.249Z] 15:34:23 INFO - 8 libxul.so!js::fun_apply [jsfun.cpp:f713977f1629 : 1302 + 0x3] [task 2017-09-29T15:34:23.250Z] 15:34:23 INFO - r3 = 0x5299f6f0 r4 = 0x5299f6e4 r5 = 0x5299f6f0 r6 = 0x00000001 [task 2017-09-29T15:34:23.250Z] 15:34:23 INFO - r7 = 0x5299fd80 r8 = 0x5299f770 r9 = 0x5299f6e0 r10 = 0x5299f700 [task 2017-09-29T15:34:23.251Z] 15:34:23 INFO - fp = 0x00000000 sp = 0x5299f6d0 pc = 0x5c8728fd [task 2017-09-29T15:34:23.251Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.251Z] 15:34:23 INFO - 9 libxul.so!js::CallJSNative [jscntxtinlines.h:f713977f1629 : 293 + 0x3] [task 2017-09-29T15:34:23.252Z] 15:34:23 INFO - r4 = 0x5299fb14 r5 = 0x52c2f800 r6 = 0x5299fd98 r7 = 0x00000000 [task 2017-09-29T15:34:23.252Z] 15:34:23 INFO - r8 = 0x00000001 r9 = 0x5c8726f9 r10 = 0x5820f000 fp = 0x5299fd90 [task 2017-09-29T15:34:23.253Z] 15:34:23 INFO - sp = 0x5299f9f0 pc = 0x5c51bfc1 [task 2017-09-29T15:34:23.253Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.253Z] 15:34:23 INFO - 10 libxul.so!js::InternalCallOrConstruct [Interpreter.cpp:f713977f1629 : 495 + 0x7] [task 2017-09-29T15:34:23.254Z] 15:34:23 INFO - r4 = 0x52c2f800 r5 = 0x5299fb14 r6 = 0x00000000 r7 = 0x00000000 [task 2017-09-29T15:34:23.254Z] 15:34:23 INFO - r8 = 0x5299fa34 r9 = 0x52cd8000 r10 = 0x00000000 fp = 0x5299fb50 [task 2017-09-29T15:34:23.254Z] 15:34:23 INFO - sp = 0x5299fa28 pc = 0x5c536185 [task 2017-09-29T15:34:23.255Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.255Z] 15:34:23 INFO - 11 libxul.so!js::jit::DoCallFallback [BaselineIC.cpp:f713977f1629 : 2586 + 0x7] [task 2017-09-29T15:34:23.255Z] 15:34:23 INFO - r4 = 0x5299fb74 r5 = 0x52c2f800 r6 = 0x5299fb08 r7 = 0x5299faeb [task 2017-09-29T15:34:23.256Z] 15:34:23 INFO - r8 = 0x5299fdc0 r9 = 0x0000004f r10 = 0x00000000 fp = 0x5299fb50 [task 2017-09-29T15:34:23.256Z] 15:34:23 INFO - sp = 0x5299fa80 pc = 0x5c5df9e1 [task 2017-09-29T15:34:23.257Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.257Z] 15:34:23 INFO - 12 0x5e66a122 [task 2017-09-29T15:34:23.257Z] 15:34:23 INFO - r4 = 0x5299fd48 r5 = 0x5299fd60 r6 = 0x5eb81bcc r7 = 0xffffff8c [task 2017-09-29T15:34:23.258Z] 15:34:23 INFO - r8 = 0x00003043 r9 = 0x558f6400 r10 = 0x00000003 fp = 0x5299fd90 [task 2017-09-29T15:34:23.258Z] 15:34:23 INFO - sp = 0x5299fd38 pc = 0x5e66a124 [task 2017-09-29T15:34:23.258Z] 15:34:23 INFO - Found by: call frame info [task 2017-09-29T15:34:23.259Z] 15:34:23 INFO - 13 libxul.so!nexttowardf + 0x177bb51 [task 2017-09-29T15:34:23.259Z] 15:34:23 INFO - sp = 0x5299fd54 pc = 0x5e657aa0 [task 2017-09-29T15:34:23.259Z] 15:34:23 INFO - Found by: stack scanning
Flags: needinfo?(bwerth)
Comment 14•7 years ago
|
||
Backed out for crashing in Android mochitests, e.g. layout/style/test/test_additional_sheets.html: https://hg.mozilla.org/integration/autoland/rev/776ded750b88f711f99dc2a658bb8b85f53235ff
Assignee | ||
Comment 15•7 years ago
|
||
Oh, I'm sorry I didn't test that more thoroughly. More is needed here. nsStyleSheetService::LoadAndRegisterSheet asserts that a successful return from the function changed in Part 1 means that two sheets were added (wrong).
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bwerth
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8913802 -
Flags: review?(xidorn+moz)
Assignee | ||
Comment 18•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=39a34416928a762d17020c148228909792933a7a
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8913802 -
Attachment is obsolete: true
Attachment #8913802 -
Flags: review?(xidorn+moz)
Assignee | ||
Comment 20•7 years ago
|
||
Xidorn, I had to significantly rework the patch, and I wasn't able to clear your review of Part 1. Would you please re-review?
Flags: needinfo?(bwerth) → needinfo?(xidorn+moz)
Assignee | ||
Comment 21•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab8a424794f30a293a9f180fbd86d02eebcb7b92
Updated•7 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Updated•7 years ago
|
Summary: Crash in mozilla::ServoStyleSet::AppendSheetOfType → stylo: Crash in mozilla::ServoStyleSet::AppendSheetOfType
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8913485 [details] Bug 1403694 Part 1: Prevent nsStyleSheetService from storing null servo stylesheets. https://reviewboard.mozilla.org/r/184828/#review190818 r=me with the new patch ::: layout/base/nsStyleSheetService.cpp:189 (Diff revision 4) > nsTArray<nsCOMPtr<nsIPresShell>> toNotify(mPresShells); > for (nsIPresShell* presShell : toNotify) { > if (presShell->StyleSet()) { > - StyleSheet* sheet = presShell->StyleSet()->IsGecko() ? geckoSheet > - : servoSheet; > + bool isGecko = presShell->StyleSet()->IsGecko(); > + // We always have a Gecko sheet; we sometimes have a Servo sheet. > + if (isGecko || servoSheetWasAdded) { I'm wondering whether this can just become an assertion, since on a process that stylo is not supported, no presshell with servo backend should be created... But it probably doesn't matter. Let's just leave the check here. You can consider adding an `else` branch to `MOZ_ASSERT_UNREACHABLE`, though.
Updated•7 years ago
|
Flags: needinfo?(xidorn+moz)
Comment hidden (mozreview-request) |
Comment 24•7 years ago
|
||
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e6fd9d522e93 Part 1: Prevent nsStyleSheetService from storing null servo stylesheets. r=xidorn
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e6fd9d522e93
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 26•7 years ago
|
||
(FWIW, as I mentioned in comment 7, I don't really believe this would fix anything, but we can see. If the crash doesn't stop, we can try to figure out the reason in a new bug, I guess.)
Comment 27•7 years ago
|
||
All the crash reports we've gotten for this signature are on Beta, so should we consider uplifting this there to see if it makes a difference?
Flags: needinfo?(bwerth)
Assignee | ||
Comment 28•7 years ago
|
||
I think it should be uplifted, but I don't recall which flags to set and to what values. I do have the details: [Feature/regressing bug #]: Fixes a gap in the implementation of nsStyleService, when it was extended to support Servo sheets. [User impact if declined]: We have no Steps To Reproduce, but the crash signature shows that this could crash from a NULL dereference in release under some unusual conditions. [Describe test coverage new/current, TBPL]: No new tests were generated. The fix plugs a theoretical problem that is appearing in crashes in the wild. [Risks and why]: Low. There's no code that should be relying on the previous behavior, because it would consistently lead to a crash. [String/UUID change made/needed]: none
Flags: needinfo?(bwerth)
Updated•7 years ago
|
Attachment #8913485 -
Flags: approval-mozilla-beta?
Comment on attachment 8913485 [details] Bug 1403694 Part 1: Prevent nsStyleSheetService from storing null servo stylesheets. Stylo related crash fix, let's hope this helps, Beta57+ Philipp FYI if you see this signature go away, please mark this as verified on 57. Thanks!
Flags: needinfo?(madperson)
Attachment #8913485 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 30•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/8c7645234f86
Reporter | ||
Comment 31•7 years ago
|
||
looks fixed with b6 (no more reports via crash stats during the last days).
You need to log in
before you can comment on or make changes to this bug.
Description
•