Bug 1869933 - Avoid allocating external image ID until ready for sharing with compositor process. r=gfx-reviewers,lsalzman

There are some error paths where we could allocate an external image ID
for a SourceSurfaceSharedData before we are able to finish sharing it.
This patch makes it so that we defer allocating the ID until the last
possible moment. This will ensure they are always increasing in order.

The patch also adds in the necessary plumbing for notifications for
waking up threads blocked on a particular external image ID for a shared
surface inside the compositor process. This is less relevant now since
it should always been in the SharedSurfacesParent map, as recordings are
only created from the main thread, but may become more important as we
add recordings to DOM workers.

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

gfx/layers/ipc/CompositorManagerParent.cpp

@@ -270,6 +270,28 @@ CompositorManagerParent::AllocPCompositorBridgeParent(
   return nullptr;
 }
 
+/* static */ void CompositorManagerParent::AddSharedSurface(
+    const wr::ExternalImageId& aId, gfx::SourceSurfaceSharedData* aSurface) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+
+  StaticMonitorAutoLock lock(sMonitor);
+  if (NS_WARN_IF(!sInstance)) {
+    return;
+  }
+
+  if (NS_WARN_IF(!sInstance->OwnsExternalImageId(aId))) {
+    MOZ_ASSERT_UNREACHABLE("Wrong namespace?");
+    return;
+  }
+
+  SharedSurfacesParent::AddSameProcess(aId, aSurface);
+
+  uint32_t resourceId = static_cast<uint32_t>(wr::AsUint64(aId));
+  MOZ_RELEASE_ASSERT(sInstance->mLastSharedSurfaceResourceId < resourceId);
+  sInstance->mLastSharedSurfaceResourceId = resourceId;
+  sMonitor.NotifyAll();
+}
+
 mozilla::ipc::IPCResult CompositorManagerParent::RecvAddSharedSurface(
     const wr::ExternalImageId& aId, SurfaceDescriptorShared&& aDesc) {
   if (NS_WARN_IF(!OwnsExternalImageId(aId))) {

gfx/layers/ipc/CompositorManagerParent.h

@@ -18,6 +18,11 @@
 #include "nsTArray.h"  // for AutoTArray
 
 namespace mozilla {
+
+namespace gfx {
+class SourceSurfaceSharedData;
+}
+
 namespace layers {
 
 class CompositorBridgeParent;
@@ -44,6 +49,9 @@ class CompositorManagerParent final : public PCompositorManagerParent {
 
   static void WaitForSharedSurface(const wr::ExternalImageId& aId);
 
+  static void AddSharedSurface(const wr::ExternalImageId& aId,
+                               gfx::SourceSurfaceSharedData* aSurface);
+
   mozilla::ipc::IPCResult RecvAddSharedSurface(const wr::ExternalImageId& aId,
                                                SurfaceDescriptorShared&& aDesc);
   mozilla::ipc::IPCResult RecvRemoveSharedSurface(

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -5,7 +5,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SharedSurfacesChild.h"
-#include "SharedSurfacesParent.h"
 #include "CompositorManagerChild.h"
 #include "mozilla/layers/IpcResourceUpdateQueue.h"
 #include "mozilla/layers/SourceSurfaceSharedData.h"
@@ -13,6 +12,7 @@
 #include "mozilla/layers/RenderRootStateManager.h"
 #include "mozilla/layers/WebRenderLayerManager.h"
 #include "mozilla/layers/CompositorBridgeChild.h"
+#include "mozilla/layers/CompositorManagerParent.h"
 #include "mozilla/SchedulerGroup.h"
 #include "mozilla/StaticPrefs_image.h"
 #include "mozilla/PresShell.h"
@@ -58,10 +58,9 @@ void SharedSurfacesChild::ImageKeyData::MergeDirtyRect(
   }
 }
 
-SharedSurfacesChild::SharedUserData::SharedUserData(
-    const wr::ExternalImageId& aId)
+SharedSurfacesChild::SharedUserData::SharedUserData()
     : Runnable("SharedSurfacesChild::SharedUserData"),
-      mId(aId),
+      mId({}),
       mShared(false) {}
 
 SharedSurfacesChild::SharedUserData::~SharedUserData() {
@@ -187,17 +186,18 @@ nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface,
   SharedUserData* data =
       static_cast<SharedUserData*>(aSurface->GetUserData(&sSharedKey));
   if (!data) {
-    data =
-        MakeAndAddRef<SharedUserData>(manager->GetNextExternalImageId()).take();
+    data = MakeAndAddRef<SharedUserData>().take();
     aSurface->AddUserData(&sSharedKey, data, SharedUserData::Destroy);
-  } else if (!manager->OwnsExternalImageId(data->Id())) {
+  } else if (data->IsShared()) {
+    if (manager->OwnsExternalImageId(data->Id())) {
+      // It has already been shared with the GPU process.
+      *aUserData = data;
+      return NS_OK;
+    }
+
     // If the id isn't owned by us, that means the bridge was reinitialized, due
     // to the GPU process crashing. All previous mappings have been released.
-    data->SetId(manager->GetNextExternalImageId());
-  } else if (data->IsShared()) {
-    // It has already been shared with the GPU process.
-    *aUserData = data;
-    return NS_OK;
+    data->ClearShared();
   }
 
   // Ensure that the handle doesn't get released until after we have finished
@@ -211,8 +211,8 @@ nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface,
   // asking the parent instance to store a pointer to the same data, no need
   // to map the data into our memory space twice.
   if (manager->SameProcess()) {
-    SharedSurfacesParent::AddSameProcess(data->Id(), aSurface);
-    data->MarkShared();
+    data->MarkShared(manager->GetNextExternalImageId());
+    CompositorManagerParent::AddSharedSurface(data->Id(), aSurface);
     *aUserData = data;
     return NS_OK;
   }
@@ -244,7 +244,7 @@ nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface,
       format == SurfaceFormat::B8G8R8X8 || format == SurfaceFormat::B8G8R8A8,
       "bad format");
 
-  data->MarkShared();
+  data->MarkShared(manager->GetNextExternalImageId());
   manager->SendAddSharedSurface(
       data->Id(),
       SurfaceDescriptorShared(aSurface->GetSize(), aSurface->Stride(), format,

gfx/layers/ipc/SharedSurfacesChild.h

@@ -135,7 +135,7 @@ class SharedSurfacesChild {
 
   class SharedUserData final : public Runnable {
    public:
-    explicit SharedUserData(const wr::ExternalImageId& aId);
+    SharedUserData();
     virtual ~SharedUserData();
 
     SharedUserData(const SharedUserData& aOther) = delete;
@@ -150,16 +150,16 @@ class SharedSurfacesChild {
 
     const wr::ExternalImageId& Id() const { return mId; }
 
-    void SetId(const wr::ExternalImageId& aId) {
-      mId = aId;
+    void ClearShared() {
       mKeys.Clear();
       mShared = false;
     }
 
     bool IsShared() const { return mShared; }
 
-    void MarkShared() {
+    void MarkShared(const wr::ExternalImageId& aId) {
       MOZ_ASSERT(!mShared);
+      mId = aId;
       mShared = true;
     }
 

gfx/layers/ipc/SharedSurfacesParent.h

@@ -28,7 +28,7 @@ class DataSourceSurface;
 
 namespace layers {
 
-class SharedSurfacesChild;
+class CompositorManagerParent;
 class SharedSurfacesMemoryReport;
 
 class SharedSurfacesParent final {
@@ -69,7 +69,7 @@ class SharedSurfacesParent final {
   static bool AgeAndExpireOneGeneration();
 
  private:
-  friend class SharedSurfacesChild;
+  friend class CompositorManagerParent;
   friend class gfx::SourceSurfaceSharedDataWrapper;
 
   SharedSurfacesParent();