Backed out 2 changesets (bug 1493779) for test_convolverNodeOOM.html failures CLOSED TREE
authorBogdan Tara <btara@mozilla.com>
Thu, 04 Oct 2018 20:24:19 +0300
changeset 495382 d9f4923892754c462907bdc524efcf37d75306a7
parent 495381 5dc2619c0a5bac07a3e7409004589e2fceade1b9
child 495383 a816c74a4f6b01d9d87e627d0c72e938b597e460
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1493779
milestone64.0a1
backs oute78d9996b1a44045dcf8a85722189a3db77033d3
53950ea4b1055b291c417948cc685f044fef97aa
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
Backed out 2 changesets (bug 1493779) for test_convolverNodeOOM.html failures CLOSED TREE Backed out changeset e78d9996b1a4 (bug 1493779) Backed out changeset 53950ea4b105 (bug 1493779)
dom/locales/en-US/chrome/dom/dom.properties
dom/media/webaudio/ConvolverNode.cpp
dom/media/webaudio/blink/Reverb.cpp
dom/media/webaudio/blink/Reverb.h
dom/media/webaudio/test/mochitest.ini
dom/media/webaudio/test/test_convolverNodeOOM.html
--- a/dom/locales/en-US/chrome/dom/dom.properties
+++ b/dom/locales/en-US/chrome/dom/dom.properties
@@ -102,18 +102,16 @@ MediaDecodeAudioDataNoAudio=The buffer p
 MediaElementAudioSourceNodeCrossOrigin=The HTMLMediaElement passed to createMediaElementSource has a cross-origin resource, the node will output silence.
 # LOCALIZATION NOTE: Do not translate MediaStream and createMediaStreamSource.
 MediaStreamAudioSourceNodeCrossOrigin=The MediaStream passed to createMediaStreamSource has a cross-origin resource, the node will output silence.
 # LOCALIZATION NOTE: Do not translate HTMLMediaElement and MediaStream.
 MediaElementAudioCaptureOfMediaStreamError=The captured HTMLMediaElement is playing a MediaStream. Applying volume or mute status is not currently supported.
 MediaLoadExhaustedCandidates=All candidate resources failed to load. Media load paused.
 MediaLoadSourceMissingSrc=<source> element has no “src” attribute. Media resource load failed.
 MediaStreamAudioSourceNodeDifferentRate=Connecting AudioNodes from AudioContexts with different sample-rate is currently not supported.
-# LOCALIZATION NOTE: Do not translate ConvolverNode
-ConvolverNodeAllocationError=Out-of-memory error when instantiating a ConvolverNode: the node will output silence.
 # LOCALIZATION NOTE: %1$S is the Http error code the server returned (e.g. 404, 500, etc), %2$S is the URL of the media resource which failed to load.
 MediaLoadHttpError=HTTP load failed with status %1$S. Load of media resource %2$S failed.
 # LOCALIZATION NOTE: %S is the URL of the media resource which failed to load.
 MediaLoadInvalidURI=Invalid URI. Load of media resource %S failed.
 # LOCALIZATION NOTE: %1$S is the media resource's format/codec type (basically equivalent to the file type, e.g. MP4,AVI,WMV,MOV etc), %2$S is the URL of the media resource which failed to load.
 MediaLoadUnsupportedTypeAttribute=Specified “type” attribute of “%1$S” is not supported. Load of media resource %2$S failed.
 # LOCALIZATION NOTE: %1$S is the "media" attribute value of the <source> element. It is a media query. %2$S is the URL of the media resource which failed to load.
 MediaLoadSourceMediaNotMatched=Specified “media” attribute of “%1$S” does not match the environment. Load of media resource %2$S failed.
--- a/dom/media/webaudio/ConvolverNode.cpp
+++ b/dom/media/webaudio/ConvolverNode.cpp
@@ -23,19 +23,18 @@ NS_INTERFACE_MAP_END_INHERITING(AudioNod
 
 NS_IMPL_ADDREF_INHERITED(ConvolverNode, AudioNode)
 NS_IMPL_RELEASE_INHERITED(ConvolverNode, AudioNode)
 
 class ConvolverNodeEngine final : public AudioNodeEngine
 {
   typedef PlayingRefChangeHandler PlayingRefChanged;
 public:
-  ConvolverNodeEngine(AudioNode* aNode, bool aNormalize, uint64_t aWindowID)
+  ConvolverNodeEngine(AudioNode* aNode, bool aNormalize)
     : AudioNodeEngine(aNode)
-    , mWindowID(aWindowID)
     , mUseBackgroundThreads(!aNode->Context()->IsOffline())
     , mNormalize(aNormalize)
   {
   }
 
   // Indicates how the right output channel is generated.
   enum class RightConvolverMode {
     // A right convolver is always used when there is more than one impulse
@@ -136,26 +135,18 @@ public:
     }
 
     // Assume for now that convolution of channel difference is not required.
     // Direct may change to Difference during processing.
     mRightConvolverMode =
       aBuffer.ChannelCount() == 1 ? RightConvolverMode::Direct
       : RightConvolverMode::Always;
 
-    bool allocationFailure = false;
     mReverb = new WebCore::Reverb(aBuffer, MaxFFTSize, mUseBackgroundThreads,
-                                  mNormalize, mSampleRate, &allocationFailure);
-    if (allocationFailure) {
-      // If the allocation failed, this AudioNodeEngine is going to output
-      // silence. This is signaled to developers in the console.
-      mReverb = nullptr;
-      WebAudioUtils::LogToDeveloperConsole(mWindowID,
-                                           "ConvolverNodeAllocationError");
-    }
+                                  mNormalize, mSampleRate);
   }
 
   void AllocateReverbInput(const AudioBlock& aInput,
                            uint32_t aTotalChannelCount)
   {
     uint32_t inputChannelCount = aInput.ChannelCount();
     MOZ_ASSERT(inputChannelCount <= aTotalChannelCount);
     mReverbInput.AllocateChannels(aTotalChannelCount);
@@ -200,17 +191,16 @@ public:
   {
     return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf);
   }
 
 private:
   // Keeping mReverbInput across process calls avoids unnecessary reallocation.
   AudioBlock mReverbInput;
   nsAutoPtr<WebCore::Reverb> mReverb;
-  uint64_t mWindowID;
   // Tracks samples of the tail remaining to be output.  INT32_MIN is a
   // special value to indicate that the end of any previous tail has been
   // handled.
   int32_t mRemainingLeftOutput = INT32_MIN;
   // mRemainingRightOutput and mRemainingRightHistory are only used when
   // mRightOutputMode != Always.  There is no special handling required at the
   // end of tail times and so INT32_MIN is not used.
   // mRemainingRightOutput tracks how much longer this node needs to continue
@@ -402,18 +392,17 @@ ConvolverNodeEngine::ProcessBlock(AudioN
 
 ConvolverNode::ConvolverNode(AudioContext* aContext)
   : AudioNode(aContext,
               2,
               ChannelCountMode::Clamped_max,
               ChannelInterpretation::Speakers)
   , mNormalize(true)
 {
-  uint64_t windowID = aContext->GetParentObject()->WindowID();
-  ConvolverNodeEngine* engine = new ConvolverNodeEngine(this, mNormalize, windowID);
+  ConvolverNodeEngine* engine = new ConvolverNodeEngine(this, mNormalize);
   mStream = AudioNodeStream::Create(aContext, engine,
                                     AudioNodeStream::NO_STREAM_FLAGS,
                                     aContext->Graph());
 }
 
 /* static */ already_AddRefed<ConvolverNode>
 ConvolverNode::Create(JSContext* aCx, AudioContext& aAudioContext,
                       const ConvolverOptions& aOptions,
--- a/dom/media/webaudio/blink/Reverb.cpp
+++ b/dom/media/webaudio/blink/Reverb.cpp
@@ -72,36 +72,30 @@ static float calculateNormalizationScale
 
     // True-stereo compensation
     if (numberOfChannels == 4)
         scale *= 0.5f;
 
     return scale;
 }
 
-Reverb::Reverb(const AudioChunk& impulseResponse, size_t maxFFTSize, bool useBackgroundThreads, bool normalize, float sampleRate, bool* aAllocationFailure)
+Reverb::Reverb(const AudioChunk& impulseResponse, size_t maxFFTSize, bool useBackgroundThreads, bool normalize, float sampleRate)
 {
-    MOZ_ASSERT(aAllocationFailure);
     size_t impulseResponseBufferLength = impulseResponse.mDuration;
     float scale = impulseResponse.mVolume;
 
     AutoTArray<const float*,4> irChannels(impulseResponse.ChannelData<float>());
     AutoTArray<float,1024> tempBuf;
 
     if (normalize) {
         scale = calculateNormalizationScale(irChannels, impulseResponseBufferLength, sampleRate);
     }
 
     if (scale != 1.0f) {
-        bool rv = tempBuf.SetLength(irChannels.Length()*impulseResponseBufferLength, mozilla::fallible);
-        *aAllocationFailure = !rv;
-        if (*aAllocationFailure) {
-          return;
-        }
-
+        tempBuf.SetLength(irChannels.Length()*impulseResponseBufferLength);
         for (uint32_t i = 0; i < irChannels.Length(); ++i) {
             float* buf = &tempBuf[i*impulseResponseBufferLength];
             AudioBufferCopyWithScale(irChannels[i], scale, buf,
                                      impulseResponseBufferLength);
             irChannels[i] = buf;
         }
     }
 
--- a/dom/media/webaudio/blink/Reverb.h
+++ b/dom/media/webaudio/blink/Reverb.h
@@ -39,22 +39,18 @@ namespace WebCore {
 
 // Multi-channel convolution reverb with channel matrixing - one or more ReverbConvolver objects are used internally.
 
 class Reverb {
 public:
     enum { MaxFrameSize = 256 };
 
     // renderSliceSize is a rendering hint, so the FFTs can be optimized to not all occur at the same time (very bad when rendering on a real-time thread).
-    // aAllocation failure is to be checked by the caller. If false, internal
-    // memory could not be allocated, and this Reverb instance is not to be
-    // used.
     Reverb(const mozilla::AudioChunk& impulseResponseBuffer, size_t maxFFTSize,
-           bool useBackgroundThreads, bool normalize, float sampleRate,
-           bool* aAllocationFailure);
+           bool useBackgroundThreads, bool normalize, float sampleRate);
 
     void process(const mozilla::AudioBlock* sourceBus,
                  mozilla::AudioBlock* destinationBus);
 
     size_t impulseResponseLength() const { return m_impulseResponseLength; }
 
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
--- a/dom/media/webaudio/test/mochitest.ini
+++ b/dom/media/webaudio/test/mochitest.ini
@@ -114,17 +114,16 @@ skip-if = toolkit == 'android' # bug 105
 [test_channelSplitterNodeWithVolume.html]
 skip-if = (android_version == '18' && debug) # bug 1158417
 [test_convolverNode.html]
 [test_convolverNode_mono_mono.html]
 [test_convolverNodeChannelCount.html]
 [test_convolverNodeChannelInterpretationChanges.html]
 [test_convolverNodeDelay.html]
 [test_convolverNodeFiniteInfluence.html]
-[test_convolverNodeOOM.html]
 [test_convolverNodeNormalization.html]
 [test_convolverNodePassThrough.html]
 [test_convolverNodeWithGain.html]
 [test_convolver-upmixing-1-channel-response.html]
 # This is a copy of
 # testing/web-platform/tests/webaudio/the-audio-api/the-convolvernode-interface/convolver-upmixing-1-channel-response.html,
 # but WPT are not run with ASan or Android builds.
 skip-if = !asan && toolkit != android
deleted file mode 100644
--- a/dom/media/webaudio/test/test_convolverNodeOOM.html
+++ /dev/null
@@ -1,41 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <title>Test ConvolverNode with very large buffer that triggers an OOM</title>
-  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <script type="text/javascript" src="webaudio.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-</head>
-<body>
-<pre id="test">
-<script class="testbody" type="text/javascript">
-
-var gTest = {
-  length: 2048,
-  numberOfChannels: 1,
-  skipOfflineContextTests: true,
-  createGraph: function(context) {
-    var source = context.createOscillator();
-    var convolver = context.createConvolver();
-    // Very big buffer that results in an OOM
-    try {
-      var buffer = context.createBuffer(2, 300000000, context.sampleRate)
-    } catch(e) {
-      // OOM when attempting to create the buffer, this can happen on 32bits
-      // OSes. Simply return here.
-      return convolver;
-    }
-    var channel = buffer.getChannelData(0);
-    source.connect(convolver);
-    convolver.buffer = buffer;
-    source.start();
-    return convolver;
-  }
-};
-
-runTest();
-
-</script>
-</pre>
-</body>
-</html>