Bug 74528 - [Qt] Support requestAnimationFrame API
Summary: [Qt] Support requestAnimationFrame API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-14 12:03 PST by Igor Trindade Oliveira
Modified: 2011-12-19 03:50 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.86 KB, patch)
2011-12-14 12:13 PST, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch v2 (6.34 KB, patch)
2011-12-15 06:49 PST, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Trindade Oliveira 2011-12-14 12:03:23 PST
SSIA
Comment 1 Igor Trindade Oliveira 2011-12-14 12:13:14 PST
Created attachment 119274 [details]
Patch

Proposed patch.
Comment 2 Kenneth Rohde Christiansen 2011-12-15 03:54:37 PST
Comment on attachment 119274 [details]
Patch

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

probably fine but let Tor Arne or Simon have a look

> Source/JavaScriptCore/wtf/Platform.h:1131
> -#if PLATFORM(MAC) || PLATFORM(GTK) || PLATFORM(EFL) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO))
> +#if PLATFORM(MAC) || PLATFORM(GTK) || PLATFORM(EFL) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO)) || PLATFORM(QT)
>  #define WTF_USE_REQUEST_ANIMATION_FRAME_TIMER 1

I wonder if this could be an opt out instead :)
Comment 3 Simon Hausmann 2011-12-15 03:58:23 PST
Comment on attachment 119274 [details]
Patch

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

> Source/WebCore/Target.pri:3325
> +contains(DEFINES, ENABLE_REQUEST_ANIMATION_FRAME=1) {
> +    !v8 {
> +        SOURCES += \
> +            bindings/js/JSRequestAnimationFrameCallbackCustom.cpp
> +    }
> +
> +    SOURCES += \
> +        dom/ScriptedAnimationController.cpp
> +
> +    HEADERS += \
> +        dom/ScriptedAnimationController.h
> +}

I believe that you can add these files to the build unconditionally. They follow the common WebKit pattern that all files are buildable and contain corresponding preprocessor guards. The files in question here appear to contain them, so they will compile with and without the feature enabled.

So AFAICS we don't _need_ to make Target.pri more complex :)
Comment 4 Igor Trindade Oliveira 2011-12-15 06:49:40 PST
Created attachment 119421 [details]
Patch v2

proposed patch.
Comment 5 WebKit Review Bot 2011-12-15 08:28:12 PST
Comment on attachment 119421 [details]
Patch v2

Clearing flags on attachment: 119421

Committed r102939: <http://trac.webkit.org/changeset/102939>
Comment 6 WebKit Review Bot 2011-12-15 08:28:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Csaba Osztrogonác 2011-12-15 09:18:45 PST
Reopen, because tests still fail:

+CONSOLE MESSAGE: line 7: TypeError: 'undefined' is not a function (evaluating 'window.webkitRequestAnimationFrame(function()

I think you should have run layout tests before unsipping them ...
Comment 8 Igor Trindade Oliveira 2011-12-15 09:23:50 PST
It was working, but now after rebase webkit, it is failing.
I am investigating the problem.

(In reply to comment #7)
> Reopen, because tests still fail:
> 
> +CONSOLE MESSAGE: line 7: TypeError: 'undefined' is not a function (evaluating 'window.webkitRequestAnimationFrame(function()
> 
> I think you should have run layout tests before unsipping them ...
Comment 9 Igor Trindade Oliveira 2011-12-15 09:43:31 PST
The bot needs to clean up the QtWebKit build. It needs to regenerate JSDOMWindow.* files.

(In reply to comment #7)
> Reopen, because tests still fail:
> 
> +CONSOLE MESSAGE: line 7: TypeError: 'undefined' is not a function (evaluating 'window.webkitRequestAnimationFrame(function()
> 
> I think you should have run layout tests before unsipping them ...
Comment 10 Tor Arne Vestbø 2011-12-15 09:51:20 PST
(In reply to comment #9)
> The bot needs to clean up the QtWebKit build. It needs to regenerate JSDOMWindow.* files.
> 
> (In reply to comment #7)
> > Reopen, because tests still fail:
> > 
> > +CONSOLE MESSAGE: line 7: TypeError: 'undefined' is not a function (evaluating 'window.webkitRequestAnimationFrame(function()
> > 
> > I think you should have run layout tests before unsipping them ...

This is a limitation of the clean-check in build-webkit. It only detects changed or removed, not added defines. I'll have a look.
Comment 11 Csaba Osztrogonác 2011-12-15 23:02:08 PST
(In reply to comment #10)
> (In reply to comment #9)
> > The bot needs to clean up the QtWebKit build. It needs to regenerate JSDOMWindow.* files.
> > 
> > (In reply to comment #7)
> > > Reopen, because tests still fail:
> > > 
> > > +CONSOLE MESSAGE: line 7: TypeError: 'undefined' is not a function (evaluating 'window.webkitRequestAnimationFrame(function()
> > > 
> > > I think you should have run layout tests before unsipping them ...
> 
> This is a limitation of the clean-check in build-webkit. It only detects changed or removed, not added defines. I'll have a look.

Ah, one more build system related bug. Good to know. Sorry for the invalid suspection. :) Hmmm ... maybe it caused problems near the webkit_testfonts patch. (It added HAVE_FONTCONFIG)
Comment 12 Csaba Osztrogonác 2011-12-15 23:06:01 PST
I file a new bug report: https://bugs.webkit.org/show_bug.cgi?id=74689
Comment 13 Simon Hausmann 2011-12-19 03:12:59 PST
With this feature landed fast/animation/request-animation-frame-during-modal.html fails permanently, due to the lack of modal dialog support.

See also http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2
Comment 14 Simon Hausmann 2011-12-19 03:50:32 PST
(In reply to comment #13)
> With this feature landed fast/animation/request-animation-frame-during-modal.html fails permanently, due to the lack of modal dialog support.
> 
> See also http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2

I've created bug #74852 to fix this. Working on it atm