Bug 1691475 - Remove shared surfaces on the compositor thread. r=jrmuizel, a=pascalc

We add a shared surface on the main thread to the shared surfaces table
when in the compositor process because it uses a different wrapper
object if the shared surface is in a different process. This structure
was mirrored for the removal of a shared surface created in the
compositor process, however that created a race between the main thread
freeing the surface before the display list creation bound an image key
to the shared surface's external image ID. As such, this patch makes
removal always post to the compositor thread, whether in the compositor
process or not.

Differential Revision: https://phabricator.services.mozilla.com/D104426

Author: aosmond

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -416,20 +416,11 @@ void SharedSurfacesChild::Unshare(const wr::ExternalImageId& aId,
     return;
   }
 
-  if (manager->OtherPid() == base::GetCurrentProcId()) {
-    // We are in the combined UI/GPU process. Call directly to it to remove its
-    // wrapper surface to free the underlying buffer, but only if the external
-    // image ID is owned by the manager. It can be different if the surface was
-    // last shared with the GPU process, which crashed several times, and its
-    // job was moved into the parent process.
-    if (manager->OwnsExternalImageId(aId)) {
-      SharedSurfacesParent::RemoveSameProcess(aId);
-    }
-  } else if (manager->OwnsExternalImageId(aId)) {
-    // Only attempt to release current mappings in the GPU process. It is
-    // possible we had a surface that was previously shared, the GPU process
-    // crashed / was restarted, and then we freed the surface. In that case
-    // we know the mapping has already been freed.
+  if (manager->OwnsExternalImageId(aId)) {
+    // Only attempt to release current mappings in the compositor process. It is
+    // possible we had a surface that was previously shared, the compositor
+    // process crashed / was restarted, and then we freed the surface. In that
+    // case we know the mapping has already been freed.
     manager->SendRemoveSharedSurface(aId);
   }
 }

gfx/layers/ipc/SharedSurfacesParent.cpp

@@ -144,13 +144,6 @@ void SharedSurfacesParent::AddSameProcess(const wr::ExternalImageId& aId,
   sInstance->mSurfaces.Put(id, std::move(surface));
 }
 
-/* static */
-void SharedSurfacesParent::RemoveSameProcess(const wr::ExternalImageId& aId) {
-  MOZ_ASSERT(XRE_IsParentProcess());
-  MOZ_ASSERT(NS_IsMainThread());
-  Release(aId, /* aForCreator */ true);
-}
-
 /* static */
 void SharedSurfacesParent::DestroyProcess(base::ProcessId aPid) {
   StaticMutexAutoLock lock(sMutex);

gfx/layers/ipc/SharedSurfacesParent.h

@@ -67,7 +67,6 @@ class SharedSurfacesParent final {
 
   static void AddSameProcess(const wr::ExternalImageId& aId,
                              gfx::SourceSurfaceSharedData* aSurface);
-  static void RemoveSameProcess(const wr::ExternalImageId& aId);
 
   static StaticMutex sMutex;