Bug 67112 – [EFL] Support requestAnimationFrame API

• Component: WebKit EFL
• Status: RESOLVED FIXED

[ChangSeok Oh]

This bug is for implementing requestAnimationFrame JS interface for EFL port.

[ChangSeok Oh]

Created attachment 110977 [details]
patch

[Gyuyoung Kim]

Comment on attachment 110977 [details]
patch

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

> Source/WebCore/CMakeLists.txt:2182
> +IF (ENABLE_REQUEST_ANIMATION_FRAME)

Where is ENABLE_REQUEST_ANIMATION_FRAME defined?  Is IF (WTF_USE_REQUEST_ANIMATION_FRAME) correct?

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 110977 [details]
patch

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

I've noticed you use different email addresses in the ChangeLog and in your Bugzilla account. Is that intentional? Do you know of any test (either a page on the internet or some page in LayoutTests) where I can see this feature working?

Still r? while waiting for the answers.

> ChangeLog:8
> +        Add build-option for reqeustAnimationFrame feature.

Typo in reqeustAnimationFrame.

>> Source/WebCore/CMakeLists.txt:2182
>> +IF (ENABLE_REQUEST_ANIMATION_FRAME)
> 
> Where is ENABLE_REQUEST_ANIMATION_FRAME defined?  Is IF (WTF_USE_REQUEST_ANIMATION_FRAME) correct?

Defined in Source/cmake/OptionsEfl.cmake below?

> Source/cmake/OptionsEfl.cmake:87
> +WEBKIT_FEATURE(ENABLE_REQUEST_ANIMATION_FRAME "Enable requestAnimationFrame API" DEFAULT OFF)

Any reason for it to be OFF by default?

[Gyuyoung Kim]

(In reply to comment #3)
> (From update of attachment 110977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110977&action=review
> 
> >> Source/WebCore/CMakeLists.txt:2182
> >> +IF (ENABLE_REQUEST_ANIMATION_FRAME)
> > 
> > Where is ENABLE_REQUEST_ANIMATION_FRAME defined?  Is IF (WTF_USE_REQUEST_ANIMATION_FRAME) correct?
> 
> Defined in Source/cmake/OptionsEfl.cmake below?

Yes, right. My mistake. Sorry.

[ChangSeok Oh]

Comment on attachment 110977 [details]
patch

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

>> ChangeLog:8

> 
> Typo in reqeustAnimationFrame.

Oops. Done

>> Source/cmake/OptionsEfl.cmake:87
>> +WEBKIT_FEATURE(ENABLE_REQUEST_ANIMATION_FRAME "Enable requestAnimationFrame API" DEFAULT OFF)
> 
> Any reason for it to be OFF by default?

This feature is not fixed yet. Formal spec is under working so that it may be possible to change later. (http://www.w3.org/TR/animation-timing/)
And most important reason is that I found critical issue related with this.
I'm trying to fix it in here, bug70170.

[ChangSeok Oh]

(In reply to comment #3)
> (From update of attachment 110977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110977&action=review
> 
> I've noticed you use different email addresses in the ChangeLog and in your Bugzilla account. Is that intentional? Do you know of any test (either a page on the internet or some page in LayoutTests) where I can see this feature working?
Yes. that's intentional. [e-mail] is my private account so that I can find somebody's question and request quickly, and also I don't want to receive too much mail from bugzilla. :p

Well. Test cases for this feature already exist in current repository. I'll update them in ChangLog.

[ChangSeok Oh]

Created attachment 111128 [details]
Updated patch

[Gyuyoung Kim]

(In reply to comment #5)
> (From update of attachment 110977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110977&action=review

> This feature is not fixed yet. Formal spec is under working so that it may be possible to change later. (http://www.w3.org/TR/animation-timing/)
> And most important reason is that I found critical issue related with this.
> I'm trying to fix it in here, bug70170.

I think it is better to add this feature enabled after closing Bug 70170.

[ChangSeok Oh]

Created attachment 113288 [details]
Updated patch

[Ryuan Choi]

Comment on attachment 113288 [details]
Updated patch

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

> Source/WebCore/CMakeLists.txt:2187
> +        bindings/js/JSRequestAnimationFrameCallbackCustom.cpp

IMO, JSRequestAnimationFrameCallbackCustom.cpp should be moved to UseJSC.cmake

[ChangSeok Oh]

Created attachment 113294 [details]
Updated patch

[ChangSeok Oh]

(In reply to comment #10)
> (From update of attachment 113288 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113288&action=review
> 
> > Source/WebCore/CMakeLists.txt:2187
> > +        bindings/js/JSRequestAnimationFrameCallbackCustom.cpp
> 
> IMO, JSRequestAnimationFrameCallbackCustom.cpp should be moved to UseJSC.cmake
Done.

[Ryuan Choi]

LGTM.

[Gyuyoung Kim]

LGTM too.

[WebKit Review Bot]

Comment on attachment 113294 [details]
Updated patch

Clearing flags on attachment: 113294

Committed r99442: <http://trac.webkit.org/changeset/99442>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.