Bug 206625 – Try adding glBindFramebuffer flush workaround to ANGLE as remedy for flaky WebGL tests

• Status: RESOLVED
• Resolution: FIXED
• Component: ANGLE

[Kenneth Russell]

An attempt was made recently in bug 205483 / https://trac.webkit.org/changeset/253926 to switch WebKit's WebGL implementation to use ANGLE by default. Unfortunately, a significant number of tests became flaky after that transition, as can be seen from the bugs linked from bug 205483.

Some of these flakes have been triaged and a couple have been addressed. However, a few of the most concerning bugs have not been reproducible on any machines except the bots.

ANGLE virtualizes OpenGL contexts on top of one hardware context; this is probably the essential difference compared to WebKit's current WebGL backend, which uses a separate CGLContextObj per WebGL context.

Chromium on Mac using ANGLE (which is not the current shipping configuration, but is being tested) does not exhibit these flakes. We suspect that some glFlushes which were added to Chromium to address unexplained crashes in the OpenGL driver may be the reason.

There is a known driver bug affecting Intel GPUs on a couple of configurations described in https://bugs.chromium.org/p/chromium/issues/detail?id=1018028 . There is a hypothesis that adding that workaround - amounting to calling glFlush before glBindFramebuffer - might work around the widespread flakiness seen on the bots with the ANGLE switch.

After ANGLE is updated in bug 206318, we plan to add that workaround to WebKit's copy of ANGLE, and then try switching ANGLE on again. If the tests aren't flaky, we can try disabling the workaround and see whether that workaround is responsible. If they are still flaky, we can disable ANGLE again and redouble our efforts to reproduce the flakiness and find a more robust workaround.

[Kenneth Russell]

ANGLE was rolled but was reverted. A new ANGLE roll will have to be done before this is attempted.

[Kenneth Russell]

Created attachment 389929 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[Kenneth Russell]

ANGLE has been rolled forward - should consider adding this driver bug workaround now.

Tested minimally locally, but even without this patch, wasn't able to reproduce the flaky failures that were seen on the WebKit test bots.

[WebKit Commit Bot]

Comment on attachment 389929 [details]
Patch

Rejecting attachment 389929 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 389929, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Traceback (most recent call last):
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 80, in <module>
    main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 75, in main
    WebKitPatch(os.path.abspath(__file__)).main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute
    self._sequence.run_and_handle_errors(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run
    step(tool, options).run(state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 54, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: https://webkit-queues.webkit.org/results/13318059

[Kenneth Russell]

Comment on attachment 389929 [details]
Patch

Could we try the CQ again? I think the r/cq bits got set in the wrong order.

[WebKit Commit Bot]

Comment on attachment 389929 [details]
Patch

Rejecting attachment 389929 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 389929, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Traceback (most recent call last):
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 80, in <module>
    main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch", line 75, in main
    WebKitPatch(os.path.abspath(__file__)).main()
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute
    self._sequence.run_and_handle_errors(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run
    step(tool, options).run(state)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 54, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: https://webkit-queues.webkit.org/results/13318772

[James Darpinian]

Hmm. In the past this happened to me when I accidentally deleted the "Reviewed by NOBODY (OOPS!)." line from my changelog. But your change seems to have that. Is the formatting somehow different than it normally expects?

[Kenneth Russell]

(In reply to James Darpinian from comment #8)
> Hmm. In the past this happened to me when I accidentally deleted the
> "Reviewed by NOBODY (OOPS!)." line from my changelog. But your change seems
> to have that. Is the formatting somehow different than it normally expects?

Based on the emails I got, I think the cq+ bit was accidentally set just before the r+ bit.

[Kenneth Russell]

Created attachment 390135 [details]
Patch

[Kenneth Russell]

Argh - in the old patch I somehow deleted the leading "2" in "2020" - so the date was invalid. Fixed in the revised patch.

[WebKit Commit Bot]

Comment on attachment 390135 [details]
Patch

Clearing flags on attachment: 390135

Committed r256076: <https://trac.webkit.org/changeset/256076>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59277628>