Bug 54810

Summary: Fix isLayoutTimerActive for ports that set a minimumLayoutDelay
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: New BugsAssignee: Tony Gentilcore <tonyg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch for landing none

Tony Gentilcore
Reported 2011-02-19 11:26:10 PST
Fix isLayoutTimerActive for ports that set a minimumLayoutDelay
Attachments
Patch (4.13 KB, patch)
2011-02-19 11:31 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Patch for landing (3.56 KB, patch)
2011-02-24 15:51 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tony Gentilcore
Comment 1 2011-02-19 11:31:10 PST
Created attachment 83081 [details] Patch
Eric Seidel (no email)
Comment 2 2011-02-19 12:46:53 PST
Comment on attachment 83081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83081&action=review > Source/WebCore/ChangeLog:8 > + No new tests because no new functionality. So we have no way to test that we're yielding between tokens? Do we have any way to detect first paint from JS? Certainly there is in FF, but I'm not sure there is in WebKit. > Source/WebCore/dom/Document.cpp:2160 > + return view() && view()->layoutPending() && minimumLayoutDelay() == m_extraLayoutDelay; Maybe the minimum = extra check needs itself to be a function. Given how strange that reads.
Adam Barth
Comment 3 2011-02-21 12:34:24 PST
Comment on attachment 83081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83081&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests because no new functionality. > > So we have no way to test that we're yielding between tokens? Do we have any way to detect first paint from JS? Certainly there is in FF, but I'm not sure there is in WebKit. It's very difficult to test. It might be observable, but I can't think of how to observe it without races. >> Source/WebCore/dom/Document.cpp:2160 >> + return view() && view()->layoutPending() && minimumLayoutDelay() == m_extraLayoutDelay; > > Maybe the minimum = extra check needs itself to be a function. Given how strange that reads. Yeah, if I were reading this code in the future, it would be pretty mysterious.
Eric Seidel (no email)
Comment 4 2011-02-24 15:50:03 PST
Comment on attachment 83081 [details] Patch cq- for functionization.
Tony Gentilcore
Comment 5 2011-02-24 15:51:26 PST
Created attachment 83737 [details] Patch for landing
Tony Gentilcore
Comment 6 2011-02-24 15:52:09 PST
I went with a bool "isPendingLayoutImmediate" to make this more clear. We can pull it out to a function if anything else needs it. But I'm not sure proliferation of this condition should be encouraged.
WebKit Commit Bot
Comment 7 2011-02-26 03:20:27 PST
Comment on attachment 83737 [details] Patch for landing Clearing flags on attachment: 83737 Committed r79775: <http://trac.webkit.org/changeset/79775>
WebKit Commit Bot
Comment 8 2011-02-26 03:20:33 PST
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 9 2011-02-26 04:07:13 PST
The commit-queue encountered the following flaky tests while processing attachment 83737 [details]: http/tests/websocket/tests/multiple-connections.html bug 53825 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.
Note You need to log in before you can comment on or make changes to this bug.