Bug 152311

Summary: Add v3 UI to perf dashboard
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Perf DashboardAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, cdumez, dewei_zhu, jond, kling, koivisto, rniwa, slewis, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 152312    
Bug Blocks: 152324, 152354    
Attachments:
Description Flags
Adds v3 UI
none
Minor tweak cdumez: review+

Description Ryosuke Niwa 2015-12-15 13:16:46 PST
V3 UI is coming.
Comment 1 Ryosuke Niwa 2015-12-15 18:34:09 PST
Created attachment 267416 [details]
Adds v3 UI
Comment 2 Ryosuke Niwa 2015-12-15 19:51:15 PST
This new UI reduces the amount of data downloaded from 27MB to 3MB on our internal dashboard.
Comment 3 Ryosuke Niwa 2015-12-15 19:55:18 PST
Created attachment 267424 [details]
Minor tweak
Comment 4 Chris Dumez 2015-12-15 20:58:52 PST
Comment on attachment 267424 [details]
Minor tweak

View in context: https://bugs.webkit.org/attachment.cgi?id=267424&action=review

r=me with nits

> Websites/perf.webkit.org/public/v3/main.js:22
> +        var chartToolbar = new ChartsToolbar;

nit: chartsToolbar for consistency?

> Websites/perf.webkit.org/public/v3/components/chart-status-view.js:92
> +            this._comparisonClass =  status.className;

Nit: extra space

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:375
> +/*                var x1 = i ? (series[i - 1].x + point.x) / 2 : point.x;

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:678
> +/*console.log(TimeSeriesChart.computeTimeGrid(

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:691
> +/*

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/models/analysis-task.js:104
> +        // FIXME: The backend shouldn't create a separate bug rows per task for the same bug number.

"row" ?

> Websites/perf.webkit.org/public/v3/pages/analysis-category-toolbar.js:74
> +            this._filterCallback(this._filter);

this._filter is always null here is this intended?

> Websites/perf.webkit.org/public/v3/pages/chart-pane-status-view.js:37
> +            // selected ? '\u25B8' : '\u25B4'

did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:12
> +        this._startTime = new Date(2015, 10, 19, 0, 0, 0) - 60 * 24 * 3600 * 1000;

these hardcoded dates seem weird

> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:13
> +        this._endTime = new Date(2015, 10, 19, 0, 0, 0);

ditto

> Websites/perf.webkit.org/public/v3/pages/heading.js:12
> +

blank line

> Websites/perf.webkit.org/public/v3/pages/heading.js:176
> +/*

Did you mean to keep this in?

> Websites/perf.webkit.org/tools/bundle-v3-scripts.py:26
> +    minidifed_script = jsmin.communicate(input=bundled_script)[0]

minified_script?
Comment 5 Ryosuke Niwa 2015-12-15 21:19:00 PST
Comment on attachment 267424 [details]
Minor tweak

View in context: https://bugs.webkit.org/attachment.cgi?id=267424&action=review

Thanks for the review!

>> Websites/perf.webkit.org/public/v3/main.js:22
>> +        var chartToolbar = new ChartsToolbar;
> 
> nit: chartsToolbar for consistency?

Fixed

>> Websites/perf.webkit.org/public/v3/components/chart-status-view.js:92
>> +            this._comparisonClass =  status.className;
> 
> Nit: extra space

Fixed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:375
>> +/*                var x1 = i ? (series[i - 1].x + point.x) / 2 : point.x;
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:678
>> +/*console.log(TimeSeriesChart.computeTimeGrid(
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:691
>> +/*
> 
> Did you mean to keep this in?

Ditto.

>> Websites/perf.webkit.org/public/v3/models/analysis-task.js:104
>> +        // FIXME: The backend shouldn't create a separate bug rows per task for the same bug number.
> 
> "row" ?

Fixed.

>> Websites/perf.webkit.org/public/v3/pages/analysis-category-toolbar.js:74
>> +            this._filterCallback(this._filter);
> 
> this._filter is always null here is this intended?

Yes, this is intended. We clear the filter when we change the category.

>> Websites/perf.webkit.org/public/v3/pages/chart-pane-status-view.js:37
>> +            // selected ? '\u25B8' : '\u25B4'
> 
> did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:12
>> +        this._startTime = new Date(2015, 10, 19, 0, 0, 0) - 60 * 24 * 3600 * 1000;
> 
> these hardcoded dates seem weird

Oops, this is supposed to be Date.now().  Fixed.

>> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:13
>> +        this._endTime = new Date(2015, 10, 19, 0, 0, 0);
> 
> ditto

Ditto.

>> Websites/perf.webkit.org/public/v3/pages/heading.js:12
>> +
> 
> blank line

Removed.

>> Websites/perf.webkit.org/public/v3/pages/heading.js:176
>> +/*
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/tools/bundle-v3-scripts.py:26
>> +    minidifed_script = jsmin.communicate(input=bundled_script)[0]
> 
> minified_script?

Fixed.
Comment 6 Ryosuke Niwa 2015-12-15 21:19:31 PST
Committed r194130: <http://trac.webkit.org/changeset/194130>