Bug 1034294 - Fix SharedBufferManagerParent. r=nical, a=2.0+
authorSotaro Ikeda <sikeda@mozilla.com>
Mon, 07 Jul 2014 07:27:00 -0400
changeset 207761 751bfe1f5fb30b3d2ebccd862a01db2daea8ae74
parent 207760 ceb34c8aeb95d09d8bf9cccdc247f335955a2c20
child 207762 9963585930ba234e0b98ce0871cd5ed2b6b07baa
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical, 2
bugs1034294
milestone32.0a2
Bug 1034294 - Fix SharedBufferManagerParent. r=nical, a=2.0+
dom/ipc/ContentParent.cpp
gfx/layers/ipc/SharedBufferManagerChild.cpp
gfx/layers/ipc/SharedBufferManagerParent.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1715,21 +1715,21 @@ ContentParent::InitInternal(ProcessPrior
             DebugOnly<bool> opened = PCompositor::Open(this);
             MOZ_ASSERT(opened);
 
             if (gfxPrefs::AsyncVideoOOPEnabled()) {
                 opened = PImageBridge::Open(this);
                 MOZ_ASSERT(opened);
             }
         }
+#ifdef MOZ_WIDGET_GONK
+        DebugOnly<bool> opened = PSharedBufferManager::Open(this);
+        MOZ_ASSERT(opened);
+#endif
     }
-#ifdef MOZ_WIDGET_GONK
-    DebugOnly<bool> opened = PSharedBufferManager::Open(this);
-    MOZ_ASSERT(opened);
-#endif
 
     if (aSendRegisteredChrome) {
         nsCOMPtr<nsIChromeRegistry> registrySvc = nsChromeRegistry::GetService();
         nsChromeRegistryChrome* chromeRegistry =
             static_cast<nsChromeRegistryChrome*>(registrySvc.get());
         chromeRegistry->SendRegisteredChrome(this);
     }
 
--- a/gfx/layers/ipc/SharedBufferManagerChild.cpp
+++ b/gfx/layers/ipc/SharedBufferManagerChild.cpp
@@ -32,17 +32,16 @@ SharedBufferManagerChild* SharedBufferMa
 SharedBufferManagerParent* SharedBufferManagerChild::sSharedBufferManagerParentSingleton = nullptr;
 base::Thread* SharedBufferManagerChild::sSharedBufferManagerChildThread = nullptr;
 
 SharedBufferManagerChild::SharedBufferManagerChild()
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
   : mBufferMutex("BufferMonitor")
 #endif
 {
-
 }
 
 static bool
 InSharedBufferManagerChildThread()
 {
   return SharedBufferManagerChild::sSharedBufferManagerChildThread->thread_id() == PlatformThread::CurrentId();
 }
 
@@ -149,18 +148,19 @@ SharedBufferManagerChild::ShutDown()
     sSharedBufferManagerChildThread = nullptr;
   }
 }
 
 bool
 SharedBufferManagerChild::StartUpOnThread(base::Thread* aThread)
 {
   NS_ABORT_IF_FALSE(aThread, "SharedBufferManager needs a thread.");
-  if (sSharedBufferManagerChildSingleton != nullptr)
+  if (sSharedBufferManagerChildSingleton != nullptr) {
     return false;
+  }
 
   sSharedBufferManagerChildThread = aThread;
   if (!aThread->IsRunning()) {
     aThread->Start();
   }
   sSharedBufferManagerChildSingleton = new SharedBufferManagerChild();
   char thrname[128];
   base::snprintf(thrname, 128, "BufMgrParent#%d", base::Process::Current().pid());
@@ -332,16 +332,18 @@ bool SharedBufferManagerChild::RecvDropG
   return true;
 }
 
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
 android::sp<android::GraphicBuffer>
 SharedBufferManagerChild::GetGraphicBuffer(int64_t key)
 {
   MutexAutoLock lock(mBufferMutex);
-  if (mBuffers.count(key) == 0)
+  if (mBuffers.count(key) == 0) {
+    printf_stderr("SharedBufferManagerChild::GetGraphicBuffer -- invalid key");
     return nullptr;
+  }
   return mBuffers[key];
 }
 #endif
 
 } /* namespace layers */
 } /* namespace mozilla */
--- a/gfx/layers/ipc/SharedBufferManagerParent.cpp
+++ b/gfx/layers/ipc/SharedBufferManagerParent.cpp
@@ -75,23 +75,29 @@ uint64_t SharedBufferManagerParent::sBuf
 
 SharedBufferManagerParent::SharedBufferManagerParent(Transport* aTransport, base::ProcessId aOwner, base::Thread* aThread)
   : mTransport(aTransport)
   , mThread(aThread)
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
   , mBuffersMutex("BuffersMonitor")
 #endif
 {
-  if (!sManagerMonitor)
+  if (!sManagerMonitor) {
     sManagerMonitor = new Monitor("Manager Monitor");
+  }
 
   MonitorAutoLock lock(*sManagerMonitor.get());
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread");
-  if (!aThread->IsRunning())
+  if (!aThread->IsRunning()) {
     aThread->Start();
+  }
+
+  if (sManagers.count(aOwner) != 0) {
+    printf_stderr("SharedBufferManagerParent already exists.");
+  }
   mOwner = aOwner;
   sManagers[aOwner] = this;
 }
 
 SharedBufferManagerParent::~SharedBufferManagerParent()
 {
   MonitorAutoLock lock(*sManagerMonitor.get());
   if (mTransport) {
@@ -101,16 +107,17 @@ SharedBufferManagerParent::~SharedBuffer
   sManagers.erase(mOwner);
   delete mThread;
 }
 
 void
 SharedBufferManagerParent::ActorDestroy(ActorDestroyReason aWhy)
 {
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
+  MutexAutoLock lock(mBuffersMutex);
   mBuffers.clear();
 #endif
 }
 
 static void
 ConnectSharedBufferManagerInParentProcess(SharedBufferManagerParent* aManager,
                                           Transport* aTransport,
                                           base::ProcessHandle aOtherProcess)
@@ -119,29 +126,25 @@ ConnectSharedBufferManagerInParentProces
 }
 
 PSharedBufferManagerParent* SharedBufferManagerParent::Create(Transport* aTransport, ProcessId aOtherProcess)
 {
   ProcessHandle processHandle;
   if (!base::OpenProcessHandle(aOtherProcess, &processHandle)) {
     return nullptr;
   }
+  base::Thread* thread = nullptr;
+  char thrname[128];
+  base::snprintf(thrname, 128, "BufMgrParent#%d", aOtherProcess);
+  thread = new base::Thread(thrname);
 
-  base::Thread* thread = nullptr;
-  if (sManagers.count(aOtherProcess) == 1) {
-    thread = sManagers[aOtherProcess]->mThread;
+  SharedBufferManagerParent* manager = new SharedBufferManagerParent(aTransport, aOtherProcess, thread);
+  if (!thread->IsRunning()) {
+    thread->Start();
   }
-  else {
-    char thrname[128];
-    base::snprintf(thrname, 128, "BufMgrParent#%d", aOtherProcess);
-    thread = new base::Thread(thrname);
-  }
-  SharedBufferManagerParent* manager = new SharedBufferManagerParent(aTransport, aOtherProcess, thread);
-  if (!thread->IsRunning())
-    thread->Start();
   thread->message_loop()->PostTask(FROM_HERE,
                                    NewRunnableFunction(ConnectSharedBufferManagerInParentProcess,
                                                        manager, aTransport, processHandle));
   return manager;
 }
 
 bool SharedBufferManagerParent::RecvAllocateGrallocBuffer(const IntSize& aSize, const uint32_t& aFormat, const uint32_t& aUsage, mozilla::layers::MaybeMagicGrallocBufferHandle* aHandle)
 {
@@ -164,31 +167,36 @@ bool SharedBufferManagerParent::RecvAllo
   }
 
   sp<GraphicBuffer> outgoingBuffer = new GraphicBuffer(aSize.width, aSize.height, aFormat, aUsage);
   if (!outgoingBuffer.get() || outgoingBuffer->initCheck() != NO_ERROR) {
     printf_stderr("SharedBufferManagerParent::RecvAllocateGrallocBuffer -- gralloc buffer allocation failed");
     return true;
   }
 
+  int64_t bufferKey;
+  {
+    MonitorAutoLock lock(*sManagerMonitor.get());
+    bufferKey = ++sBufferKey; 
+  }
   GrallocBufferRef ref;
   ref.mOwner = mOwner;
-  ref.mKey = ++sBufferKey;
+  ref.mKey = bufferKey;
   *aHandle = MagicGrallocBufferHandle(outgoingBuffer, ref);
 
   int bpp = 0;
   bpp = android::bytesPerPixel(outgoingBuffer->getPixelFormat());
   if (bpp > 0)
     GrallocReporter::sAmount += outgoingBuffer->getStride() * outgoingBuffer->getHeight() * bpp;
   else // Specical case for BSP specific formats(mainly YUV formats, count it as normal YUV buffer)
     GrallocReporter::sAmount += outgoingBuffer->getStride() * outgoingBuffer->getHeight() * 3 / 2;
 
   {
     MutexAutoLock lock(mBuffersMutex);
-    mBuffers[sBufferKey] = outgoingBuffer;
+    mBuffers[bufferKey] = outgoingBuffer;
   }
 #endif
   return true;
 }
 
 bool SharedBufferManagerParent::RecvDropGrallocBuffer(const mozilla::layers::MaybeMagicGrallocBufferHandle& handle)
 {
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
@@ -218,43 +226,46 @@ bool SharedBufferManagerParent::RecvDrop
 
 void SharedBufferManagerParent::DropGrallocBufferSync(SharedBufferManagerParent* mgr, mozilla::layers::SurfaceDescriptor aDesc)
 {
   mgr->DropGrallocBufferImpl(aDesc);
 }
 
 void SharedBufferManagerParent::DropGrallocBuffer(mozilla::layers::SurfaceDescriptor aDesc)
 {
-  if (aDesc.type() != SurfaceDescriptor::TNewSurfaceDescriptorGralloc)
+  if (aDesc.type() != SurfaceDescriptor::TNewSurfaceDescriptorGralloc) {
     return;
+  }
 
   if (PlatformThread::CurrentId() == mThread->thread_id()) {
     DropGrallocBufferImpl(aDesc);
   } else {
     mThread->message_loop()->PostTask(FROM_HERE,
                                       NewRunnableFunction(&DropGrallocBufferSync, this, aDesc));
   }
   return;
 }
 
 void SharedBufferManagerParent::DropGrallocBufferImpl(mozilla::layers::SurfaceDescriptor aDesc)
 {
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
   MutexAutoLock lock(mBuffersMutex);
   int64_t key = -1;
   MaybeMagicGrallocBufferHandle handle;
-  if (aDesc.type() == SurfaceDescriptor::TNewSurfaceDescriptorGralloc)
+  if (aDesc.type() == SurfaceDescriptor::TNewSurfaceDescriptorGralloc) {
     handle = aDesc.get_NewSurfaceDescriptorGralloc().buffer();
-  else
+  } else {
     return;
+  }
 
-  if (handle.type() == MaybeMagicGrallocBufferHandle::TGrallocBufferRef)
+  if (handle.type() == MaybeMagicGrallocBufferHandle::TGrallocBufferRef) {
     key = handle.get_GrallocBufferRef().mKey;
-  else if (handle.type() == MaybeMagicGrallocBufferHandle::TMagicGrallocBufferHandle)
+  } else if (handle.type() == MaybeMagicGrallocBufferHandle::TMagicGrallocBufferHandle) {
     key = handle.get_MagicGrallocBufferHandle().mRef.mKey;
+  }
 
   NS_ASSERTION(key != -1, "Invalid buffer key");
   NS_ASSERTION(mBuffers.count(key) == 1, "How can you drop others buffer");
   mBuffers.erase(key);
   SendDropGrallocBuffer(handle);
 #endif
 }
 
@@ -270,18 +281,23 @@ SharedBufferManagerParent* SharedBufferM
   return sManagers[id];
 }
 
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
 android::sp<android::GraphicBuffer>
 SharedBufferManagerParent::GetGraphicBuffer(int64_t key)
 {
   MutexAutoLock lock(mBuffersMutex);
-  NS_ASSERTION(mBuffers.count(key) == 1, "No such buffer, or the buffer is belongs to other session");
-  return mBuffers[key];
+  if (mBuffers.count(key) == 1) {
+    return mBuffers[key];
+  } else {
+    // The buffer can be dropped, or invalid
+    printf_stderr("SharedBufferManagerParent::GetGraphicBuffer -- invalid key");
+    return nullptr;
+  }
 }
 
 android::sp<android::GraphicBuffer>
 SharedBufferManagerParent::GetGraphicBuffer(GrallocBufferRef aRef)
 {
   return GetInstance(aRef.mOwner)->GetGraphicBuffer(aRef.mKey);
 }
 #endif