- Revision
- 217701
- Author
- [email protected]
- Date
- 2017-06-01 22:55:28 -0700 (Thu, 01 Jun 2017)
Log Message
Ensure a good experience for ARES-6 error reporting
https://bugs.webkit.org/show_bug.cgi?id=171699
Reviewed by Filip Pizlo and Jon Davis.
This patch fixes a bug where we would silently fail running ARES-6. The bug
was that we were calling reportError with the wrong |this| value.
I also cleaned up a bit of the code around error reporting. We
now indicate which test failed, and update the status to reflect
that a failure happened.
This patch also modifies the CSS a bit to work better on smaller
screened devices. The CSS prevents the status from having a line
break both when an error is reported and when we're running the
benchmark.
* ARES-6/driver.js:
(Driver):
(Driver.prototype.reportError):
* ARES-6/results.js:
(Results.prototype.reportError):
(Results):
* ARES-6/styles.css:
(.start):
(#status):
(.failed):
(#status.failed):
(.test .failed:before):
(#magic):
(@media only screen and (max-width: 784px)):
(.test):
(p):
(@media only screen and (max-width: 320px)):
Modified Paths
Diff
Modified: trunk/PerformanceTests/ARES-6/driver.js (217700 => 217701)
--- trunk/PerformanceTests/ARES-6/driver.js 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/PerformanceTests/ARES-6/driver.js 2017-06-02 05:55:28 UTC (rev 217701)
@@ -34,7 +34,6 @@
this._magicCell = magicCell;
this._summary = new Stats(summaryCell, "summary");
this._key = key;
- this._hadErrors = false;
if (isInBrowser)
window[key] = this;
}
@@ -83,10 +82,16 @@
reportError(...args)
{
+ if (isInBrowser)
+ console.log("Error encountered: ", ...args);
+
this._benchmarks.get(this._benchmark).reportError(...args);
- this._recomputeSummary();
- this._hadErrors = true;
- this._iterate();
+
+ if (isInBrowser) {
+ this._statusCell.innerHTML = "Test failure \u2014 error in console";
+ this._statusCell.classList.add("failed");
+ } else
+ print("Test failure");
}
_recomputeSummary()
@@ -137,11 +142,10 @@
if (!this._benchmark) {
if (!this._numIterations) {
if (isInBrowser) {
- this._statusCell.innerHTML =
- (this._hadErrors ? "Failures encountered!" : "Restart");
+ this._statusCell.innerHTML = "Restart";
this.readyTrigger();
} else
- print(this._hadErrors ? "Failures encountered!" : "Success! Benchmark is now finished.");
+ print("Success! Benchmark is now finished.");
return;
}
this._numIterations--;
@@ -165,7 +169,7 @@
magicFrame.contentDocument.open();
magicFrame.contentDocument.write(
`<!DOCTYPE html><head><title>benchmark payload</title></head><body><script>` +
- `window._onerror_ = top.${this._key}.reportError;\n` +
+ `window._onerror_ = top.${this._key}.reportError.bind(top.${this._key});\n` +
`function reportResult()\n` +
`{\n` +
` var driver = top.${this._key};\n` +
Modified: trunk/PerformanceTests/ARES-6/results.js (217700 => 217701)
--- trunk/PerformanceTests/ARES-6/results.js 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/PerformanceTests/ARES-6/results.js 2017-06-02 05:55:28 UTC (rev 217701)
@@ -66,10 +66,11 @@
reportError(message, url, lineNumber)
{
- for (let subResult of Results.subResults)
- this[subResult].reportResult(Stats.error);
- if (isInBrowser)
- this._benchmark.cells.message.innerHTML = url + ":" + lineNumber + ": " + message;
+ if (isInBrowser) {
+ this._benchmark.cells.message.classList.remove('running');
+ this._benchmark.cells.message.classList.add('failed');
+ } else
+ print("Failed running benchmark");
}
}
Modified: trunk/PerformanceTests/ARES-6/styles.css (217700 => 217701)
--- trunk/PerformanceTests/ARES-6/styles.css 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/PerformanceTests/ARES-6/styles.css 2017-06-02 05:55:28 UTC (rev 217701)
@@ -91,7 +91,6 @@
background-color: transparent;
border: 5px solid #E7B135;
font-size: 2.4rem;
- line-height: 5.4rem;
font-weight: 600;
text-transform: uppercase;
color: #E7B135;
@@ -159,6 +158,22 @@
color: #ffffff;
}
+#status {
+ line-height: 5.4rem;
+}
+
+.failed {
+ color: #ff5744;
+}
+
+#status.failed {
+ font-size: 1.5rem;
+}
+
+.test .failed:before {
+ content: '\2716';
+}
+
.test .running:before {
content: '\25b8';
}
@@ -249,6 +264,10 @@
border-left: 0 solid transparent;
}
+#magic {
+ opacity: 0;
+}
+
@keyframes fade-in {
0% { opacity: 0; }
100% { opacity: 1; }
@@ -284,10 +303,35 @@
.start {
width: 100%;
}
-
.test {
flex: none;
-
width: 100%;
}
+ #status {
+ line-height: 4.3rem;
+ }
+ .start {
+ font-size: 2.1rem;
+ }
+ #status.failed {
+ font-size: 1.4rem;
+ }
+ p {
+ font-size: 1.6rem;
+ }
}
+
+@media only screen and (max-width: 320px) {
+ #status {
+ line-height: 3.8rem;
+ }
+ .start {
+ font-size: 1.9rem;
+ }
+ #status.failed {
+ font-size: 1.1rem;
+ }
+ p {
+ font-size: 1.4rem;
+ }
+}
Modified: trunk/PerformanceTests/ChangeLog (217700 => 217701)
--- trunk/PerformanceTests/ChangeLog 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/PerformanceTests/ChangeLog 2017-06-02 05:55:28 UTC (rev 217701)
@@ -1,3 +1,39 @@
+2017-06-01 Saam Barati <[email protected]>
+
+ Ensure a good experience for ARES-6 error reporting
+ https://bugs.webkit.org/show_bug.cgi?id=171699
+
+ Reviewed by Filip Pizlo and Jon Davis.
+
+ This patch fixes a bug where we would silently fail running ARES-6. The bug
+ was that we were calling reportError with the wrong |this| value.
+ I also cleaned up a bit of the code around error reporting. We
+ now indicate which test failed, and update the status to reflect
+ that a failure happened.
+
+ This patch also modifies the CSS a bit to work better on smaller
+ screened devices. The CSS prevents the status from having a line
+ break both when an error is reported and when we're running the
+ benchmark.
+
+ * ARES-6/driver.js:
+ (Driver):
+ (Driver.prototype.reportError):
+ * ARES-6/results.js:
+ (Results.prototype.reportError):
+ (Results):
+ * ARES-6/styles.css:
+ (.start):
+ (#status):
+ (.failed):
+ (#status.failed):
+ (.test .failed:before):
+ (#magic):
+ (@media only screen and (max-width: 784px)):
+ (.test):
+ (p):
+ (@media only screen and (max-width: 320px)):
+
2017-05-19 Ryosuke Niwa <[email protected]>
REGRESSION(r217118): Speedometer 2.0: Flight.js test is broken
Modified: trunk/Websites/browserbench.org/ARES-6/driver.js (217700 => 217701)
--- trunk/Websites/browserbench.org/ARES-6/driver.js 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/Websites/browserbench.org/ARES-6/driver.js 2017-06-02 05:55:28 UTC (rev 217701)
@@ -34,7 +34,6 @@
this._magicCell = magicCell;
this._summary = new Stats(summaryCell, "summary");
this._key = key;
- this._hadErrors = false;
if (isInBrowser)
window[key] = this;
}
@@ -83,10 +82,16 @@
reportError(...args)
{
+ if (isInBrowser)
+ console.log("Error encountered: ", ...args);
+
this._benchmarks.get(this._benchmark).reportError(...args);
- this._recomputeSummary();
- this._hadErrors = true;
- this._iterate();
+
+ if (isInBrowser) {
+ this._statusCell.innerHTML = "Test failure \u2014 error in console";
+ this._statusCell.classList.add("failed");
+ } else
+ print("Test failure");
}
_recomputeSummary()
@@ -137,11 +142,10 @@
if (!this._benchmark) {
if (!this._numIterations) {
if (isInBrowser) {
- this._statusCell.innerHTML =
- (this._hadErrors ? "Failures encountered!" : "Restart");
+ this._statusCell.innerHTML = "Restart";
this.readyTrigger();
} else
- print(this._hadErrors ? "Failures encountered!" : "Success! Benchmark is now finished.");
+ print("Success! Benchmark is now finished.");
return;
}
this._numIterations--;
@@ -165,7 +169,7 @@
magicFrame.contentDocument.open();
magicFrame.contentDocument.write(
`<!DOCTYPE html><head><title>benchmark payload</title></head><body><script>` +
- `window._onerror_ = top.${this._key}.reportError;\n` +
+ `window._onerror_ = top.${this._key}.reportError.bind(top.${this._key});\n` +
`function reportResult()\n` +
`{\n` +
` var driver = top.${this._key};\n` +
Modified: trunk/Websites/browserbench.org/ARES-6/results.js (217700 => 217701)
--- trunk/Websites/browserbench.org/ARES-6/results.js 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/Websites/browserbench.org/ARES-6/results.js 2017-06-02 05:55:28 UTC (rev 217701)
@@ -66,10 +66,11 @@
reportError(message, url, lineNumber)
{
- for (let subResult of Results.subResults)
- this[subResult].reportResult(Stats.error);
- if (isInBrowser)
- this._benchmark.cells.message.innerHTML = url + ":" + lineNumber + ": " + message;
+ if (isInBrowser) {
+ this._benchmark.cells.message.classList.remove('running');
+ this._benchmark.cells.message.classList.add('failed');
+ } else
+ print("Failed running benchmark");
}
}
Modified: trunk/Websites/browserbench.org/ARES-6/styles.css (217700 => 217701)
--- trunk/Websites/browserbench.org/ARES-6/styles.css 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/Websites/browserbench.org/ARES-6/styles.css 2017-06-02 05:55:28 UTC (rev 217701)
@@ -91,7 +91,6 @@
background-color: transparent;
border: 5px solid #E7B135;
font-size: 2.4rem;
- line-height: 5.4rem;
font-weight: 600;
text-transform: uppercase;
color: #E7B135;
@@ -159,6 +158,22 @@
color: #ffffff;
}
+#status {
+ line-height: 5.4rem;
+}
+
+.failed {
+ color: #ff5744;
+}
+
+#status.failed {
+ font-size: 1.5rem;
+}
+
+.test .failed:before {
+ content: '\2716';
+}
+
.test .running:before {
content: '\25b8';
}
@@ -249,6 +264,10 @@
border-left: 0 solid transparent;
}
+#magic {
+ opacity: 0;
+}
+
@keyframes fade-in {
0% { opacity: 0; }
100% { opacity: 1; }
@@ -284,10 +303,35 @@
.start {
width: 100%;
}
-
.test {
flex: none;
-
width: 100%;
}
+ #status {
+ line-height: 4.3rem;
+ }
+ .start {
+ font-size: 2.1rem;
+ }
+ #status.failed {
+ font-size: 1.4rem;
+ }
+ p {
+ font-size: 1.6rem;
+ }
}
+
+@media only screen and (max-width: 320px) {
+ #status {
+ line-height: 3.8rem;
+ }
+ .start {
+ font-size: 1.9rem;
+ }
+ #status.failed {
+ font-size: 1.1rem;
+ }
+ p {
+ font-size: 1.4rem;
+ }
+}
Modified: trunk/Websites/browserbench.org/ChangeLog (217700 => 217701)
--- trunk/Websites/browserbench.org/ChangeLog 2017-06-02 05:32:33 UTC (rev 217700)
+++ trunk/Websites/browserbench.org/ChangeLog 2017-06-02 05:55:28 UTC (rev 217701)
@@ -1,3 +1,39 @@
+2017-06-01 Saam Barati <[email protected]>
+
+ Ensure a good experience for ARES-6 error reporting
+ https://bugs.webkit.org/show_bug.cgi?id=171699
+
+ Reviewed by Filip Pizlo and Jon Davis.
+
+ This patch fixes a bug where we would silently fail running ARES-6. The bug
+ was that we were calling reportError with the wrong |this| value.
+ I also cleaned up a bit of the code around error reporting. We
+ now indicate which test failed, and update the status to reflect
+ that a failure happened.
+
+ This patch also modifies the CSS a bit to work better on smaller
+ screened devices. The CSS prevents the status from having a line
+ break both when an error is reported and when we're running the
+ benchmark.
+
+ * ARES-6/driver.js:
+ (Driver):
+ (Driver.prototype.reportError):
+ * ARES-6/results.js:
+ (Results.prototype.reportError):
+ (Results):
+ * ARES-6/styles.css:
+ (.start):
+ (#status):
+ (.failed):
+ (#status.failed):
+ (.test .failed:before):
+ (#magic):
+ (@media only screen and (max-width: 784px)):
+ (.test):
+ (p):
+ (@media only screen and (max-width: 320px)):
+
2017-05-19 Ryosuke Niwa <[email protected]>
REGRESSION(r217118): Speedometer 2.0: Flight.js test is broken
