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
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
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
https://hg.mozilla.org/mozilla-central/rev/565bb6d72f11
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.