RESOLVED INVALID 97052
[BlackBerry] YouTube playlist/watch later bar unexpectedly appears
https://bugs.webkit.org/show_bug.cgi?id=97052
Summary [BlackBerry] YouTube playlist/watch later bar unexpectedly appears
Arvid Nilsson
Reported 2012-09-18 16:31:28 PDT
The playlist bar is usually located just below the viewport. Since it has become fashionable lately to not composite stuff that's currently not in the (WebKit thread's idea of the) viewport, RenderLayerCompositor doesn't accelerate this layer.
Attachments
Patch (3.44 KB, patch)
2012-09-18 16:58 PDT, Arvid Nilsson
simon.fraser: review-
Arvid Nilsson
Comment 1 2012-09-18 16:38:07 PDT
PR 190061
Arvid Nilsson
Comment 2 2012-09-18 16:41:54 PDT
Let's also take this opportunity to upstream the same fix, for overlap map.
Arvid Nilsson
Comment 3 2012-09-18 16:58:24 PDT
Simon Fraser (smfr)
Comment 4 2012-09-18 17:37:18 PDT
Comment on attachment 164636 [details] Patch These kinds of mysterious platform #ifdefs only serve to make the code confusing. Why not fix the real bug ("WebKit thread scroll position lags behind the compositing thread scroll position")?
Arvid Nilsson
Comment 5 2012-09-18 17:41:11 PDT
(In reply to comment #4) > (From update of attachment 164636 [details]) > These kinds of mysterious platform #ifdefs only serve to make the code confusing. Why not fix the real bug ("WebKit thread scroll position lags behind the compositing thread scroll position")? Ah - it's not a bug, it's a design decision. However, the ScrollingCoordinator might have other design decisions that solve the same problem without the WebKit thread lagging behind? The problem to be solved being "provide instant feedback to the user". In the BlackBerry port, the compositing thread processes user input first, updates its scroll position, scrolls and repositions fixed elements, and sends an async message to the WebKit thread with the new scroll position, so it can catch up later.
Arvid Nilsson
Comment 6 2012-09-19 04:55:14 PDT
I've been trying to figure out how THREADED_SCROLLING works, and I'm curious why you (Mac port) don't run into a similar bug where the FrameView's viewport becomes out of sync with the actual viewport displayed on screen. Perhaps the BlackBerry port's best chance of upstreaming stuff like this is to buy into the ScrollingCoordinator mechanism. At the very least, I have to get a grasp of how THREADED_SCROLLING works to evaluate if it could work with our architecture.
Arvid Nilsson
Comment 7 2012-09-19 07:04:51 PDT
Antonio showed me http://tale.wkb.ug/2012/03/04/scrolling-meets-compositor/, which says: "This new machinery introduced an asynchronous scrolling model to make this work. In this model, an updated scroll-position, which is requested by some other WebCore object, isn’t applied immediately. Instead, asking a new position will trigger a scrolling animation, and the position applied after the animation is finished." If this description is correct, the FrameView's scroll position will lag behind what's shown on screen, just like in the BlackBerry port. However, a difference compared to our port, is that the scroll request seems to be initiated from the WebKit main thread, so user input must bounce via the WebKit main thread before it can result in visible scrolling. This can be problematic because it will introduce delays if the WebKit main thread is busy processing JavaScript or some other long-running calculation before it can get around to processing the user input event. It seems from an initial analysis, that the ScrollingCoordinator might be unsuitable for mobile.
Simon Fraser (smfr)
Comment 8 2012-09-19 09:25:11 PDT
On Mac we don't move fixed layers around when scrolling yet, but we will, at which point we will have to revisit this code. So a Setting or #ifdef may be appropriate, but it should related to threaded scrolling, not a particular platform.
Arvid Nilsson
Comment 9 2012-09-19 09:44:47 PDT
(In reply to comment #8) > On Mac we don't move fixed layers around when scrolling yet, but we will, at which point we will have to revisit this code. So a Setting or #ifdef may be appropriate, but it should related to threaded scrolling, not a particular platform. I agree, the #ifdef is an eyesore - sorry about that. I think we can keep our code forked (not upstreamed) for the time being. Annd eventually figure out if our flavor of threaded scrolling can be integrated in a pretty way with the THREADED_SCROLLING code.
Note You need to log in before you can comment on or make changes to this bug.