Closed
Bug 1089463
Opened 10 years ago
Closed 10 years ago
crash in mozilla::RestyleTracker::AddPendingRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint)
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jbecerra, Assigned: heycam)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files)
441 bytes,
text/html
|
Details | |
2.58 KB,
patch
|
bzbarsky
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-056ffc70-194c-4602-a5d0-2c99d2141020. ============================================================= It's #25 in the list of top crashers for Firefox 36 (nightly), and it's mostly being reported on Windows 8.1, 7, and XP. It's also in the list of explosive reports. There are no comments so far. It's been happening for a while, but it looks like the number of crashers started to increase around 10/13. More reports at: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3ARestyleTracker%3A%3AAddPendingRestyle%28mozilla%3A%3Adom%3A%3AElement%2A%2C+nsRestyleHint%2C+nsChangeHint%29 0 xul.dll mozilla::RestyleTracker::AddPendingRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) layout/base/RestyleTracker.h 1 xul.dll mozilla::RestyleManager::PostRestyleEventCommon(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, bool) layout/base/RestyleManager.cpp 2 xul.dll mozilla::RestyleManager::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) layout/base/RestyleManager.cpp 3 xul.dll PresShell::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int) layout/base/nsPresShell.cpp 4 xul.dll nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) content/base/src/nsNodeUtils.cpp 5 xul.dll mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) content/base/src/Element.cpp 6 xul.dll nsStyledElementNotElementCSSInlineStyle::SetInlineStyleRule(mozilla::css::StyleRule*, nsAString_internal const*, bool) content/base/src/nsStyledElement.cpp 7 xul.dll nsDOMCSSAttributeDeclaration::SetCSSDeclaration(mozilla::css::Declaration*) layout/style/nsDOMCSSAttrDeclaration.cpp 8 xul.dll nsDOMCSSDeclaration::RemoveProperty(nsCSSProperty) layout/style/nsDOMCSSDeclaration.cpp 9 xul.dll nsDOMCSSAttributeDeclaration::SetPropertyValue(nsCSSProperty, nsAString_internal const&) layout/style/nsDOMCSSAttrDeclaration.cpp 10 xul.dll mozilla::dom::CSS2PropertiesBinding::set_display obj-firefox/dom/bindings/CSS2PropertiesBinding.cpp 11 xul.dll mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp 12 mozjs.dll js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp 13 mozjs.dll js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp 14 mozjs.dll js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp 15 mozjs.dll js::CallSetter(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>), unsigned int, bool, JS::MutableHandle<JS::Value>) js/src/jscntxtinlines.h 16 mozjs.dll js::SetPropertyIgnoringNamedGetter(JSContext*, js::BaseProxyHandler const*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>, bool, bool, JS::MutableHandle<JS::Value>) js/src/proxy/BaseProxyHandler.cpp 17 xul.dll mozilla::dom::DOMProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>) dom/bindings/DOMJSProxyHandler.cpp 18 mozjs.dll js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>) js/src/proxy/Proxy.cpp 19 mozjs.dll js::proxy_SetGeneric(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>, bool) js/src/proxy/Proxy.cpp 20 mozjs.dll JSObject::nonNativeSetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>, bool) js/src/jsobj.cpp 21 mozjs.dll js::DirectProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>) js/src/proxy/DirectProxyHandler.cpp 22 mozjs.dll js::CrossCompartmentWrapper::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>) js/src/proxy/CrossCompartmentWrapper.cpp 23 mozjs.dll js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>) js/src/proxy/Proxy.cpp 24 mozjs.dll js::proxy_SetGeneric(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>, bool) js/src/proxy/Proxy.cpp 25 mozjs.dll JSObject::nonNativeSetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>, bool) js/src/jsobj.cpp 26 mozjs.dll js::SetObjectElement(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, bool, JS::Handle<JSScript*>, unsigned char*) js/src/vm/Interpreter.cpp 27 mozjs.dll js::jit::DoSetElemFallback js/src/jit/BaselineIC.cpp 28 @0x549fda
Null curData, looks like bug 931668 and bug 1062732: if (!(curData->mRestyleHint & eRestyle_ForceDescendants)) { 59ed3fca f70100020000 test dword ptr [ecx],200h ds:002b:00000000=???????? ni dbaron since heycam has an away message.
Flags: needinfo?(dbaron)
Example crash report: bp-96094435-2013-4d66-b0d0-ebd0d2141027 Probably don't have time to look until next week. Seems like it's equally big in 35 and 36.
... until *at least* next week.
Updated•10 years ago
|
Flags: needinfo?(cam)
Assignee | ||
Comment 4•10 years ago
|
||
We are getting our restyle bits and mPendingRestyles contents out of sync. In CollectRestyles, we can return early when |element->GetCrossShadowCurrentDoc() != collector->tracker->Document()| and when we do, we leave the restyle bits intact on the element, even though up in ProcessOneRestyle we'll clear mRestyleRoots. When we re-insert the element, it will have restyle bits (and for our crash here it must have a root bit) but no entry in mRestyleRoots. If we then restyle a descendant of this element, we'll find it when searching for a restyle root and find no data for it in mRestyleRoots. There are couple of places that end up clearing restyle bits on the element that we removed, avoiding ending up in this inconsistent state: (1) in Element::BindToTree, we'll clear the restyle flags if we just inserted into a document: http://mxr.mozilla.org/mozilla-central/source/dom/base/Element.cpp#1466 but this doesn't happen if we're inserting into a shadow tree, which currently isn't considered as an insertion into the document (though I think Boris and William were thinking of changing that?) (2) even if we do insert into a shadow tree, if that causes frames to get constructed, we'll clear the restyle flags here: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#9812 So to trigger the crash the element that we remove and reinsert must be display:none and it must be inserted into a shadow tree.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Flags: needinfo?(cam)
Assignee | ||
Comment 5•10 years ago
|
||
I have no idea if the other flags that are cleared in the |if (aDocument)| branch make sense to be cleared when inserting into a shadow tree too.
Attachment #8520423 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(dbaron)
Comment 6•10 years ago
|
||
Comment on attachment 8520423 [details] [diff] [review] patch I think you should definitely clear NODE_NEEDS_FRAME | NODE_DESCENDANTS_NEED_FRAMES. It probably doesn't matter too much on the FORCE_XBL_BINDINGS bit, but clearing it would be more consistent.
Attachment #8520423 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=25c52a3b7487
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b77a97a378b
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8520423 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: bug 1062732 [User impact if declined]: crashes with certain restyling operations [Describe test coverage new/current, TBPL]: tested manually, tbpl link above, just landed on inbound [Risks and why]: low, small change [String/UUID change made/needed]: N/A
Attachment #8520423 -
Flags: approval-mozilla-aurora?
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9b77a97a378b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•10 years ago
|
Attachment #8520423 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/e6c35fc2dc05
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•