bug 1198656 refactor acquiring the content into an object method r=padenot
authorKarl Tomlinson <karlt+@karlt.net>
Tue, 25 Aug 2015 08:51:55 +1200
changeset 294336 5376e4042cf34a8845250397d670275e9e000cc4
parent 294335 b27cf1d8880741a8cab12cd7be23b6a084e08e35
child 294337 d325ca7fa94cccbd0a3f30ce7bf5c22618a7054d
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1198656
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1198656 refactor acquiring the content into an object method r=padenot This makes it clearer that the algorithm is intentionally aborted when any of the buffers have been neutered and that the stolen data has the correct length. It also makes mJSChannels available for clearing in a subsequent changeset.
dom/media/webaudio/AudioBuffer.cpp
dom/media/webaudio/AudioBuffer.h
--- a/dom/media/webaudio/AudioBuffer.cpp
+++ b/dom/media/webaudio/AudioBuffer.cpp
@@ -109,16 +109,18 @@ JSObject*
 AudioBuffer::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return AudioBufferBinding::Wrap(aCx, this, aGivenProto);
 }
 
 bool
 AudioBuffer::RestoreJSChannelData(JSContext* aJSContext)
 {
+  // "4. Attach ArrayBuffers containing copies of the data to the AudioBuffer,
+  // to be returned by the next call to getChannelData."
   if (mSharedChannels) {
     for (uint32_t i = 0; i < mJSChannels.Length(); ++i) {
       const float* data = mSharedChannels->GetData(i);
       // The following code first zeroes the array and then copies our data
       // into it. We could avoid this with additional JS APIs to construct
       // an array (or ArrayBuffer) containing initial data.
       JS::Rooted<JSObject*> array(aJSContext,
                                   JS_NewFloat32Array(aJSContext, mLength));
@@ -212,52 +214,58 @@ AudioBuffer::GetChannelData(JSContext* a
   }
 
   if (mJSChannels[aChannel]) {
     JS::ExposeObjectToActiveJS(mJSChannels[aChannel]);
   }
   aRetval.set(mJSChannels[aChannel]);
 }
 
-static already_AddRefed<ThreadSharedFloatArrayBufferList>
-StealJSArrayDataIntoThreadSharedFloatArrayBufferList(JSContext* aJSContext,
-                                                     const nsTArray<JSObject*>& aJSArrays)
+already_AddRefed<ThreadSharedFloatArrayBufferList>
+AudioBuffer::StealJSArrayDataIntoSharedChannels(JSContext* aJSContext)
 {
+  // "1. If any of the AudioBuffer's ArrayBuffer have been neutered, abort
+  // these steps, and return a zero-length channel data buffers to the
+  // invoker."
+  for (uint32_t i = 0; i < mJSChannels.Length(); ++i) {
+    if (mLength != JS_GetTypedArrayLength(mJSChannels[i])) {
+      // Probably one of the arrays was neutered
+      return nullptr;
+    }
+  }
+
+  // "2. Neuter all ArrayBuffers for arrays previously returned by
+  // getChannelData on this AudioBuffer."
+  // "3. Retain the underlying data buffers from those ArrayBuffers and return
+  // references to them to the invoker."
   nsRefPtr<ThreadSharedFloatArrayBufferList> result =
-    new ThreadSharedFloatArrayBufferList(aJSArrays.Length());
-  for (uint32_t i = 0; i < aJSArrays.Length(); ++i) {
-    JS::Rooted<JSObject*> arrayBufferView(aJSContext, aJSArrays[i]);
+    new ThreadSharedFloatArrayBufferList(mJSChannels.Length());
+  for (uint32_t i = 0; i < mJSChannels.Length(); ++i) {
+    JS::Rooted<JSObject*> arrayBufferView(aJSContext, mJSChannels[i]);
     JS::Rooted<JSObject*> arrayBuffer(aJSContext,
                                       JS_GetArrayBufferViewBuffer(aJSContext, arrayBufferView));
     auto stolenData = arrayBuffer
       ? static_cast<float*>(JS_StealArrayBufferContents(aJSContext,
                                                         arrayBuffer))
       : nullptr;
     if (stolenData) {
       result->SetData(i, stolenData, js_free, stolenData);
     } else {
+      NS_ASSERTION(i == 0, "some channels lost when contents not acquired");
       return nullptr;
     }
   }
   return result.forget();
 }
 
 ThreadSharedFloatArrayBufferList*
 AudioBuffer::GetThreadSharedChannelsForRate(JSContext* aJSContext)
 {
   if (!mSharedChannels) {
-    for (uint32_t i = 0; i < mJSChannels.Length(); ++i) {
-      if (mLength != JS_GetTypedArrayLength(mJSChannels[i])) {
-        // Probably one of the arrays was neutered
-        return nullptr;
-      }
-    }
-
-    mSharedChannels =
-      StealJSArrayDataIntoThreadSharedFloatArrayBufferList(aJSContext, mJSChannels);
+    mSharedChannels = StealJSArrayDataIntoSharedChannels(aJSContext);
   }
 
   return mSharedChannels;
 }
 
 size_t
 AudioBuffer::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
--- a/dom/media/webaudio/AudioBuffer.h
+++ b/dom/media/webaudio/AudioBuffer.h
@@ -106,16 +106,20 @@ public:
 protected:
   AudioBuffer(AudioContext* aContext, uint32_t aNumberOfChannels,
               uint32_t aLength, float aSampleRate,
               already_AddRefed<ThreadSharedFloatArrayBufferList>
                 aInitialContents);
   ~AudioBuffer();
 
   bool RestoreJSChannelData(JSContext* aJSContext);
+
+  already_AddRefed<ThreadSharedFloatArrayBufferList>
+  StealJSArrayDataIntoSharedChannels(JSContext* aJSContext);
+
   void ClearJSChannels();
 
   nsWeakPtr mOwnerWindow;
   // Float32Arrays
   nsAutoTArray<JS::Heap<JSObject*>, 2> mJSChannels;
 
   // mSharedChannels aggregates the data from mJSChannels. This is non-null
   // if and only if the mJSChannels are neutered.