Bug 1034294 - Fix SharedBufferManagerParent r=nical
authorSotaro Ikeda <sikeda@mozilla.com>
Tue, 08 Jul 2014 06:46:17 -0700
changeset 192856 f74675fb5620cf89141f02f926ad451bb8f9fa4b
parent 192855 ed8b3cc1123b601bd44bee8eec36e693bd32f50c
child 192857 373ec3f226af4cf6329b739b1006e469fa5419a2
push id7663
push userkwierso@gmail.com
push dateWed, 09 Jul 2014 03:08:08 +0000
treeherderfx-team@48de6f4f82af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1034294
milestone33.0a1
Bug 1034294 - Fix SharedBufferManagerParent r=nical
dom/ipc/ContentParent.cpp
gfx/layers/ipc/SharedBufferManagerChild.cpp
gfx/layers/ipc/SharedBufferManagerParent.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1899,21 +1899,21 @@ ContentParent::InitInternal(ProcessPrior
                 opened = PImageBridge::Open(this);
                 MOZ_ASSERT(opened);
             }
 #else
             opened = PImageBridge::Open(this);
             MOZ_ASSERT(opened);
 #endif
         }
+#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
@@ -93,23 +93,29 @@ void InitGralloc() {
 
 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) {
@@ -119,16 +125,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)
@@ -137,29 +144,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)
 {
@@ -182,24 +185,29 @@ 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);
 
   {
     MutexAutoLock lock(mBuffersMutex);
-    mBuffers[sBufferKey] = outgoingBuffer;
+    mBuffers[bufferKey] = outgoingBuffer;
   }
 #endif
   return true;
 }
 
 bool SharedBufferManagerParent::RecvDropGrallocBuffer(const mozilla::layers::MaybeMagicGrallocBufferHandle& handle)
 {
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
@@ -222,43 +230,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, "No such buffer");
   mBuffers.erase(key);
   SendDropGrallocBuffer(handle);
 #endif
 }
 
@@ -276,19 +287,19 @@ SharedBufferManagerParent* SharedBufferM
 
 #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
 android::sp<android::GraphicBuffer>
 SharedBufferManagerParent::GetGraphicBuffer(int64_t key)
 {
   MutexAutoLock lock(mBuffersMutex);
   if (mBuffers.count(key) == 1) {
     return mBuffers[key];
-  }
-  else {
+  } 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);