Bug 1205164 - Fix ShmemPool detection of failed allocations. r=jesup

dom/media/systemservices/CamerasParent.cpp

@@ -161,7 +161,7 @@ CamerasParent::DeliverFrameOverIPC(CaptureEngine cap_engine,
     ShmemBuffer shMemBuff = mShmemPool.Get(this, size);
 
     if (!shMemBuff.Valid()) {
-      LOG(("Video shmem is not writeable in DeliverFrame"));
+      LOG(("No usable Video shmem in DeliverFrame (out of buffers?)"));
       // We can skip this frame if we run out of buffers, it's not a real error.
       return 0;
     }
@@ -175,6 +175,7 @@ CamerasParent::DeliverFrameOverIPC(CaptureEngine cap_engine,
       return -1;
     }
   } else {
+    MOZ_ASSERT(buffer.Valid());
     // ShmemBuffer was available, we're all good. A single copy happened
     // in the original webrtc callback.
     if (!SendDeliverFrame(cap_engine, cap_id,
@@ -205,7 +206,7 @@ CallbackHelper::DeliverFrame(unsigned char* buffer,
   ShmemBuffer shMemBuffer = mParent->GetBuffer(size);
   if (!shMemBuffer.Valid()) {
     // Either we ran out of buffers or they're not the right size yet
-    LOG(("Video shmem is not available in DeliverFrame"));
+    LOG(("Correctly sized Video shmem not available in DeliverFrame"));
     // We will do the copy into a(n extra) temporary buffer inside
     // the DeliverFrameRunnable constructor.
   } else {

dom/media/systemservices/ShmemPool.cpp

@@ -40,12 +40,14 @@ mozilla::ShmemBuffer ShmemPool::GetIfAvailable(size_t aSize)
   ShmemBuffer& res = mShmemPool[mPoolFree - 1];
 
   if (!res.mInitialized) {
+    LOG(("No free preallocated Shmem"));
     return ShmemBuffer();
   }
 
   MOZ_ASSERT(res.mShmem.IsWritable(), "Pool in Shmem is not writable?");
 
   if (res.mShmem.Size<char>() < aSize) {
+    LOG(("Free Shmem but not of the right size"));
     return ShmemBuffer();
   }
 
@@ -71,28 +73,36 @@ mozilla::ShmemBuffer ShmemPool::Get(T* aInstance, size_t aSize)
     return ShmemBuffer();
   }
 
-  ShmemBuffer res = Move(mShmemPool[mPoolFree - 1]);
+  ShmemBuffer& res = mShmemPool[mPoolFree - 1];
 
   if (!res.mInitialized) {
-    LOG(("Initiaizing new Shmem in pool"));
-    aInstance->AllocShmem(aSize, SharedMemory::TYPE_BASIC, &res.mShmem);
+    LOG(("Initializing new Shmem in pool"));
+    if (!aInstance->AllocShmem(aSize, SharedMemory::TYPE_BASIC, &res.mShmem)) {
+      LOG(("Failure allocating new Shmem buffer"));
+      return ShmemBuffer();
+    }
     res.mInitialized = true;
   }
 
-  MOZ_ASSERT(res.mShmem.IsWritable(), "Pool in Shmem is not writable?");
+  MOZ_ASSERT(res.mShmem.IsWritable(), "Shmem in Pool is not writable?");
 
   // Prepare buffer, increase size if needed (we never shrink as we don't
   // maintain seperate sized pools and we don't want to keep reallocating)
   if (res.mShmem.Size<char>() < aSize) {
     LOG(("Size change/increase in Shmem Pool"));
     aInstance->DeallocShmem(res.mShmem);
+    res.mInitialized = false;
     // this may fail; always check return value
     if (!aInstance->AllocShmem(aSize, SharedMemory::TYPE_BASIC, &res.mShmem)) {
-      LOG(("Failure allocating new size Shmem buffer"));
+      LOG(("Failure allocating resized Shmem buffer"));
       return ShmemBuffer();
+    } else {
+      res.mInitialized = true;
     }
   }
 
+  MOZ_ASSERT(res.mShmem.IsWritable(), "Shmem in Pool is not writable post resize?");
+
   mPoolFree--;
 #ifdef DEBUG
   size_t poolUse = mShmemPool.Length() - mPoolFree;
@@ -101,7 +111,7 @@ mozilla::ShmemBuffer ShmemPool::Get(T* aInstance, size_t aSize)
     LOG(("Maximum ShmemPool use increased: %d buffers", mMaxPoolUse));
   }
 #endif
-  return res;
+  return Move(res);
 }
 
 void ShmemPool::Put(ShmemBuffer&& aShmem)
@@ -110,6 +120,12 @@ void ShmemPool::Put(ShmemBuffer&& aShmem)
   MOZ_ASSERT(mPoolFree < mShmemPool.Length());
   mShmemPool[mPoolFree] = Move(aShmem);
   mPoolFree++;
+#ifdef DEBUG
+  size_t poolUse = mShmemPool.Length() - mPoolFree;
+  if (poolUse > 0) {
+    LOG(("ShmemPool usage reduced to %d buffers", poolUse));
+  }
+#endif
 }
 
 template <class T>