<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[198464] trunk/Websites/perf.webkit.org</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/198464">198464</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2016-03-18 20:29:44 -0700 (Fri, 18 Mar 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Perf Dashboard v3 confuses better and worse on A/B task page
https://bugs.webkit.org/show_bug.cgi?id=155675
&lt;rdar://problem/25208723&gt;

Reviewed by Joseph Pecoraro.

The analysis results viewer on v3 UI sometimes treats regressions as progressions and vice versa when
the first set (i.e. set A) of the revisions used in an A/B testing never appears in the original graph,
and its latest commit time matches that of the second set, which appears in the original graph.

Because the analysis results viewer compares results in the increasing row number, this results in
B to be compared to A instead of A to be compared to B. Fixed the bug by preventing the wrong ordering
to occur in _buildRowsForPointsAndTestGroups by always inserting a root set A before B when B appears
and A doesn't appear in the original graph.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer.prototype._collectRootSetsInTestGroups): Remember the succeeding root set B
when creating an entry for root set A.
(AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Fixed the bug. Also un-duplicated
the code to create a new row.
(AnalysisResultsViewer.RootSetInTestGroup): Now takes a succeeding root set. e.g. it's B for A and
undefined for B in A/B testing.
(AnalysisResultsViewer.RootSetInTestGroup.prototype.succeedingRootSet): Added.
* public/v3/components/time-series-chart.js:
(TimeSeriesChart.computeTimeGrid): Fixed the bug that we would end up showing 0 AM instead of dates
when both dates and months change.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebsitesperfwebkitorgChangeLog">trunk/Websites/perf.webkit.org/ChangeLog</a></li>
<li><a href="#trunkWebsitesperfwebkitorgpublicv3componentsanalysisresultsviewerjs">trunk/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js</a></li>
<li><a href="#trunkWebsitesperfwebkitorgpublicv3componentstimeserieschartjs">trunk/Websites/perf.webkit.org/public/v3/components/time-series-chart.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebsitesperfwebkitorgChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/ChangeLog (198463 => 198464)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/ChangeLog        2016-03-19 02:50:46 UTC (rev 198463)
+++ trunk/Websites/perf.webkit.org/ChangeLog        2016-03-19 03:29:44 UTC (rev 198464)
</span><span class="lines">@@ -1,5 +1,34 @@
</span><span class="cx"> 2016-03-18  Ryosuke Niwa  &lt;rniwa@webkit.org&gt;
</span><span class="cx"> 
</span><ins>+        Perf Dashboard v3 confuses better and worse on A/B task page
+        https://bugs.webkit.org/show_bug.cgi?id=155675
+        &lt;rdar://problem/25208723&gt;
+
+        Reviewed by Joseph Pecoraro.
+
+        The analysis results viewer on v3 UI sometimes treats regressions as progressions and vice versa when
+        the first set (i.e. set A) of the revisions used in an A/B testing never appears in the original graph,
+        and its latest commit time matches that of the second set, which appears in the original graph.
+
+        Because the analysis results viewer compares results in the increasing row number, this results in
+        B to be compared to A instead of A to be compared to B. Fixed the bug by preventing the wrong ordering
+        to occur in _buildRowsForPointsAndTestGroups by always inserting a root set A before B when B appears
+        and A doesn't appear in the original graph.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer.prototype._collectRootSetsInTestGroups): Remember the succeeding root set B
+        when creating an entry for root set A.
+        (AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Fixed the bug. Also un-duplicated
+        the code to create a new row.
+        (AnalysisResultsViewer.RootSetInTestGroup): Now takes a succeeding root set. e.g. it's B for A and
+        undefined for B in A/B testing.
+        (AnalysisResultsViewer.RootSetInTestGroup.prototype.succeedingRootSet): Added.
+        * public/v3/components/time-series-chart.js:
+        (TimeSeriesChart.computeTimeGrid): Fixed the bug that we would end up showing 0 AM instead of dates
+        when both dates and months change.
+
+2016-03-18  Ryosuke Niwa  &lt;rniwa@webkit.org&gt;
+
</ins><span class="cx">         Add unit tests for measurement-set.js and measurement-adapter.js
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=155673
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgpublicv3componentsanalysisresultsviewerjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js (198463 => 198464)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js        2016-03-19 02:50:46 UTC (rev 198463)
+++ trunk/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js        2016-03-19 03:29:44 UTC (rev 198464)
</span><span class="lines">@@ -139,7 +139,7 @@
</span><span class="cx">         for (var group of this._testGroups) {
</span><span class="cx">             var sortedSets = group.requestedRootSets();
</span><span class="cx">             for (var i = 0; i &lt; sortedSets.length; i++)
</span><del>-                rootSetsInTestGroups.push(new AnalysisResultsViewer.RootSetInTestGroup(group, sortedSets[i]));
</del><ins>+                rootSetsInTestGroups.push(new AnalysisResultsViewer.RootSetInTestGroup(group, sortedSets[i], sortedSets[i + 1]));
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         return rootSetsInTestGroups;
</span><span class="lines">@@ -191,14 +191,20 @@
</span><span class="cx">             }
</span><span class="cx"> 
</span><span class="cx">             var groupTime = entry.rootSet().latestCommitTime();
</span><ins>+            var newRow = new ResultsTableRow(null, entry.rootSet());
+            rowToMatchingRootSets.set(newRow, [entry]);
+
</ins><span class="cx">             for (var i = 0; i &lt; rowList.length; i++) {
</span><span class="cx">                 if (rowList[i] instanceof AnalysisResultsViewer.ExpandableRow)
</span><span class="cx">                     continue;
</span><span class="cx"> 
</span><ins>+                if (rowList[i].rootSet().equals(entry.succeedingRootSet())) {
+                    rowList.splice(i, 0, newRow);
+                    return;
+                }
+
</ins><span class="cx">                 var rowTime = rowList[i].rootSet().latestCommitTime();
</span><span class="cx">                 if (rowTime &gt; groupTime) {
</span><del>-                    var newRow = new ResultsTableRow(null, entry.rootSet());
-                    rowToMatchingRootSets.set(newRow, [entry]);
</del><span class="cx">                     rowList.splice(i, 0, newRow);
</span><span class="cx">                     return;
</span><span class="cx">                 }
</span><span class="lines">@@ -213,9 +219,7 @@
</span><span class="cx">                             var newCommit = entry.rootSet().commitForRepository(repository);
</span><span class="cx">                             var rowCommit = rowList[j].rootSet().commitForRepository(repository);
</span><span class="cx">                             if (!rowCommit || newCommit.time() &lt; rowCommit.time()) {
</span><del>-                                var row = new ResultsTableRow(null, entry.rootSet());
-                                rowToMatchingRootSets.set(row, [entry]);
-                                rowList.splice(j, 0, row);
</del><ins>+                                rowList.splice(j, 0, newRow);
</ins><span class="cx">                                 return;
</span><span class="cx">                             }
</span><span class="cx">                         }
</span><span class="lines">@@ -352,16 +356,18 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> AnalysisResultsViewer.RootSetInTestGroup = class {
</span><del>-    constructor(testGroup, rootSet)
</del><ins>+    constructor(testGroup, rootSet, succeedingRootSet)
</ins><span class="cx">     {
</span><span class="cx">         console.assert(testGroup instanceof TestGroup);
</span><span class="cx">         console.assert(rootSet instanceof RootSet);
</span><span class="cx">         this._testGroup = testGroup;
</span><span class="cx">         this._rootSet = rootSet;
</span><ins>+        this._succeedingRootSet = succeedingRootSet;
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     testGroup() { return this._testGroup; }
</span><span class="cx">     rootSet() { return this._rootSet; }
</span><ins>+    succeedingRootSet() { return this._succeedingRootSet; }
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> AnalysisResultsViewer.TestGroupStackingBlock = class {
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgpublicv3componentstimeserieschartjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/public/v3/components/time-series-chart.js (198463 => 198464)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/public/v3/components/time-series-chart.js        2016-03-19 02:50:46 UTC (rev 198463)
+++ trunk/Websites/perf.webkit.org/public/v3/components/time-series-chart.js        2016-03-19 03:29:44 UTC (rev 198464)
</span><span class="lines">@@ -579,6 +579,7 @@
</span><span class="cx">         var result = [];
</span><span class="cx"> 
</span><span class="cx">         var previousDate = null;
</span><ins>+        var previousMonth = null;
</ins><span class="cx">         var previousHour = null;
</span><span class="cx">         while (currentTime &lt;= max) {
</span><span class="cx">             var time = new Date(currentTime);
</span><span class="lines">@@ -590,7 +591,7 @@
</span><span class="cx">             iterator.next(currentTime);
</span><span class="cx"> 
</span><span class="cx">             var label;
</span><del>-            if (date == previousDate)
</del><ins>+            if (date == previousDate &amp;&amp; month == previousMonth)
</ins><span class="cx">                 label = hourLabel;
</span><span class="cx">             else {
</span><span class="cx">                 label = `${month}/${date}`;
</span><span class="lines">@@ -601,6 +602,7 @@
</span><span class="cx">             result.push({time: time, label: label});
</span><span class="cx"> 
</span><span class="cx">             previousDate = date;
</span><ins>+            previousMonth = month;
</ins><span class="cx">             previousHour = hour;
</span><span class="cx">         }
</span><span class="cx">         
</span></span></pre>
</div>
</div>

</body>
</html>