[webkit-reviews] review requested: [Bug 22357] Crash when setting className from js : [Attachment 25932] Potential patch for issue 22357

[bugzilla-daemon at webkit.org]

Glenn Wilson <gwilson at google.com> has asked  for review:
Bug 22357: Crash when setting className from js
https://bugs.webkit.org/show_bug.cgi?id=22357

Attachment 25932: Potential patch for issue 22357
https://bugs.webkit.org/attachment.cgi?id=25932&action=review

------- Additional Comments from Glenn Wilson <gwilson at google.com>
Here is a potential patch for this issue.

So it seems like this problem would arise when a css class is applied to an
element that has no mappedAttributes.

>From what I can tell, the issue arises in a difference in assertions between
line 640 of CSSStyleSelector and StyledElement::classNames().  Specifically,
when looking for styles to apply, CSSStyleSelector::matchRules checks that this
element has a class applied (hasClass) without checking that it has mapped
attributes set.  Typically, this isn't a problem, because the class attribute
is a mapped attribute itself!

But on line 642, when StyledElement::classNames() is called, the classNames()
method asserts that this element not only has a class set, but also has
mappedAttributes set.  This will fail (and crash) for elements that have the
class attribute set but no mappedAttributes.  In this repro case, it's an svg
ellipse created on the fly with Javascript.

So, to fix this, I added one additional check on line 640 that does not enter
this block if mappedAttributes is not set.  In essence, this changes
CSSStyleSelector::matchRules to check on the same values that
StyledElement::classNames does.

(Since m_styledElement is ASSERT'ed on the next line, it must be valid anyway,
so the call is being done early.  If anything, this makes the ASSERT on 641
redundant.  Let me know if this should be removed.)