Bug 217543 - [ macOS ] webaudio/OfflineAudioContext/onstatechange.html is a flaky failure
Summary: [ macOS ] webaudio/OfflineAudioContext/onstatechange.html is a flaky failure
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (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 17:16 PDT by Hector Lopez
Modified: 2020-11-02 13:21 PST (History)
13 users (show)

See Also:


Attachments
Patch (5.87 KB, patch)
2020-11-02 12:23 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.83 KB, patch)
2020-11-02 12:37 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 17:16:47 PDT
webaudio/OfflineAudioContext/onstatechange.html


Test is a flaky failure according to history on macOS. First occurrence coincides with import at r267307

History:

https://results.webkit.org/?suite=layout-tests&test=webaudio%2FOfflineAudioContext%2Fonstatechange.html&platform=mac&limit=50000

Diff:

--- /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/webaudio/OfflineAudioContext/onstatechange-expected.txt
+++ /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/webaudio/OfflineAudioContext/onstatechange-actual.txt
@@ -3,11 +3,11 @@
 PASS Executing "test"
 PASS Audit report
 PASS > [test] Signaling of statechange event
-PASS   context.state is equal to running.
+FAIL X context.state is not equal to running. Got closed. assert_true: expected true got false
+PASS   Context rendering resolved correctly.
 PASS   context.state is equal to closed.
-PASS   Context rendering resolved correctly.
 PASS   stateChangeCount is equal to 2.
 PASS   After rendering context.state is equal to closed.
-PASS < [test] All assertions passed. (total 5 assertions)
-PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully.
+FAIL < [test] 1 out of 5 assertions were failed. assert_true: expected true got false
+FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 1 tasks were failed. assert_true: expected true got false
Comment 1 Radar WebKit Bug Importer 2020-10-09 17:17:06 PDT
<rdar://problem/70160122>
Comment 2 Hector Lopez 2020-10-09 17:21:41 PDT
Test expectation while investigated:

https://trac.webkit.org/changeset/268303/webkit
Comment 3 Chris Dumez 2020-11-02 12:23:27 PST
Created attachment 412960 [details]
Patch
Comment 4 Darin Adler 2020-11-02 12:34:05 PST
Comment on attachment 412960 [details]
Patch

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

> Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:105
> +        callOnMainThread([this, result, currentSampleFrame = m_currentSampleFrame, protectedThis = WTFMove(protectedThis)]() mutable {
> +            context().postTask([this, protectedThis = WTFMove(protectedThis), result, currentSampleFrame]() mutable {

Doesn’t seem like we need callOnMainThread if calling postTask. It already does callOnMainThread.
Comment 5 Darin Adler 2020-11-02 12:34:35 PST
Oops, didn’t mean to change this to darin/review?

Don’t know why bugs.webkit.org does that.
Comment 6 Chris Dumez 2020-11-02 12:35:06 PST
(In reply to Darin Adler from comment #4)
> Comment on attachment 412960 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=412960&action=review
> 
> > Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:105
> > +        callOnMainThread([this, result, currentSampleFrame = m_currentSampleFrame, protectedThis = WTFMove(protectedThis)]() mutable {
> > +            context().postTask([this, protectedThis = WTFMove(protectedThis), result, currentSampleFrame]() mutable {
> 
> Doesn’t seem like we need callOnMainThread if calling postTask. It already
> does callOnMainThread.

I think you are looking at the wrong postTask?

void BaseAudioContext::postTask(WTF::Function<void()>&& task)
{
    ASSERT(isMainThread());
    if (m_isStopScheduled)
        return;

    queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, WTFMove(task));
}
Comment 7 Darin Adler 2020-11-02 12:35:27 PST
OK.
Comment 8 Darin Adler 2020-11-02 12:35:52 PST
Alex said review+. Alex, want to do it again so bugs.webkit.org has the record?
Comment 9 Chris Dumez 2020-11-02 12:37:32 PST
Created attachment 412961 [details]
Patch
Comment 10 EWS 2020-11-02 13:21:49 PST
Committed r269271: <https://trac.webkit.org/changeset/269271>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412961 [details].