Closed Bug 1376449 Opened 4 years ago Closed 4 years ago

Frequent failures in dom/canvas/test/webgl-mochitest/test_capture.html on linux64-qr


(Core :: Graphics: WebRender, defect, P3)

Other Branch



Tracking Status
firefox56 --- fixed


(Reporter: kats, Assigned: kats)



(Whiteboard: [gfx-noted])


(1 file)

With the landing of APZ on webrender, the dom/canvas/test/webgl-mochitest/test_capture.html test started failing quite frequently on linux64-qr platforms. This spike in intermittents was lumped together with a pre-existing android failure in a different test_capture.html test (the one at dom/canvas/test/test_capture.html) into bug 1206887.

This bug is specifically about tracking down and fixing the "new" failures on the linux64-qr platform. Here is an orangefactor link for the bug tracking the intermittents; type "linux64-qr" into the search filter to see the new failures.
I looked through the failing run output, the last bit of which is reproduced below:

TEST-PASS | dom/canvas/test/webgl-mochitest/test_capture.html | vmanual should still be red
WebGL: drawArrays(green)
Requesting frame from vmanual
Waiting for video vmanual to match [0,255,0,255] - green (should become green after call order test)
WebGL: clearColor(red)
Frame: 0 IsPixel ref=0,255,0,255 threshold=0 value=255,0,0,255
Buffered messages finished
TEST-UNEXPECTED-FAIL | dom/canvas/test/webgl-mochitest/test_capture.html | Test timed out.

I compared this to what the test is supposed to do and to a passing local run. The interesting thing here is the "WebGL: clearColor(red)" call in the middle of the output below - that's not supposed to happen, and that's what causes the green pixel to end up red and the test to time out while it waits for the expected green pixel to appear at [1].

I suspect the reason this clearColor(red) appears is because the the previous hunk of the test registered a requestAnimationFrame callback that does a red clearColor [2]. That previous hunk did clean up after itself by calling "stop" on the RAF looper code [3] but I think there's a bug in this code.

Specifically, the RAF looper code only uses the stop variable to stop *additional* RAF calls, but it still runs the provided callback function [4]. That means if a RAF call is queued up at the time stop() is called, the next frame will still run the callback function. And I suspect that's what happening in this case, allowing the "clearColor(red)" call to leak outside the hunk of the test that it's supposed to be contained in.

I can see two possible fixes: one is to expand the scope of the "if (stop)" block to include the callback function call. The other is to do a cancelAnimationFrame call when stop() is called. The first of these two approaches seems like the more conservative approach, because I'm not sure what other tests use this code and if they implicitly or explicitly assume there's going to be another animation frame coming. Cancelling the animation frame might cause those tests to hang if that's the case.

Marking this as blocking bug 1161913 also since that's the bug that introduced this code.
Blocks: 1161913
Comment on attachment 8881408 [details]
Bug 1376449 - Robustify startDrawing to prevent calling callback function after stop() has been called.

nit: IMO `if (stop) { return; }` is cleaner.
Attachment #8881408 - Flags: review?(apehrson) → review+
Thanks, updated patch to do the early-return.
Pushed by
Robustify startDrawing to prevent calling callback function after stop() has been called. r=pehrsons
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.