Bug 90018 – [Gtk] too long function in animation script cause repaint to never happen.

• Component: WebKitGTK
• Status: UNCONFIRMED

[arno.]

Hi,
currently, timers are implemented as a heap, whichever is scheduled first is executed first.
Additionally, when a paint is in the heap, but a new paint is scheduled, the paint timer is just rescheduled in the heap.
Also, on some dom operations (may be all of them, but I'm not sure), a layout timer is scheduled, but also a paint timer.
Last, when layout timer is fired, it schedule a paint.

So, if you have a timer (defined with webkitRequestAnimationFrame) which takes a long time to execute, modify the dom, and defines a new timer, the repaint never happens.

After adding an animation with window.webkitRequestAnimationFrame, the timer heap is:

|animation|

During animation, a layout then a repaint are scheduled. And also a new animation. If animation has been running long enough (more than MinimumAnimationInterval), animation is schedule as soon as possible (right after layout and paint). Timer heap is:

|layout paint animation|

When layout timer is fired, it also schedule a paint as soon as possible. But this is after animation though. So, the paint timer is moved in the heap. heap is then:

|animation paint|

Then, animation is executed, layout, paint and animations are scheduled again. heap is:

|layout paint animation|

and the loop starts over. Paint timer is never fired.

If loop executes fast enough, the next animation is scheduled long enough in the future, and the paint scheduled by the layout happens before this animation.

[arno.]

Created attachment 149612 [details]
testcase

[arno.]

Created attachment 149613 [details]
testcase

[arno.]

Created attachment 149634 [details]
patch proposal; use glib loop instead WebKit shared timer for paint timer

[arno.]

add kov as he recently reviewed patches in ChromeClientGtk.cpp

[arno.]

Created attachment 166531 [details]
Patch

updated patch to build on recent trunk

[Gustavo Noronha (kov)]

Comment on attachment 166531 [details]
Patch

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

You have empty entries on top of the actual entries in the changelogs. I'm wondering, why do you remove the source and add a new one when the timer already exists? Doesn't just waiting for the already scheduled run to happen work?

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:571
> +

Empty new line.

[arno.]

Created attachment 169964 [details]
Patch

updated patch to address comments

[arno.]

Created attachment 170243 [details]
actually, it looks like we need remove the sources. For some reason, the video did not work with previous patch.

updated patch to address comments

[arno.]

Created attachment 170247 [details]
sorry, wrong patch. This is the correct one

updated patch

[Andreas Kling]

Comment on attachment 170247 [details]
sorry, wrong patch. This is the correct one

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.