Bug 78068 – REGRESSION(r106668-r106889): Chromium page cycler tests (Intl2) performance regressions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Ryosuke Niwa]

Chromium page cycler Intl2 tests regressed in http://src.chromium.org/viewvc/chrome?view=rev&revision=120693 (second bump):
http://build.chromium.org/f/chromium/perf/xp-release-dual-core/intl2/report.html?history=150&rev=-1

The WebKit regression range is: http://trac.webkit.org/log/?rev=106889&stop_rev=106668&verbose=on

[Andreas Kling]

Created attachment 125986 [details]
Government cover-up of regression in r106756

[Andreas Kling]

Comment on attachment 125986 [details]
Government cover-up of regression in r106756

Clearing flags on attachment: 125986

Committed r107029: <http://trac.webkit.org/changeset/107029>

[Ryosuke Niwa]

Created attachment 126020 [details]
Instrument result on mac

Here's instrument result in the case this patch didn't fix the regression.

[Csaba Osztrogonác]

(In reply to comment #2)
> (From update of attachment 125986 [details])
> Clearing flags on attachment: 125986
> 
> Committed r107029: <http://trac.webkit.org/changeset/107029>

It broke 5 tests on GTK, Qt and SL and maybe on other platforms too:
--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-false-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-false-actual.txt 
@@ -8,8 +8,8 @@
 PASS document.getElementById("div1").isContentEditable is false
 PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-only"
 PASS document.getElementById("p2").contentEditable is "false"
-PASS document.getElementById("p2").isContentEditable is false
-PASS window.getComputedStyle(p2, "").getPropertyValue("-webkit-user-modify") is "read-only"
+FAIL document.getElementById("p2").isContentEditable should be false. Was true.
+FAIL window.getComputedStyle(p2, "").getPropertyValue("-webkit-user-modify") should be read-only. Was read-write.
 PASS successfullyParsed is true
 
 TEST COMPLETE



--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-true-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-true-actual.txt 
@@ -5,13 +5,13 @@
 
 PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
 PASS document.getElementById("div1").contentEditable is "true"
-PASS document.getElementById("div1").isContentEditable is true
+FAIL document.getElementById("div1").isContentEditable should be true. Was false.
 FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
-PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"
+FAIL window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") should be read-write. Was read-only.
 PASS document.getElementById("p2").getAttribute("contentEditable") is "true"
 PASS document.getElementById("p2").contentEditable is "true"
-PASS document.getElementById("p2").isContentEditable is true
-PASS window.getComputedStyle(p2, "").getPropertyValue("-webkit-user-modify") is "read-write"
+FAIL document.getElementById("p2").isContentEditable should be true. Was false.
+FAIL window.getComputedStyle(p2, "").getPropertyValue("-webkit-user-modify") should be read-write. Was read-only.
 PASS successfullyParsed is true
 
 TEST COMPLETE



--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/dom/HTMLElement/set-value-caseinsensitive-actual.txt 
@@ -5,9 +5,9 @@
 
 PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
 PASS document.getElementById("div1").contentEditable is "true"
-PASS document.getElementById("div1").isContentEditable is true
+FAIL document.getElementById("div1").isContentEditable should be true. Was false.
 FIXME: isContentEditable is not working properly. Related to https://bugs.webkit.org/show_bug.cgi?id=52058
-PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"
+FAIL window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") should be read-write. Was read-only.
 PASS document.getElementById("p1").getAttribute("contentEditable") is "false"
 PASS document.getElementById("p1").contentEditable is "false"
 PASS document.getElementById("p1").isContentEditable is false
@@ -19,11 +19,11 @@
 PASS document.getElementById("div3").hasAttribute("contentEditable") is true
 PASS document.getElementById("div3").contentEditable is "plaintext-only"
 PASS document.getElementById("div3").isContentEditable is true
-PASS window.getComputedStyle(div3, "").getPropertyValue("-webkit-user-modify") is "read-write-plaintext-only"
+FAIL window.getComputedStyle(div3, "").getPropertyValue("-webkit-user-modify") should be read-write-plaintext-only. Was read-write.
 PASS document.getElementById("p3").hasAttribute("contentEditable") is false
 PASS document.getElementById("p3").contentEditable is "inherit"
 PASS document.getElementById("p3").isContentEditable is true
-PASS window.getComputedStyle(p3, "").getPropertyValue("-webkit-user-modify") is "read-write-plaintext-only"
+FAIL window.getComputedStyle(p3, "").getPropertyValue("-webkit-user-modify") should be read-write-plaintext-only. Was read-write.
 PASS successfullyParsed is true
 
 TEST COMPLETE



--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/html/hidden-attr-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/html/hidden-attr-actual.txt 
@@ -16,7 +16,7 @@
 PASS getComputedStyle(test3, false).display is "block"
 PASS getComputedStyle(test4, false).display is "block"
 test4.setAttribute("hidden", "")
-PASS getComputedStyle(test4, false).display is "none"
+FAIL getComputedStyle(test4, false).display should be none. Was block.
 
 PASS successfullyParsed is true
 
@@ -25,3 +25,4 @@
 
 
 a
+b



--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/forms/input-align-image-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/forms/input-align-image-actual.txt 
@@ -7,15 +7,16 @@
         RenderText {#text} at (0,0) size 650x21
           text run at (0,0) width 650: "The following 4 images should be all be rendered exactly the same, aligned to the right side."
       RenderImage {INPUT} at (767,37) size 17x19
-      RenderBlock (anonymous) at (0,37) size 784x126
+      RenderBlock (anonymous) at (0,37) size 784x129
         RenderBR {BR} at (0,0) size 0x21
         RenderBR {BR} at (0,21) size 0x21
         RenderImage {INPUT} at (767,42) size 17x19
         RenderBR {BR} at (0,42) size 0x21
         RenderBR {BR} at (0,63) size 0x21
-        RenderImage {INPUT} at (767,84) size 17x19
-        RenderText {#text} at (0,0) size 0x0
-        RenderBR {BR} at (0,84) size 0x21
-        RenderBR {BR} at (0,105) size 0x21
-      RenderBlock {DIV} at (0,163) size 784x0
+        RenderImage {INPUT} at (0,84) size 17x19
+        RenderText {#text} at (17,87) size 4x21
+          text run at (17,87) width 4: " "
+        RenderBR {BR} at (0,0) size 0x0
+        RenderBR {BR} at (0,108) size 0x21
+      RenderBlock {DIV} at (0,166) size 784x0
         RenderImage {INPUT} at (767,0) size 17x19

[Ryosuke Niwa]

That's because void setProperty(const CSSProperty&, CSSProperty* slot = 0); doesn't call setNeedsStyleRecalc unlike every other setProperty :( What a disaster but what a confusing behavior difference. I don't understand why this particular version has to behave differently.

[Andreas Kling]

Committed r107061: <http://trac.webkit.org/changeset/107061>

[Julien Chaffraix]

For the record, we are still not back to our baseline after the patch. The government managed to cover up half of the regression though!

See 78070 that has more information about that.