Bug 217542 - REGRESSION(r268161?): [ macOS ] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https.html is a flaky failure
Summary: REGRESSION(r268161?): [ macOS ] imported/w3c/web-platform-tests/webaudio/the-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-09 16:59 PDT by Hector Lopez
Modified: 2021-06-09 00:33 PDT (History)
13 users (show)

See Also:


Attachments
Patch (10.25 KB, patch)
2020-11-03 10:01 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.22 KB, patch)
2020-11-04 07:57 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hector Lopez 2020-10-09 16:59:11 PDT
imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https.html

Test is a flaky failure according to history on macOS. First occurrence of failure is at r268163 but I believe the change that restarted the testing for this test at r268161 might be a better regression point.

History:

https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-audiocontext-interface%2Fprocessing-after-resume.https.html&platform=mac

Diff:

--- /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https-expected.txt
+++ /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https-actual.txt
@@ -1,3 +1,3 @@
 
-PASS Test consistency of processing after resume()
+FAIL Test consistency of processing after resume() assert_equals: node1 processed frame count expected 128 but got 0
Comment 1 Radar WebKit Bug Importer 2020-10-09 16:59:35 PDT
<rdar://problem/70159546>
Comment 2 Hector Lopez 2020-10-09 17:11:11 PDT
Test expectation while investigated:

https://trac.webkit.org/changeset/268301/webkit
Comment 3 Chris Dumez 2020-11-02 16:52:20 PST
Looks like I was able to reproduce like so:
 Tools/Scripts/run-webkit-tests imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https.html  --repeat-each=100 -g --additional-env-var=JSC_collectContinuously=true --force -1

Diff:
@@ -1,3 +1,3 @@

-PASS Test consistency of processing after resume()
+FAIL Test consistency of processing after resume() assert_equals: construct time before resume expected 0.19156462585034015 but got 0.20317460317460317
Comment 4 Chris Dumez 2020-11-03 10:01:46 PST
Created attachment 413068 [details]
Patch
Comment 5 youenn fablet 2020-11-04 03:04:26 PST
Comment on attachment 413068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413068&action=review

> Source/WebCore/ChangeLog:13
> +        still increase a bit, a short while after the suspend promise has been resolved. To address the issue, we now to a round

s/to/do/

> Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:132
> +        dispatchToRenderThread(WTFMove(callCompletionHandlerOnMainThread));

dispatchToRenderThread takes a function not a completion handler.
Should we make it take a CompletionHandler? It seems it should always call the function in normal cases.
Comment 6 Chris Dumez 2020-11-04 07:56:41 PST
Comment on attachment 413068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413068&action=review

>> Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:132
>> +        dispatchToRenderThread(WTFMove(callCompletionHandlerOnMainThread));
> 
> dispatchToRenderThread takes a function not a completion handler.
> Should we make it take a CompletionHandler? It seems it should always call the function in normal cases.

I do not believe this is currently possible because there is an assertion in CompletionHandler enforcing that the CompletionHandler gets called on the thread it was constructed on.
Comment 7 Chris Dumez 2020-11-04 07:57:36 PST
Created attachment 413164 [details]
Patch
Comment 8 EWS 2020-11-04 13:39:32 PST
Committed r269383: <https://trac.webkit.org/changeset/269383>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413164 [details].
Comment 9 Diego Pino 2021-06-09 00:21:25 PDT
The only test filed under this bug was:

  - imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https.html

GTK post-commit test bot reports the test has been failing consistently for the last 4000 revisions.

Diff:

--- /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https-expected.txt
+++ /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/processing-after-resume.https-actual.txt
@@ -1,3 +1,3 @@
 
-PASS Test consistency of processing after resume()
+FAIL Test consistency of processing after resume() assert_equals: construct time before resume expected 0.06965986394557823 but got 0.07546485260770976