Bug 1145015 - Part 2: Add more checking rules for GrallocBuffer allocation. r=sotaro, r=nical
authorJerryShih <hshih@mozilla.com>
Tue, 24 Mar 2015 18:40:00 -0400
changeset 264834 03f1dc776f271455b4b9cf2fdd54f4038bd9cac3
parent 264833 6c08aa7c7142a7f3643aa77c962ed4c6e464dbc0
child 264835 d7bfe22ed06657672c9f1baac0d6ee6a60426184
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro, nical
bugs1145015
milestone39.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 1145015 - Part 2: Add more checking rules for GrallocBuffer allocation. r=sotaro, r=nical fix the assert checking.
gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
gfx/layers/ipc/SharedBufferManagerChild.cpp
--- a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
+++ b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
@@ -106,32 +106,36 @@ ParamTraits<MagicGrallocBufferHandle>::W
     aMsg->WriteFileDescriptor(FileDescriptor(fds[n], false));
   }
 }
 
 bool
 ParamTraits<MagicGrallocBufferHandle>::Read(const Message* aMsg,
                                             void** aIter, paramType* aResult)
 {
+  MOZ_ASSERT(!aResult->mGraphicBuffer.get());
+  MOZ_ASSERT(aResult->mRef.mOwner == 0);
+  MOZ_ASSERT(aResult->mRef.mKey == -1);
+
   size_t nbytes;
   const char* data;
   int owner;
   int64_t index;
 
   if (!aMsg->ReadInt(aIter, &owner) ||
       !aMsg->ReadInt64(aIter, &index) ||
       !aMsg->ReadSize(aIter, &nbytes) ||
       !aMsg->ReadBytes(aIter, &data, nbytes)) {
     printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to read a message\n");
     return false;
   }
 
   size_t nfds = aMsg->num_fds();
   int fds[nfds];
-  bool sameProcess = (XRE_GetProcessType() == GeckoProcessType_Default);
+
   for (size_t n = 0; n < nfds; ++n) {
     FileDescriptor fd;
     if (!aMsg->ReadFileDescriptor(aIter, &fd)) {
       printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to read file descriptors\n");
       return false;
     }
     // If the GraphicBuffer was shared cross-process, SCM_RIGHTS does
     // the right thing and dup's the fd. If it's shared cross-thread,
@@ -139,17 +143,19 @@ ParamTraits<MagicGrallocBufferHandle>::R
     // But in shared cross-thread, dup fd is not necessary because we get
     // a pointer to the GraphicBuffer directly from SharedBufferManagerParent
     // and don't create a new GraphicBuffer around the fd.
     fds[n] = fd.fd;
   }
 
   aResult->mRef.mOwner = owner;
   aResult->mRef.mKey = index;
-  if (sameProcess) {
+  if (XRE_GetProcessType() == GeckoProcessType_Default) {
+    // If we are in chrome process, we can just get GraphicBuffer directly from
+    // SharedBufferManagerParent.
     aResult->mGraphicBuffer = SharedBufferManagerParent::GetGraphicBuffer(aResult->mRef);
   } else {
     // Deserialize GraphicBuffer
 #if ANDROID_VERSION >= 19
     sp<GraphicBuffer> buffer(new GraphicBuffer());
     const void* datap = (const void*)data;
     const int* fdsp = &fds[0];
     if (NO_ERROR != buffer->unflatten(datap, nbytes, fdsp, nfds)) {
@@ -157,17 +163,17 @@ ParamTraits<MagicGrallocBufferHandle>::R
     }
 #else
     sp<GraphicBuffer> buffer(new GraphicBuffer());
     Flattenable *flattenable = buffer.get();
     if (NO_ERROR != flattenable->unflatten(data, nbytes, fds, nfds)) {
       buffer = nullptr;
     }
 #endif
-    if(buffer.get() && !aResult->mGraphicBuffer.get()) {
+    if (buffer.get()) {
       aResult->mGraphicBuffer = buffer;
     }
   }
 
   if (!aResult->mGraphicBuffer.get()) {
     printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to get gralloc buffer\n");
     return false;
   }
--- a/gfx/layers/ipc/SharedBufferManagerChild.cpp
+++ b/gfx/layers/ipc/SharedBufferManagerChild.cpp
@@ -270,24 +270,27 @@ SharedBufferManagerChild::AllocGrallocBu
                                                 const uint32_t& aUsage,
                                                 mozilla::layers::MaybeMagicGrallocBufferHandle* aHandle)
 {
   // These are protected functions, we can just assert and ask the caller to test
   MOZ_ASSERT(aSize.width >= 0 && aSize.height >= 0);
 
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
   mozilla::layers::MaybeMagicGrallocBufferHandle handle;
-  SendAllocateGrallocBuffer(aSize, aFormat, aUsage, &handle);
+  if (!SendAllocateGrallocBuffer(aSize, aFormat, aUsage, &handle)) {
+    return false;
+  }
   if (handle.type() != mozilla::layers::MaybeMagicGrallocBufferHandle::TMagicGrallocBufferHandle) {
     return false;
   }
   *aHandle = handle.get_MagicGrallocBufferHandle().mRef;
 
   {
     MutexAutoLock lock(mBufferMutex);
+    MOZ_ASSERT(mBuffers.count(handle.get_MagicGrallocBufferHandle().mRef.mKey)==0);
     mBuffers[handle.get_MagicGrallocBufferHandle().mRef.mKey] = handle.get_MagicGrallocBufferHandle().mGraphicBuffer;
   }
   return true;
 #else
   NS_RUNTIMEABORT("No GrallocBuffer for you");
   return true;
 #endif
 }