[Webkit-unassigned] [Bug 66280] [GTK] requestAnimationFrame support for gtk port

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-09-08 06:26:40 PST ---
(From update of attachment 106114)
View in context: https://bugs.webkit.org/attachment.cgi?id=106114&action=review

Instead of duplicating the code between webkit1 and webkit2 it would be better to move common code to a new class in WebCore and share it between webkit1 and webkit2

> configure.ac:935
> +# check whether to enable requestAnimationFrame support
> +AC_MSG_CHECKING([whether to enable requestAnimationFrame support])
> +AC_ARG_ENABLE(request_animation_frame,
> +              AC_HELP_STRING([--enable-request-animation-frame],
> +                             [enable support for requestAnimationFrame [default=no]]),
> +              [],[enable_request_animation_frame="no"])
> +AC_MSG_RESULT([$enable_request_animation_frame])
> +

Why not enable it by default?

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:673
> +#if ENABLE(REQUEST_ANIMATION_FRAME)
> +void ChromeClient::scheduleAnimation()
> +{
> +    g_signal_emit_by_name(m_webView, "schedule-animation");
> +}

Maybe we could implement the animation stuff here, so that we don't need to add a new signal to web view.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:210
> +#if ENABLE(REQUEST_ANIMATION_FRAME)
> +    SCHEDULE_ANIMATION,
> +#endif

I would avoid adding a new signal for this.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:1365
> +
> +        g_main_context_wakeup(gMainContext);

Why do you need to wakeup the main context here?

> Source/WebKit/gtk/webkit/webkitwebview.cpp:1369
> +        priv->requestAnimationFrameTimerSource = g_timeout_source_new(0);
> +        ASSERT(priv->requestAnimationFrameTimerSource);
> +

I don't understand this. The first time you create a timeout surce with 0, to be dispatched inmediately (I think you could use a g_idle_source() instead), but in the callback you check whether if elapsed time is 16ms, why not schedule a 16ms timeout here instead? I guess the first time timerFiredCallback is called it will always schedule a new timeout source.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.