[Webkit-unassigned] [Bug 170198] MotionMark tests using Date.now() to drive animations don't work under web-page-replay

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=170198

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

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

Useful inline comments would explain why you're making each code change, and how it impacts the rest of the codebase.

> PerformanceTests/MotionMark/tests/3d/resources/webgl.js:143
> -                this._startTime = Stage.dateCounterValue(1000);
> -            var elapsedTime = Stage.dateCounterValue(1000) - this._startTime;
> +                this._startTime = this.timestampCounterValue(1000);
> +            var elapsedTime = this.timestampCounterValue(1000) - this._startTime;

Why are we replacing Stage.dateCounterValue with this.timestampCounterValue?
What's the significance of it? e.g. does Stage.dateCounterValue use Date.now() and this.timestampCounterValue doesn't?
None of that is clear from your main change log description.

> PerformanceTests/MotionMark/tests/master/resources/canvas-tests.js:19
> +        this._stage = stage;

A useful inline comment here would be explaining why we need to store "stage" as an instance variable.

> PerformanceTests/MotionMark/tests/resources/main.js:654
> +    set timestamp(stamp)
> +    {
> +        this._currentTimestamp = stamp;
> +    },
> +
> +    get timestamp()
> +    {
> +        return this._currentTimestamp;
> +    },
> +

What are these getters and setters used for? Who is getting the value?

> PerformanceTests/MotionMark/tests/resources/main.js:676
> +    rotatingColor: function(cycleLengthMs, saturation, lightness)
> +    {

Why are you moving these methods here?
That should be explained in the change log.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170330/928f25c6/attachment.html>