Detach buffer in decodeAudioData

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

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 266 matching lines @@
   DCHECK(isMainThread());
   DCHECK(audioData);
 
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
 
   float rate = isContextClosed() ? closedContextSampleRate() : sampleRate();
 
   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;
 }
 
 void BaseAudioContext::handleDecodeAudioData(
     AudioBuffer* audioBuffer,
     ScriptPromiseResolver* resolver,
     AudioBufferCallback* successCallback,
     AudioBufferCallback* errorCallback) {
   DCHECK(isMainThread());
@@ skipping 601 matching lines @@
 }
 
 SecurityOrigin* BaseAudioContext::getSecurityOrigin() const {
   if (getExecutionContext())
     return getExecutionContext()->getSecurityOrigin();
 
   return nullptr;
 }
 
 }  // namespace blink