[webkit-reviews] review denied: [Bug 195912] Fix a bug from r226303 that latest build time is not correctly calculated. : [Attachment 365063] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied dewei_zhu at apple.com's request for
review:
Bug 195912: Fix a bug from r226303 that latest build time is not correctly
calculated.
https://bugs.webkit.org/show_bug.cgi?id=195912

Attachment 365063: Patch

https://bugs.webkit.org/attachment.cgi?id=365063&action=review




--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 365063
  --> https://bugs.webkit.org/attachment.cgi?id=365063
Patch

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

> Websites/perf.webkit.org/public/v3/models/time-series.js:21
> +	   if (!this._latestBuildPoint || (this._latestBuildPoint.build() && 
this._latestBuildPoint.build().buildTime() < item.build().buildTime()))
> +	       this._latestBuildPoint = item;

This is a layering violation. TimeSeries class shouldn't be aware of build()
and its values, etc...

> Websites/perf.webkit.org/public/v3/pages/test-freshness-page.js:94
> +		       const latestBuildPoint =
currentTimeSeries.latestBuildPoint();

Just find the point based on build time here.
This happens once for each callback from measurementSet.fetchBetween anyway so
the perf isn't an issue.
Doing a bunch of checks in TimeSeries.append would be way slower.