Bug 206625

Summary: Try adding glBindFramebuffer flush workaround to ANGLE as remedy for flaky WebGL tests
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: ANGLEAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, graouts, jdarpinian, justin_fan, kondapallykalyan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 206318    
Bug Blocks: 205483, 214104    
Attachments:
Description Flags
Patch
none
Patch none

Description Kenneth Russell 2020-01-22 16:02:48 PST 
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.
Comment 1 Kenneth Russell 2020-01-23 18:02:16 PST 
ANGLE was rolled but was reverted. A new ANGLE roll will have to be done before this is attempted.
Comment 2 Kenneth Russell 2020-02-05 18:30:34 PST 
Created attachment 389929 [details]
Patch
Comment 3 EWS Watchlist 2020-02-05 18:31:40 PST 
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 4 Kenneth Russell 2020-02-05 18:32:05 PST 
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.
Comment 5 WebKit Commit Bot 2020-02-06 14:59:47 PST 
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
Comment 6 Kenneth Russell 2020-02-06 15:36:32 PST 
Comment on attachment 389929 [details]
Patch

Could we try the CQ again? I think the r/cq bits got set in the wrong order.
Comment 7 WebKit Commit Bot 2020-02-07 14:31:27 PST 
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
Comment 8 James Darpinian 2020-02-07 15:01:17 PST 
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?
Comment 9 Kenneth Russell 2020-02-07 15:03:23 PST 
(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.
Comment 10 Kenneth Russell 2020-02-07 15:23:06 PST 
Created attachment 390135 [details]
Patch
Comment 11 Kenneth Russell 2020-02-07 15:23:55 PST 
Argh - in the old patch I somehow deleted the leading "2" in "2020" - so the date was invalid. Fixed in the revised patch.
Comment 12 WebKit Commit Bot 2020-02-07 16:03:32 PST 
Comment on attachment 390135 [details]
Patch

Clearing flags on attachment: 390135

Committed r256076: <https://trac.webkit.org/changeset/256076>
Comment 13 WebKit Commit Bot 2020-02-07 16:03:34 PST 
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2020-02-07 16:07:51 PST 
<rdar://problem/59277628>