Closed Bug 1398479 Opened 7 years ago Closed 7 years ago

stylo: Assertion failure: !Servo_DeclarationBlock_PropertyIsSet(mDecl, aId) (Presentation attribute mappers should never attempt to set the same property twice), at layout/style/ServoSpecifiedValues.cpp:40

Categories

(Core :: CSS Parsing and Computation, defect, P2)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 + fixed

People

(Reporter: bc, Assigned: xidorn)

References

Details

(Keywords: assertion, regression, regressionwindow-wanted)

Attachments

(3 files, 1 obsolete file)

[Tracking Requested - why for this release]: regression 1. NSFW http://www.babechannels.co.uk/studio66.html 2. Assertion failure: !Servo_DeclarationBlock_PropertyIsSet(mDecl, aId) (Presentation attribute mappers should never attempt to set the same property twice), at /builds/worker/workspace/build/src/layout/style/ServoSpecifiedValues.cpp:40 Windows / Linux at least. Thread 0 (crashed) 0 libxul.so!mozilla::ServoSpecifiedValues::PropertyIsSet [ServoSpecifiedValues.cpp:3c96d611ebd6 : 38 + 0x0] rax = 0x0000000000000000 rdx = 0x0000000000000000 rcx = 0x0000000000000b40 rbx = 0x00007ffcc62df5d0 rsi = 0x00007f72d159b730 rdi = 0x00007f72d159a500 rbp = 0x00007ffcc62df520 rsp = 0x00007ffcc62df520 r8 = 0x00007f72d159b730 r9 = 0x00007f72d28a1080 r10 = 0x00007ffcc62dedb0 r11 = 0x0000000000000000 r12 = 0x00000000000000ab r13 = 0x00007f729607ed80 r14 = 0x00007f7297069000 r15 = 0x0000000000000003 rip = 0x00007f72c1c22da9 Found by: given as instruction pointer in context 1 libxul.so!mozilla::GenericSpecifiedValues::SetPixelValueIfUnset [GenericSpecifiedValuesInlines.h:3c96d611ebd6 : 110 + 0x5] rbx = 0x00007ffcc62df5d0 rbp = 0x00007ffcc62df550 rsp = 0x00007ffcc62df530 r12 = 0x00000000000000ab r13 = 0x00007f729607ed80 r14 = 0x00007f7297069000 r15 = 0x0000000000000003 rip = 0x00007f72c1448c7f Found by: call frame info 2 libxul.so!mozilla::dom::HTMLTableElement::MapAttributesIntoRule [HTMLTableElement.cpp:3c96d611ebd6 : 1023 + 0x19] rbx = 0x00007ffcc62df5d0 rbp = 0x00007ffcc62df5b0 rsp = 0x00007ffcc62df560 r12 = 0x00007f729607ed40 r13 = 0x00007f729607ed80 r14 = 0x00007f7297069000 r15 = 0x0000000000000003 rip = 0x00007f72c14a103f Found by: call frame info 3 libxul.so!nsMappedAttributes::LazilyResolveServoDeclaration [nsMappedAttributes.cpp:3c96d611ebd6 : 371 + 0x9] rbx = 0x00007f729607ed40 rbp = 0x00007ffcc62df610 rsp = 0x00007ffcc62df5c0 r12 = 0x00007ffcc62df5d0 r13 = 0x00007f7297069000 r14 = 0x00007f72970f85c0 r15 = 0x0000000000000000 rip = 0x00007f72c0c2a693 Found by: call frame info 4 libxul.so!nsHTMLStyleSheet::CalculateMappedServoDeclarations [nsHTMLStyleSheet.cpp:3c96d611ebd6 : 581 + 0x8] rbx = 0x00007ffcc62df620 rbp = 0x00007ffcc62df660 rsp = 0x00007ffcc62df620 r12 = 0x00007f7297069000 r13 = 0x00007ffcc62df728 r14 = 0x00007f72970f85c0 r15 = 0x0000000000000000 rip = 0x00007f72c1c9ff94 Found by: call frame info 5 libxul.so!mozilla::ServoStyleSet::ResolveMappedAttrDeclarationBlocks [ServoStyleSet.cpp:3c96d611ebd6 : 394 + 0x9] rbx = 0x00007f729700db70 rbp = 0x00007ffcc62df680 rsp = 0x00007ffcc62df670 r12 = 0x00007ffcc62df7b8 r13 = 0x00007ffcc62df728 r14 = 0x00007f72970f85c0 r15 = 0x0000000000000000 rip = 0x00007f72c1c23b66
Priority: -- → P2
Attached file simplified testcase (deleted) —
This is a testcase which reproduce the identical assertion. Note that it isn't simplified from the page. It is constructed from the observation of the related code in the stack. The method HTMLTableElement::MapAttributesIntoRule can indeed try to set the same property multiple times [1], so the assertion as well as comment in ServoSpecifiedValues::PropertyIsSet [2] doesn't really makes sense. [1] https://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/dom/html/HTMLTableElement.cpp#1006-1014,1020-1025 [2] https://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/layout/style/ServoSpecifiedValues.cpp#28-42
Given that we have a LonghandIdSet in PropertyDeclarationBlock, I think we can do O(1) lookup for Servo_DeclarationBlock_PropertyIsSet, and thus we can reliably return the correct value for ServoSpecifiedValues::PropertyIsSet in release build without too much overhead, I think.
Assignee: nobody → xidorn+moz
Comment on attachment 8906440 [details] Use the LonghandIdSet to check whether a property is set. https://reviewboard.mozilla.org/r/178156/#review183476
Attachment #8906440 - Flags: review?(manishearth) → review+
Comment on attachment 8906441 [details] Bug 1398479 - Always return correct value for ServoSpecifiedValues::PropertyIsSet. https://reviewboard.mozilla.org/r/178158/#review183478
Attachment #8906441 - Flags: review?(manishearth) → review+
> The method HTMLTableElement::MapAttributesIntoRule can indeed try to set the same property multiple times [1], so the assertion as well as comment in ServoSpecifiedValues::PropertyIsSet [2] doesn't really makes sense. Generally the idea was in such cases that we should just change the logic in MapAttributesIntoRule so that it doesn't set the property multiple times, but this is a more robust fix.
Attached file Servo PR (deleted) —
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s f8c888151761 -d 8aac9fa52e8a: rebasing 419301:f8c888151761 "Use the LonghandIdSet to check whether a property is set. r=manishearth" merging servo/components/style/properties/declaration_block.rs merging servo/ports/geckolib/glue.rs rebasing 419302:e593c0d20741 "Bug 1398479 - Always return correct value for ServoSpecifiedValues::PropertyIsSet. r=manishearth" (tip) merging layout/style/crashtests/crashtests.list warning: conflicts while merging layout/style/crashtests/crashtests.list! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Attachment #8906440 - Attachment is obsolete: true
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/565bb6d72f11 Always return correct value for ServoSpecifiedValues::PropertyIsSet. r=manishearth
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: