[chromium] Do not allow infinite pendign frames in CCFrameRateController
Created attachment 158915 [details] Patch
Created attachment 158922 [details] Patch
Comment on attachment 158922 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158922&action=review Not sure how swap acks are supposed to work for testing. Should we be seeing them during tests? They don't show up currently, so I had to add fake swap acks. The tests seem to rely on being able to submit infinite frames otherwise. > Source/WebKit/chromium/tests/CCThreadedTest.cpp:155 > + // run until after the lifetime of its client. Is there a more automatic way to cancel tasks that outlive the objects it references? > Source/WebKit/chromium/tests/CCThreadedTest.cpp:182 > + CCProxy::implThread()->postDelayedTask(adoptPtr(swapAck), 15); Do we want realistic swap ack delays for testing? Or should we make this delay shorter/zero?
Nat / John - whatcha think?
Comment on attachment 158922 [details] Patch Maybe there should be an assert(maxFramesPending > 0) in CCFrameRateController::setMaxFramesPending?
Created attachment 159507 [details] Patch
Rebased. Added assert(maxFramesPending > 0) back in to CCFrameRateController::setMaxFramesPending. (It used to be part of a previous patch.) Questions from comment #3 still apply.
Comment on attachment 158922 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158922&action=review >> Source/WebKit/chromium/tests/CCThreadedTest.cpp:182 >> + CCProxy::implThread()->postDelayedTask(adoptPtr(swapAck), 15); > > Do we want realistic swap ack delays for testing? Or should we make this delay shorter/zero? How about using a CCTimer and a counter of numPending. Any time it ticks, decrement. If numPending still greater than 0, post another tick. Then you dont need a deque or cancel logic. 15 seems fine.
Comment on attachment 159507 [details] Patch Attachment 159507 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13551033 New failing tests: platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html
Created attachment 159548 [details] Archive of layout-test-results from gce-cr-linux-06 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-06 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 159575 [details] Patch
Comment on attachment 159575 [details] Patch Nice, lgtm
Comment on attachment 159575 [details] Patch R=me
Comment on attachment 159575 [details] Patch Attachment 159575 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13547248 New failing tests: platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html
Created attachment 159585 [details] Archive of layout-test-results from gce-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
How do I run this new failing test locally? platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html
(In reply to comment #16) > How do I run this new failing test locally? > platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html Hmm, it looks like that test timed out. That test is LayoutTests/compositing/visibility/visibility-composited-transforms.html run with threaded compositing turned on. I added them as a smoke test for threaded compositing since the normal layout tests only exercise the single-threaded mode. On Linux, to run this locally, build the DumpRenderTree target and then run 'new-run-webkit-tests --chromium --debug platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html'.
I could not reproduce the visibility-composited-transforms.html timeout/error, either on Windows or Linux, although I do get an unexpected EOF on stdout. How can I re-run cr-linux on the bots? Here's the output I get on my local machine: " $ new-run-webkit-tests --chromium --debug platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html Using port 'chromium-linux-x86_64' Test configuration: <lucid, x86_64, debug> Placing test results in None Baseline search path: chromium-linux -> chromium-win -> chromium -> mac -> generic Using Debug build Pixel tests enabled Regular timeout: 12000, slow test timeout: 60000 Command line: /mnt/code/chrome0/src/out/Debug/DumpRenderTree - Found 1 test; running 1, skipping 0. Running 1 DumpRenderTree over 1 shard. unexpected EOF of stdout The test ran as expected. "
(In reply to comment #18) > I could not reproduce the visibility-composited-transforms.html timeout/error, either on Windows or Linux, although I do get an unexpected EOF on stdout. > > How can I re-run cr-linux on the bots? You could upload another patch and click the ews button. Alternatively, you could also send off a try job to the linux_layout bot.
Created attachment 159792 [details] Patch
Comment on attachment 159792 [details] Patch Attachment 159792 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13569039 New failing tests: platform/chromium/virtual/threaded/compositing/visibility/visibility-image-layers-dynamic.html platform/chromium/virtual/threaded/compositing/visibility/visibility-composited-transforms.html
Created attachment 159818 [details] Archive of layout-test-results from gce-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment on attachment 158922 [details] Patch Cleared review? from obsolete attachment 158922 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Created attachment 161388 [details] Patch
Comment on attachment 161388 [details] Patch Attachment 161388 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13682777 New failing tests: platform/chromium/virtual/threaded/compositing/visibility/visibility-simple-canvas2d-layer.html platform/chromium/virtual/threaded/compositing/webgl/webgl-no-alpha.html platform/chromium/virtual/threaded/compositing/visibility/visibility-image-layers-dynamic.html
Created attachment 161734 [details] Patch
The latest patch should eliminate the flakiness. The basic problem was that the implementations of WGC3D used by tests didn't support swap acks. Instead of faking swap acks in the tests like previous patches, I went ahead and changed CCFrameRateController to not track number of pending frames if swap acks are not supported.
Comment on attachment 161734 [details] Patch R=me
Comment on attachment 161734 [details] Patch This one should be ready to go.
Comment on attachment 161734 [details] Patch Clearing flags on attachment: 161734 Committed r127476: <http://trac.webkit.org/changeset/127476>
All reviewed patches have been landed. Closing bug.