Because you're worth it.
Created attachment 123652 [details] Patch
Attachment 123652 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/dom/StyledElement.h:93: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 123652 [details] Patch Attachment 123652 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11212804 New failing tests: fast/css/getComputedStyle/computed-style-border-image.html
Comment on attachment 123652 [details] Patch It would be good to add some comments to NamedNodeMap about the refactoring plans. The class is getting increasingly confusing as a place to hang random stuff.
Committed r105738: <http://trac.webkit.org/changeset/105738>
Reopen, because it made fast/css/getComputedStyle/computed-style-border-image.html crash (at least) on Qt. Could you check what happened?
(In reply to comment #6) > Reopen, because it made fast/css/getComputedStyle/computed-style-border-image.html crash (at least) on Qt. Could you check what happened? Naturally. Here's the stack: #0 WebCore::CSSValue::cssText (this=0x0) at CSSValue.h:153 #1 0x013fddbd in WebCore::CSSValueList::customCssText () at /Users/andreaskling/src/WebKit/Source/WebCore/css/CSSValueList.cpp:133 #2 0x013fcd0a in WebCore::CSSValue::cssText (this=<value temporarily unavailable, due to optimizations>) at /Users/andreaskling/src/WebKit/Source/WebCore/css/CSSValue.cpp:145 #3 0x013b1066 in WebCore::CSSProperty::cssText (this=0x2fb5990) at /Users/andreaskling/src/WebKit/Source/WebCore/css/CSSProperty.cpp:32 #4 0x01386ff1 in WebCore::CSSMutableStyleDeclaration::cssText () at /Users/andreaskling/src/WebKit/Source/WebCore/css/CSSMutableStyleDeclaration.cpp:862 #5 0x01c3abfa in WebCore::StyledElement::updateStyleAttribute (this=0x2fb5f20) at /Users/andreaskling/src/WebKit/Source/WebCore/dom/StyledElement.cpp:116 #6 0x012e3da6 in WebCore::StyledElement::ensureInlineStyleDecl (this=0x2fb5f20) at Element.h:495 #7 0x017cacae in WebCore::jsElementStyle (exec=0xc8ea1a0, slotBase={u = {asInt64 = -21284731472, asDouble = -nan(0xffffb0b54c5b0), asBits = {payload = 190105008, tag = -5}}}) at /Users/andreaskling/Source/Apple/Build/Release/DerivedSources/WebCore/JSElement.cpp:270 I wonder why we have null CSSValues in the CSSValueList here, this smells fishy.
Looks like I exposed a bug that was introduced in <http://trac.webkit.org/changeset/105502>
Committed r105999: <http://trac.webkit.org/changeset/105999>
(In reply to comment #9) > Committed r105999: <http://trac.webkit.org/changeset/105999> I'm worried that this caused a performance regression. Here is the graph: http://build.chromium.org/f/chromium/perf/mac-release-10.5/dhtml/report.html?history=150&rev=-1 I'm running this test with all Chromium changes up to the latest WebKit roll, which included only these revisions: http://trac.webkit.org/log/?rev=106007&stop_rev=105998&verbose=on
We did a perf test right before this change, and the regression wasn't present. I'm going to rever this tomorrow morning and try again just for confirmation. Let me know if you have an objection.
(In reply to comment #11) > We did a perf test right before this change, and the regression wasn't present. I'm going to rever this tomorrow morning and try again just for confirmation. Let me know if you have an objection. Feel free to roll out, I won't have a chance to look until tomorrow anyway. :/
(In reply to comment #12) > Feel free to roll out, I won't have a chance to look until tomorrow anyway. :/ Thanks! Just did. We'll know by tomorrow if this was the root cause. Sorry for the bother.
(In reply to comment #13) > Thanks! Just did. We'll know by tomorrow if this was the root cause. Sorry for the bother. http://trac.webkit.org/changeset/106201 was the rollout, and it did fix this performance regression.
Reopening to roll back in.
Created attachment 124878 [details] Patch v2 AFAICT the performance regression was introduced by causing us to unnecessarily serialize the inline style declaration into the element's 'style' attribute a lot more often. This happened because I used Element::attributes(false) to force creation of the element's attribute map. This time around, I've added a single-purpose ensureAttributeMap() for that instead.
Comment on attachment 124878 [details] Patch v2 Attachment 124878 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11386328 New failing tests: fast/doctypes/002.html
Committed r106435: <http://trac.webkit.org/changeset/106435>