Bug 74232 – Remove partially implemented per-Element visibility checks from requestAnimationFrame logic

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

Remove partially implemented per-Element visibility checks from requestAnimationFrame logic

[James Robinson]

Created attachment 118676 [details]
Patch

[James Robinson]

Here's the thread where some of the details of this element's behavior were debated on public-web-perf:

http://lists.w3.org/Archives/Public/public-web-perf/2011May/0015.html

the part most relevant to this parameter starts around here:
http://lists.w3.org/Archives/Public/public-web-perf/2011May/0030.html

Another issue is that this parameter is very prone to misuse if it were implemented more thoroughly. For example this code:

function animateRight() {
  e.style.left.px += 10; // using imaginary typed CSSOM syntax
  requestAnimationFrame(animateRight, e);
}

will not behave at all the way the author expects if the element starts offscreen - instead of the element smoothly animating into view, it'll never animate at all.

For the record I am not at all opposed to someone implementing a better version of this later down the road. I think if it's going to be done, though, the approach will have to be somewhat different to the one here. I don't think the current code will be much help to such a future endeavor and it is a significant hindrance to other cleanups and bugfixes I have planned in this area.

[James Robinson]

To answer one of Chris' questions in another bug: The functionality that we have actually implemented is that we currently do not fire requestAnimationFrame callbacks when the element parameter was supplied if that element's style.display is "none" or if that element is part of the DOM tree. If either of these conditions are manipulated by requestAnimationFrame callbacks within the same document, then the expected thing happens (the callback fires eventually if the element gets a renderer, it doesn't if the element loses its renderer before the associated callback fires).  If either of the conditions change due to a callback running in a *different* document then the firing behavior is buggy and weird (it depends on the order in which we process documents, which is different for Chromium vs WebKit1/2).

The current behavior also means we re-resolve styles after invoking each callback even though it's typically unnecessary and inefficient.

[Dean Jackson]

James, do you want to commit this?

[James Robinson]

I sure do, I'll need to rebase it to ToT and retest it.  Will try to do later today.

[James Robinson]

Created attachment 136119 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 136119 [details]
Patch for landing

Clearing flags on attachment: 136119

Committed r113573: <http://trac.webkit.org/changeset/113573>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.