Summary: | FrameView should make more calls to postLayoutTasks() using the timer | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bdakin, ddkilzer, mitz, simon.fraser | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Beth Dakin
2010-08-28 12:47:54 PDT
Created attachment 65838 [details]
Patch
Cool! Have you tested that the sidebar in Safari RSS doesn’t jiggle when you scroll? That’s a good test for making sure that at least sometimes you do perform post-layout tasks synchronously. I just tested, and it appears to be jiggle-free :-) The name m_shouldFirePostLayoutTimer should be improved. It is really about whether you have a pending need to perform post-layout tasks. The timer already knows if it should fire :-) you’re really using this for the case(s) where you stopped the timer before it had a chance to fire. (In reply to comment #4) > The name m_shouldFirePostLayoutTimer should be improved. It is really about whether you have a pending need to perform post-layout tasks. The timer already knows if it should fire :-) you’re really using this for the case(s) where you stopped the timer before it had a chance to fire. Yes, I agree. I wasn't happy with the name. Some ideas: m_needToPerformPostLayoutTasks m_hasPostLayoutTasksPending Comment on attachment 65838 [details]
Patch
There's a lot of statefulness here that makes it very hard to follow. Can we pass state via parameters, rather than keeping it in member variables? Alternatively, maybe a single state enum would be easier to follow than two booleans.
(In reply to comment #6) > (From update of attachment 65838 [details]) > There's a lot of statefulness here that makes it very hard to follow. Can we pass state via parameters, rather than keeping it in member variables? Alternatively, maybe a single state enum would be easier to follow than two booleans. I'm not sure either of those would work. A single state enum seems weird to me since I see the two variables as being very different. m_inSynchronousPostLayout represents whether the current callstack would trace back to a synchronous call to performPostLayoutTakss(). m_shouldFirePostLayoutTimer represents whether we have a pending need to call performPostLayoutTaks() on the timer. I think a single variable representing these two concepts would be confusing. In terms of passing parameters…it seems like that would be a mess for m_inSynchronousPostLayout. I think it would mean passing a parameter to performPostLayoutTasks(), and having performPostLayoutTasks() send that same parameter into m_frame->eventHandler()->sendResizeEvent(), and keep carrying it as a parameter until it got back to FrameView::layout(). It would be similarly sloppy for m_shouldFirePostLayoutTimer. Again, a member variable seems a lot cleaner to me. Comment on attachment 65838 [details]
Patch
r=me
Thanks Dave! http://trac.webkit.org/changeset/66552 Oops! Forgot the name-change. http://trac.webkit.org/changeset/66555 |