Bug 74232 - Remove partially implemented per-Element visibility checks from requestAnimationFrame logic
: Remove partially implemented per-Element visibility checks from requestAnimat...
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 74231
:
  Show dependency treegraph
 
Reported: 2011-12-09 18:04 PST by
Modified: 2012-04-09 04:42 PST (History)


Attachments
Patch (15.24 KB, patch)
2011-12-09 18:18 PST, James Robinson
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (15.12 KB, patch)
2012-04-06 19:13 PST, James Robinson
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-12-09 18:04:52 PST
Remove partially implemented per-Element visibility checks from requestAnimationFrame logic
------- Comment #1 From 2011-12-09 18:18:51 PST -------
Created an attachment (id=118676) [details]
Patch
------- Comment #2 From 2011-12-09 18:24:12 PST -------
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.
------- Comment #3 From 2011-12-09 18:29:53 PST -------
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.
------- Comment #4 From 2012-04-06 13:28:20 PST -------
James, do you want to commit this?
------- Comment #5 From 2012-04-06 13:33:35 PST -------
I sure do, I'll need to rebase it to ToT and retest it.  Will try to do later today.
------- Comment #6 From 2012-04-06 19:13:18 PST -------
Created an attachment (id=136119) [details]
Patch for landing
------- Comment #7 From 2012-04-09 04:42:33 PST -------
(From update of attachment 136119 [details])
Clearing flags on attachment: 136119

Committed r113573: <http://trac.webkit.org/changeset/113573>
------- Comment #8 From 2012-04-09 04:42:39 PST -------
All reviewed patches have been landed.  Closing bug.