Skip to content

Commit a0cb972

Browse files
cdumezcdumez
committed
Crash when accessing OfflineAudioContext.length after failing to construct rendering AudioBuffer
https://bugs.webkit.org/show_bug.cgi?id=218754 <rdar://problem/71186978> Reviewed by Eric Carlson. Source/WebCore: OfflineAudioContext.length should return the length passed to the constructor, even if we failed to construct the internal AudioBuffer (and obviously we should not crash). This matches the behavior of Firefox and Chrome. I have also added a console message when we fail to construct the internal rendering AudioBuffer, for clarity. Test: webaudio/OfflineAudioContext/bad-buffer-length.html * Modules/webaudio/OfflineAudioContext.cpp: (WebCore::OfflineAudioContext::OfflineAudioContext): (WebCore::OfflineAudioContext::create): * Modules/webaudio/OfflineAudioContext.h: LayoutTests: Add layout test coverage and rebaseline a couple of tests now that a console message is logged. * webaudio/OfflineAudioContext-bad-buffer-crash-expected.txt: * webaudio/OfflineAudioContext/bad-buffer-length-expected.txt: Added. * webaudio/OfflineAudioContext/bad-buffer-length.html: Added. * webaudio/dom-exceptions-expected.txt: Canonical link: https://commits.webkit.org/231422@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@269632 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent e56bbb1 commit a0cb972

8 files changed

Lines changed: 72 additions & 11 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
2020-11-10 Chris Dumez <cdumez@apple.com>
2+
3+
Crash when accessing OfflineAudioContext.length after failing to construct rendering AudioBuffer
4+
https://bugs.webkit.org/show_bug.cgi?id=218754
5+
<rdar://problem/71186978>
6+
7+
Reviewed by Eric Carlson.
8+
9+
Add layout test coverage and rebaseline a couple of tests now that a console message is logged.
10+
11+
* webaudio/OfflineAudioContext-bad-buffer-crash-expected.txt:
12+
* webaudio/OfflineAudioContext/bad-buffer-length-expected.txt: Added.
13+
* webaudio/OfflineAudioContext/bad-buffer-length.html: Added.
14+
* webaudio/dom-exceptions-expected.txt:
15+
116
2020-11-10 Jer Noble <jer.noble@apple.com>
217

318
Add support for AudioConfiguration.spatialRendering

‎LayoutTests/webaudio/OfflineAudioContext-bad-buffer-crash-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
CONSOLE MESSAGE: Failed to construct internal AudioBuffer with 1 channel(s), a sample rate of 44000 and a length of 536870912.
12
This test passes if it does not crash.
23

34
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

‎LayoutTests/webaudio/OfflineAudioContext/bad-buffer-length-expected.txt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CONSOLE MESSAGE: Failed to construct internal AudioBuffer with 1 channel(s), a sample rate of 44000 and a length of 536870912.
2+
Make sure that the length returned by OfflineAudioContext even if we failed to construct the rendering buffer.
3+
4+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
5+
6+
7+
PASS new OfflineAudioContext(1, 2**29, 44000).length is 2**29
8+
PASS successfullyParsed is true
9+
10+
TEST COMPLETE
11+

‎LayoutTests/webaudio/OfflineAudioContext/bad-buffer-length.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<body>
4+
<script src="../../resources/js-test.js"></script>
5+
<script>
6+
description("Make sure that the length returned by OfflineAudioContext even if we failed to construct the rendering buffer.");
7+
8+
shouldBe("new OfflineAudioContext(1, 2**29, 44000).length", "2**29");
9+
</script>
10+
</body>
11+
</html>

‎LayoutTests/webaudio/dom-exceptions-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
CONSOLE MESSAGE: Failed to construct internal AudioBuffer with 1 channel(s), a sample rate of 44100 and a length of 1448390656.
12

23
PASS # AUDIT TASK RUNNER STARTED.
34
PASS Executing "initialize"

‎Source/WebCore/ChangeLog‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
2020-11-10 Chris Dumez <cdumez@apple.com>
2+
3+
Crash when accessing OfflineAudioContext.length after failing to construct rendering AudioBuffer
4+
https://bugs.webkit.org/show_bug.cgi?id=218754
5+
<rdar://problem/71186978>
6+
7+
Reviewed by Eric Carlson.
8+
9+
OfflineAudioContext.length should return the length passed to the constructor, even if we
10+
failed to construct the internal AudioBuffer (and obviously we should not crash). This
11+
matches the behavior of Firefox and Chrome.
12+
13+
I have also added a console message when we fail to construct the internal rendering
14+
AudioBuffer, for clarity.
15+
16+
Test: webaudio/OfflineAudioContext/bad-buffer-length.html
17+
18+
* Modules/webaudio/OfflineAudioContext.cpp:
19+
(WebCore::OfflineAudioContext::OfflineAudioContext):
20+
(WebCore::OfflineAudioContext::create):
21+
* Modules/webaudio/OfflineAudioContext.h:
22+
123
2020-11-10 Jer Noble <jer.noble@apple.com>
224

325
Add support for AudioConfiguration.spatialRendering

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,13 @@ namespace WebCore {
3838

3939
WTF_MAKE_ISO_ALLOCATED_IMPL(OfflineAudioContext);
4040

41-
inline OfflineAudioContext::OfflineAudioContext(Document& document, unsigned numberOfChannels, float sampleRate, RefPtr<AudioBuffer>&& renderTarget)
41+
inline OfflineAudioContext::OfflineAudioContext(Document& document, unsigned numberOfChannels, unsigned length, float sampleRate, RefPtr<AudioBuffer>&& renderTarget)
4242
: BaseAudioContext(document, numberOfChannels, sampleRate, WTFMove(renderTarget))
43+
, m_length(length)
4344
{
4445
}
4546

46-
ExceptionOr<Ref<OfflineAudioContext>> OfflineAudioContext::create(ScriptExecutionContext& context, unsigned numberOfChannels, size_t length, float sampleRate)
47+
ExceptionOr<Ref<OfflineAudioContext>> OfflineAudioContext::create(ScriptExecutionContext& context, unsigned numberOfChannels, unsigned length, float sampleRate)
4748
{
4849
if (!is<Document>(context))
4950
return Exception { NotSupportedError, "OfflineAudioContext is only supported in Document contexts"_s };
@@ -55,7 +56,10 @@ ExceptionOr<Ref<OfflineAudioContext>> OfflineAudioContext::create(ScriptExecutio
5556
return Exception { SyntaxError, "sampleRate is not in range"_s };
5657

5758
auto renderTarget = AudioBuffer::create(numberOfChannels, length, sampleRate);
58-
auto audioContext = adoptRef(*new OfflineAudioContext(downcast<Document>(context), numberOfChannels, sampleRate, WTFMove(renderTarget)));
59+
if (!renderTarget)
60+
context.addConsoleMessage(MessageSource::JS, MessageLevel::Warning, makeString("Failed to construct internal AudioBuffer with ", numberOfChannels, " channel(s), a sample rate of ", sampleRate, " and a length of ", length, "."));
61+
62+
auto audioContext = adoptRef(*new OfflineAudioContext(downcast<Document>(context), numberOfChannels, length, sampleRate, WTFMove(renderTarget)));
5963
audioContext->suspendIfNeeded();
6064
return audioContext;
6165
}
@@ -217,11 +221,6 @@ void OfflineAudioContext::didFinishOfflineRendering(ExceptionOr<Ref<AudioBuffer>
217221
promise->resolve<IDLInterface<AudioBuffer>>(result.releaseReturnValue());
218222
}
219223

220-
unsigned OfflineAudioContext::length() const
221-
{
222-
return renderTarget()->length();
223-
}
224-
225224
void OfflineAudioContext::offlineLock(bool& mustReleaseLock)
226225
{
227226
ASSERT(!isMainThread());

‎Source/WebCore/Modules/webaudio/OfflineAudioContext.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ namespace WebCore {
3636
class OfflineAudioContext final : public BaseAudioContext {
3737
WTF_MAKE_ISO_ALLOCATED(OfflineAudioContext);
3838
public:
39-
static ExceptionOr<Ref<OfflineAudioContext>> create(ScriptExecutionContext&, unsigned numberOfChannels, size_t length, float sampleRate);
39+
static ExceptionOr<Ref<OfflineAudioContext>> create(ScriptExecutionContext&, unsigned numberOfChannels, unsigned length, float sampleRate);
4040

4141
static ExceptionOr<Ref<OfflineAudioContext>> create(ScriptExecutionContext&, const OfflineAudioContextOptions&);
4242

4343
void startOfflineRendering(Ref<DeferredPromise>&&);
4444
void suspendOfflineRendering(double suspendTime, Ref<DeferredPromise>&&);
4545
void resumeOfflineRendering(Ref<DeferredPromise>&&);
4646

47-
unsigned length() const;
47+
unsigned length() const { return m_length; }
4848
bool shouldSuspend() final;
4949

5050
OfflineAudioDestinationNode* destination() { return static_cast<OfflineAudioDestinationNode*>(BaseAudioContext::destination()); }
@@ -72,14 +72,15 @@ class OfflineAudioContext final : public BaseAudioContext {
7272
};
7373

7474
private:
75-
OfflineAudioContext(Document&, unsigned numberOfChannels, float sampleRate, RefPtr<AudioBuffer>&& renderTarget);
75+
OfflineAudioContext(Document&, unsigned numberOfChannels, unsigned length, float sampleRate, RefPtr<AudioBuffer>&& renderTarget);
7676

7777
void didFinishOfflineRendering(ExceptionOr<Ref<AudioBuffer>>&&) final;
7878
void didSuspendRendering(size_t frame) final;
7979
void uninitialize() final;
8080

8181
RefPtr<DeferredPromise> m_pendingOfflineRenderingPromise;
8282
HashMap<unsigned /* frame */, RefPtr<DeferredPromise>, WTF::IntHash<unsigned>, WTF::UnsignedWithZeroKeyHashTraits<unsigned>> m_suspendRequests;
83+
unsigned m_length;
8384
bool m_didStartOfflineRendering { false };
8485
};
8586

0 commit comments

Comments
 (0)