[webkit-reviews] review granted: [Bug 209491] [ Mac ] fast/animation/request-animation-frame-cancel2.html is flaky failing. : [Attachment 398491] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 5 02:53:36 PDT 2020


Antoine Quint <graouts at webkit.org> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 209491: [ Mac ] fast/animation/request-animation-frame-cancel2.html is
flaky failing.
https://bugs.webkit.org/show_bug.cgi?id=209491

Attachment 398491: Patch

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




--- Comment #5 from Antoine Quint <graouts at webkit.org> ---
Comment on attachment 398491
  --> https://bugs.webkit.org/attachment.cgi?id=398491
Patch

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

> LayoutTests/fast/animation/request-animation-frame-cancel2.html:10
> +    var e = document.getElementById("e");

Technically you're redefining `e` here because `window.e` was already defined
to be that element. It's kinda dirty, but that's the Web :)

I'm not sure what is the use of this variable though, you seem to use it only
as the second parameter to requestAnimationFrame() calls and that method only
takes a single parameter.

> LayoutTests/fast/animation/request-animation-frame-cancel2.html:16
> +	       window.requestAnimationFrame(function(timestamp) {

You're mixing arrow-syntax and `function`, could you use a consistent style?

> LayoutTests/fast/animation/request-animation-frame-cancel2.html:20
> +	       }, e);

What is this second parameter to requestAnimationFrame?

> LayoutTests/fast/animation/request-animation-frame-cancel2.html:29
> +	       }, e);

What is this second parameter to requestAnimationFrame?

> LayoutTests/fast/animation/request-animation-frame-cancel2.html:32
> +	       setTimeout(function() {
> +		   resolve();
> +	       }, 100);

setTimeout(resolve, 100);


More information about the webkit-reviews mailing list