Detach buffer in decodeAudioData

third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp

Index: third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
index d0acd8dbd40e27c20f1486655552d659e763b0ab..bbca9785cba69655f2c1cc4d9c3c7bb4c13d1937 100644
--- a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
@@ -284,9 +284,26 @@ ScriptPromise BaseAudioContext::decodeAudioData(
 
   DCHECK_GT(rate, 0);
 
-  m_decodeAudioResolvers.insert(resolver);
-  m_audioDecoder.decodeAsync(audioData, rate, successCallback, errorCallback,
-                             resolver, this);
+  if (audioData->isShared()) {
+    // If audioData is detached, we need to reject the promise with

[haraken, 2017/03/14 03:17:49]

Is isShared() equivalent to a detached buffer in the spec?

[Raymond Toy, 2017/03/14 15:01:03]

I don't know. I can't seem to find the correspondence between the ECMAscript
IsDetachedBuffer and DetachArrayBuffer to anything in blink.

But transfer() has a DCHECK(!isShared()), so that seems, at a minimum, something
we need to ensure.

[binji, 2017/03/14 17:50:55]

Right, a SharedArrayBuffer cannot be detached, and it cannot be transferred. Not
exactly sure what it should do here, but throwing an error seems reasonable.

+    // error TypeError.
+    DOMException* error =
+        DOMException::create(V8TypeError, "Cannot decode detached buffer");

[haraken, 2017/03/14 03:17:49]

Nit: resolver->reject(V8ThrowException::createTypeError(...))

[Raymond Toy, 2017/03/14 19:32:59]

I still need an DOMException object for the errorCallback.  Do you want me to
create that separately before line 294 below?

[haraken, 2017/03/14 19:57:51]

Ah, then DOMException is fine :)

+    resolver->reject(error);
+    if (errorCallback) {
+      errorCallback->handleEvent(error);
+    }
+  } else {
+    // Detach the audio array buffer from the main thread and start
+    // async decoding of the data.
+    WTF::ArrayBufferContents buf;
+    audioData->transfer(buf);

[haraken, 2017/03/14 03:17:49]

Why was this detachment & transfer not needed before this CL?

[Raymond Toy, 2017/03/14 15:01:03]

The original version of the spec didn't have this.  Detach and transfer was
added a while ago and I just never got around to implementing it until now. 
(There are other places we need to do this, which I'l do in a followup.)

+    DOMArrayBuffer* audio = DOMArrayBuffer::create(buf);
+
+    m_decodeAudioResolvers.insert(resolver);
+    m_audioDecoder.decodeAsync(audio, rate, successCallback, errorCallback,
+                               resolver, this);
+  }
 
   return promise;
 }