Skip to content

Commit 135ae60

Browse files
cdumezcdumez
committed
[ macOS ] 3 webaudio/OfflineAudioContext/ layout-tests are flakey text failures
https://bugs.webkit.org/show_bug.cgi?id=224387 <rdar://problem/76468058> Reviewed by Eric Carlson. Source/WebCore: Replace internals.numberOfBaseAudioContexts() test infrastructure with internals.baseAudioContextIdentifier() & internals.isBaseAudioContextAlive(). This allows tests to check if specific BaseAudioContext instances created by the tests are actually alive. As a result, the tests are no longer impacted by tests running before them (and potentially leaking, see Bug 224399) or in parallel to them in the same process. This is the same approach we used for leak testing Documents (internals.documentIdentifier() & internals.isDocumentAlive()). * Modules/webaudio/BaseAudioContext.cpp: (WebCore::generateAudioContextID): (WebCore::liveAudioContexts): (WebCore::BaseAudioContext::BaseAudioContext): (WebCore::BaseAudioContext::~BaseAudioContext): (WebCore::BaseAudioContext::isContextAlive): * Modules/webaudio/BaseAudioContext.h: (WebCore::BaseAudioContext::contextID const): * testing/Internals.cpp: (WebCore::Internals::countMatchesForText): (WebCore::Internals::baseAudioContextIdentifier): (WebCore::Internals::isBaseAudioContextAlive): * testing/Internals.h: * testing/Internals.idl: LayoutTests: Update OfflineAudioContext leak tests to use the new test infrastructure. They are no longer impacted by tests running before them or in parallel to them. * webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering-expected.txt: * webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering.html: * webaudio/OfflineAudioContext/offlineaudiocontext-leak-expected.txt: * webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended-expected.txt: * webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended.html: * webaudio/OfflineAudioContext/offlineaudiocontext-leak.html: * webaudio/resources/audiocontext-leak-test.js: Added. (didGCAtLeastOneContext): (gcAndCheckForContextLeaks): Canonical link: https://commits.webkit.org/236369@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275798 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 342fdba commit 135ae60

14 files changed

Lines changed: 157 additions & 73 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
2021-04-10 Chris Dumez <cdumez@apple.com>
2+
3+
[ macOS ] 3 webaudio/OfflineAudioContext/ layout-tests are flakey text failures
4+
https://bugs.webkit.org/show_bug.cgi?id=224387
5+
<rdar://problem/76468058>
6+
7+
Reviewed by Eric Carlson.
8+
9+
Update OfflineAudioContext leak tests to use the new test infrastructure. They are no longer impacted
10+
by tests running before them or in parallel to them.
11+
12+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering-expected.txt:
13+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering.html:
14+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak-expected.txt:
15+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended-expected.txt:
16+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended.html:
17+
* webaudio/OfflineAudioContext/offlineaudiocontext-leak.html:
18+
* webaudio/resources/audiocontext-leak-test.js: Added.
19+
(didGCAtLeastOneContext):
20+
(gcAndCheckForContextLeaks):
21+
122
2021-04-09 Antoine Quint <graouts@webkit.org>
223

324
calc() simplification for a multiplication should apply the multiplication to each value of an addition

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering-expected.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ Makes sure that the OfflineAudioContext objects are not leaking after rendering.
33
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
44

55

6-
PASS internals.numberOfBaseAudioContexts() >= instancesToCreate is true
7-
PASS internals.numberOfBaseAudioContexts() < instancesToCreate is true
6+
PASS didGCAtLeastOneContext() is true
87
PASS successfullyParsed is true
98

109
TEST COMPLETE

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak-after-rendering.html‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,21 @@
22
<html>
33
<body>
44
<script src="../../resources/js-test.js"></script>
5+
<script src="../resources/audiocontext-leak-test.js"></script>
56
<script>
67
description("Makes sure that the OfflineAudioContext objects are not leaking after rendering.");
78
jsTestIsAsync = true;
89

910
const instancesToCreate = 100;
10-
11-
function test() {
12-
let promises = [];
13-
for (let i = 0; i < instancesToCreate; i++) {
14-
let context = new OfflineAudioContext(2, 1, 44100);
15-
promises.push(context.startRendering());
16-
}
17-
shouldBeTrue("internals.numberOfBaseAudioContexts() >= instancesToCreate");
18-
Promise.all(promises).then((values) => {
19-
gc();
20-
setTimeout(() => {
21-
gc();
22-
shouldBeTrue("internals.numberOfBaseAudioContexts() < instancesToCreate");
23-
finishJSTest();
24-
}, 0);
25-
});
11+
let renderingPromises = [];
12+
for (let i = 0; i < instancesToCreate; i++) {
13+
let context = new OfflineAudioContext(2, 1, 44100);
14+
trackContextForLeaks(context);
15+
renderingPromises.push(context.startRendering());
2616
}
27-
28-
test();
17+
Promise.all(renderingPromises).then((values) => {
18+
gcAndCheckForContextLeaks();
19+
});
2920
</script>
3021
</body>
3122
</html>

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak-expected.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ Makes sure that the OfflineAudioContext objects are not leaking.
33
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
44

55

6-
PASS internals.numberOfBaseAudioContexts() >= instancesToCreate is true
7-
PASS internals.numberOfBaseAudioContexts() < instancesToCreate is true
6+
PASS didGCAtLeastOneContext() is true
87
PASS successfullyParsed is true
98

109
TEST COMPLETE

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended-expected.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ Makes sure that the OfflineAudioContext objects are not leaking when suspended.
33
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
44

55

6-
PASS internals.numberOfBaseAudioContexts() >= instancesToCreate is true
7-
PASS internals.numberOfBaseAudioContexts() < instancesToCreate is true
6+
PASS didGCAtLeastOneContext() is true
87
PASS successfullyParsed is true
98

109
TEST COMPLETE

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak-while-suspended.html‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,23 @@
22
<html>
33
<body>
44
<script src="../../resources/js-test.js"></script>
5+
<script src="../resources/audiocontext-leak-test.js"></script>
56
<script>
67
description("Makes sure that the OfflineAudioContext objects are not leaking when suspended.");
78
jsTestIsAsync = true;
89

910
const instancesToCreate = 100;
10-
11-
function test() {
12-
let promises = [];
13-
for (let i = 0; i < instancesToCreate; i++) {
14-
let context = new OfflineAudioContext(2, 44100, 44100);
15-
promises.push(context.suspend(0.5));
16-
context.startRendering();
17-
}
18-
shouldBeTrue("internals.numberOfBaseAudioContexts() >= instancesToCreate");
19-
Promise.all(promises).then((values) => {
20-
gc();
21-
setTimeout(() => {
22-
gc();
23-
shouldBeTrue("internals.numberOfBaseAudioContexts() < instancesToCreate");
24-
finishJSTest();
25-
}, 0);
26-
});
11+
let suspendPromises = [];
12+
for (let i = 0; i < instancesToCreate; i++) {
13+
let context = new OfflineAudioContext(2, 44100, 44100);
14+
trackContextForLeaks(context);
15+
suspendPromises.push(context.suspend(0.5));
16+
context.startRendering();
2717
}
2818

29-
test();
19+
Promise.all(suspendPromises).then((values) => {
20+
gcAndCheckForContextLeaks();
21+
});
3022
</script>
3123
</body>
3224
</html>

‎LayoutTests/webaudio/OfflineAudioContext/offlineaudiocontext-leak.html‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,16 @@
22
<html>
33
<body>
44
<script src="../../resources/js-test.js"></script>
5+
<script src="../resources/audiocontext-leak-test.js"></script>
56
<script>
67
description("Makes sure that the OfflineAudioContext objects are not leaking.");
78
jsTestIsAsync = true;
89

910
const instancesToCreate = 100;
11+
for (let i = 0; i < instancesToCreate; i++)
12+
trackContextForLeaks(new OfflineAudioContext(2, 1, 44100));
1013

11-
function allocateContexts() {
12-
let contexts = [];
13-
for (let i = 0; i < instancesToCreate; i++)
14-
contexts.push(new OfflineAudioContext(2, 1, 44100));
15-
shouldBeTrue("internals.numberOfBaseAudioContexts() >= instancesToCreate");
16-
}
17-
18-
allocateContexts();
19-
gc();
20-
setTimeout(() => {
21-
gc();
22-
shouldBeTrue("internals.numberOfBaseAudioContexts() < instancesToCreate");
23-
finishJSTest();
24-
}, 0);
14+
gcAndCheckForContextLeaks();
2515
</script>
2616
</body>
2717
</html>

‎LayoutTests/webaudio/resources/audiocontext-leak-test.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
let contextIDs = [];
2+
3+
function trackContextForLeaks(context)
4+
{
5+
let contextID = internals.baseAudioContextIdentifier(context);
6+
contextIDs.push(contextID);
7+
8+
// Sanity check.
9+
if (!internals.isBaseAudioContextAlive(contextID))
10+
testFailed("Test harness failure: internals.isBaseAudioContextAlive() is not working as expected!");
11+
}
12+
13+
function didGCAtLeastOneContext()
14+
{
15+
for (let contextID of contextIDs) {
16+
if (!internals.isBaseAudioContextAlive(contextID))
17+
return true;
18+
}
19+
return false;
20+
}
21+
22+
function gcAndCheckForContextLeaks()
23+
{
24+
gc();
25+
setTimeout(() => {
26+
gc();
27+
shouldBeTrue("didGCAtLeastOneContext()");
28+
finishJSTest();
29+
}, 0);
30+
}

‎Source/WebCore/ChangeLog‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,36 @@
1+
2021-04-10 Chris Dumez <cdumez@apple.com>
2+
3+
[ macOS ] 3 webaudio/OfflineAudioContext/ layout-tests are flakey text failures
4+
https://bugs.webkit.org/show_bug.cgi?id=224387
5+
<rdar://problem/76468058>
6+
7+
Reviewed by Eric Carlson.
8+
9+
Replace internals.numberOfBaseAudioContexts() test infrastructure with
10+
internals.baseAudioContextIdentifier() & internals.isBaseAudioContextAlive().
11+
This allows tests to check if specific BaseAudioContext instances created by the
12+
tests are actually alive. As a result, the tests are no longer impacted by tests
13+
running before them (and potentially leaking, see Bug 224399) or in parallel to
14+
them in the same process.
15+
16+
This is the same approach we used for leak testing Documents (internals.documentIdentifier()
17+
& internals.isDocumentAlive()).
18+
19+
* Modules/webaudio/BaseAudioContext.cpp:
20+
(WebCore::generateAudioContextID):
21+
(WebCore::liveAudioContexts):
22+
(WebCore::BaseAudioContext::BaseAudioContext):
23+
(WebCore::BaseAudioContext::~BaseAudioContext):
24+
(WebCore::BaseAudioContext::isContextAlive):
25+
* Modules/webaudio/BaseAudioContext.h:
26+
(WebCore::BaseAudioContext::contextID const):
27+
* testing/Internals.cpp:
28+
(WebCore::Internals::countMatchesForText):
29+
(WebCore::Internals::baseAudioContextIdentifier):
30+
(WebCore::Internals::isBaseAudioContextAlive):
31+
* testing/Internals.h:
32+
* testing/Internals.idl:
33+
134
2021-04-10 Mark Lam <mark.lam@apple.com>
235

336
Enable VMTraps checks in RETURN_IF_EXCEPTION.

‎Source/WebCore/Modules/webaudio/BaseAudioContext.cpp‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,21 @@ bool BaseAudioContext::isSupportedSampleRate(float sampleRate)
115115
return sampleRate >= 3000 && sampleRate <= 384000;
116116
}
117117

118-
static unsigned numberOfBaseAudioContexts = 0;
119-
unsigned BaseAudioContext::s_hardwareContextCount = 0;
118+
unsigned BaseAudioContext::s_hardwareContextCount;
119+
120+
static uint64_t generateAudioContextID()
121+
{
122+
ASSERT(isMainThread());
123+
static uint64_t contextIDSeed = 0;
124+
return ++contextIDSeed;
125+
}
126+
127+
static HashSet<uint64_t>& liveAudioContexts()
128+
{
129+
ASSERT(isMainThread());
130+
static NeverDestroyed<HashSet<uint64_t>> contexts;
131+
return contexts;
132+
}
120133

121134
// Constructor for rendering to the audio hardware.
122135
BaseAudioContext::BaseAudioContext(Document& document, IsLegacyWebKitAudioContext isLegacyWebKitAudioContext, const AudioContextOptions& contextOptions)
@@ -125,11 +138,12 @@ BaseAudioContext::BaseAudioContext(Document& document, IsLegacyWebKitAudioContex
125138
, m_logger(document.logger())
126139
, m_logIdentifier(uniqueLogIdentifier())
127140
#endif
141+
, m_contextID(generateAudioContextID())
128142
, m_worklet(AudioWorklet::create(*this))
129143
, m_destinationNode(makeUniqueRef<DefaultAudioDestinationNode>(*this, contextOptions.sampleRate))
130144
, m_listener(isLegacyWebKitAudioContext == IsLegacyWebKitAudioContext::Yes ? Ref<AudioListener>(WebKitAudioListener::create(*this)) : AudioListener::create(*this))
131145
{
132-
++numberOfBaseAudioContexts;
146+
liveAudioContexts().add(m_contextID);
133147

134148
// According to spec AudioContext must die only after page navigate.
135149
// Lets mark it as ActiveDOMObject with pending activity and unmark it in clear method.
@@ -155,20 +169,20 @@ BaseAudioContext::BaseAudioContext(Document& document, IsLegacyWebKitAudioContex
155169
, m_logger(document.logger())
156170
, m_logIdentifier(uniqueLogIdentifier())
157171
#endif
172+
, m_contextID(generateAudioContextID())
158173
, m_worklet(AudioWorklet::create(*this))
159174
, m_isOfflineContext(true)
160175
, m_renderTarget(WTFMove(renderTarget))
161176
, m_destinationNode(makeUniqueRef<OfflineAudioDestinationNode>(*this, numberOfChannels, sampleRate, m_renderTarget.copyRef()))
162177
, m_listener(isLegacyWebKitAudioContext == IsLegacyWebKitAudioContext::Yes ? Ref<AudioListener>(WebKitAudioListener::create(*this)) : AudioListener::create(*this))
163178
{
164-
++numberOfBaseAudioContexts;
179+
liveAudioContexts().add(m_contextID);
165180
FFTFrame::initialize();
166181
}
167182

168183
BaseAudioContext::~BaseAudioContext()
169184
{
170-
ASSERT(numberOfBaseAudioContexts);
171-
--numberOfBaseAudioContexts;
185+
liveAudioContexts().remove(m_contextID);
172186
#if DEBUG_AUDIONODE_REFERENCES
173187
fprintf(stderr, "%p: BaseAudioContext::~AudioContext()\n", this);
174188
#endif
@@ -181,9 +195,9 @@ BaseAudioContext::~BaseAudioContext()
181195
// FIXME: Can we assert that m_deferredBreakConnectionList is empty?
182196
}
183197

184-
unsigned BaseAudioContext::numberOfInstances()
198+
bool BaseAudioContext::isContextAlive(uint64_t contextID)
185199
{
186-
return numberOfBaseAudioContexts;
200+
return liveAudioContexts().contains(contextID);
187201
}
188202

189203
void BaseAudioContext::lazyInitialize()

0 commit comments

Comments
 (0)