Bug 1829026 - Remove waiting texture ids if nothing uses them. r=aosmond

We can accumulate a bunch of waiting texture ids that never actually get
composited. By the time we go to lock a successive canvas frame, if nothing
has queried a surface descriptor for the preceding frame, it is safe to assume
that the canvas frame will never be rendered, since any record of its texture
id has been erased. In this case, we want to remove the resources allocated
to that texture immediately from the host side rather than let it sit around
indefinitely, in case no composition ever occurs.

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

gfx/layers/RecordedCanvasEventImpl.h

@@ -140,11 +140,13 @@ class RecordedTextureLock final
     : public RecordedEventDerived<RecordedTextureLock> {
  public:
   RecordedTextureLock(int64_t aTextureId, const OpenMode aMode,
-                      RemoteTextureId aId)
+                      RemoteTextureId aId,
+                      RemoteTextureId aObsoleteId = RemoteTextureId())
       : RecordedEventDerived(TEXTURE_LOCK),
         mTextureId(aTextureId),
         mMode(aMode),
-        mLastRemoteTextureId(aId) {}
+        mLastRemoteTextureId(aId),
+        mObsoleteRemoteTextureId(aObsoleteId) {}
 
   template <class S>
   MOZ_IMPLICIT RecordedTextureLock(S& aStream);
@@ -160,11 +162,13 @@ class RecordedTextureLock final
   int64_t mTextureId;
   OpenMode mMode;
   RemoteTextureId mLastRemoteTextureId;
+  RemoteTextureId mObsoleteRemoteTextureId;
 };
 
 inline bool RecordedTextureLock::PlayCanvasEvent(
     CanvasTranslator* aTranslator) const {
-  if (!aTranslator->LockTexture(mTextureId, mMode, mLastRemoteTextureId)) {
+  if (!aTranslator->LockTexture(mTextureId, mMode, mLastRemoteTextureId,
+                                mObsoleteRemoteTextureId)) {
     return false;
   }
   return true;
@@ -175,6 +179,7 @@ void RecordedTextureLock::Record(S& aStream) const {
   WriteElement(aStream, mTextureId);
   WriteElement(aStream, mMode);
   WriteElement(aStream, mLastRemoteTextureId.mId);
+  WriteElement(aStream, mObsoleteRemoteTextureId.mId);
 }
 
 template <class S>
@@ -184,6 +189,7 @@ RecordedTextureLock::RecordedTextureLock(S& aStream)
   ReadElementConstrained(aStream, mMode, OpenMode::OPEN_NONE,
                          OpenMode::OPEN_READ_WRITE_ASYNC);
   ReadElement(aStream, mLastRemoteTextureId.mId);
+  ReadElement(aStream, mObsoleteRemoteTextureId.mId);
 }
 
 class RecordedTextureUnlock final

gfx/layers/RemoteTextureMap.cpp

@@ -346,6 +346,29 @@ void RemoteTextureMap::PushTexture(
   }
 }
 
+bool RemoteTextureMap::RemoveTexture(const RemoteTextureId aTextureId,
+                                     const RemoteTextureOwnerId aOwnerId,
+                                     const base::ProcessId aForPid) {
+  MonitorAutoLock lock(mMonitor);
+
+  auto* owner = GetTextureOwner(lock, aOwnerId, aForPid);
+  if (!owner) {
+    return false;
+  }
+
+  for (auto it = owner->mWaitingTextureDataHolders.begin();
+       it != owner->mWaitingTextureDataHolders.end(); it++) {
+    auto& data = *it;
+    if (data->mTextureId == aTextureId) {
+      owner->mReleasingTextureDataHolders.push_back(std::move(data));
+      owner->mWaitingTextureDataHolders.erase(it);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 void RemoteTextureMap::GetLatestBufferSnapshot(
     const RemoteTextureOwnerId aOwnerId, const base::ProcessId aForPid,
     const mozilla::ipc::Shmem& aDestShmem, const gfx::IntSize& aSize) {

gfx/layers/RemoteTextureMap.h

@@ -184,6 +184,11 @@ class RemoteTextureMap {
                    RefPtr<TextureHost>& aTextureHost,
                    UniquePtr<SharedResourceWrapper>&& aResourceWrapper);
 
+  // Remove waiting texture that will not be used.
+  bool RemoveTexture(const RemoteTextureId aTextureId,
+                     const RemoteTextureOwnerId aOwnerId,
+                     const base::ProcessId aForPid);
+
   void GetLatestBufferSnapshot(const RemoteTextureOwnerId aOwnerId,
                                const base::ProcessId aForPid,
                                const mozilla::ipc::Shmem& aDestShmem,

gfx/layers/client/TextureRecorded.cpp

@@ -51,8 +51,18 @@ bool RecordedTextureData::Lock(OpenMode aMode) {
     return false;
   }
 
+  RemoteTextureId obsoleteRemoteTextureId;
   if (mRemoteTextureOwnerId.IsValid()) {
+    // By the time we allocate a new remote texture id, the previous texture id
+    // should have been used. Since we're overwriting its id, if it hasn't been
+    // used yet, then it is safe to preemptively remove it since nothing can
+    // actually composite it. This prevents accumulation of a series of canvas
+    // frames that never get shown.
+    if (!mUsedRemoteTexture) {
+      obsoleteRemoteTextureId = mLastRemoteTextureId;
+    }
     mLastRemoteTextureId = RemoteTextureId::GetNext();
+    mUsedRemoteTexture = false;
   }
 
   if (!mDT) {
@@ -70,8 +80,8 @@ bool RecordedTextureData::Lock(OpenMode aMode) {
     return true;
   }
 
-  mCanvasChild->RecordEvent(
-      RecordedTextureLock(mTextureId, aMode, mLastRemoteTextureId));
+  mCanvasChild->RecordEvent(RecordedTextureLock(
+      mTextureId, aMode, mLastRemoteTextureId, obsoleteRemoteTextureId));
   if (aMode & OpenMode::OPEN_WRITE) {
     mCanvasChild->OnTextureWriteLock();
   }
@@ -143,6 +153,8 @@ bool RecordedTextureData::Serialize(SurfaceDescriptor& aDescriptor) {
   if (mRemoteTextureOwnerId.IsValid()) {
     aDescriptor = SurfaceDescriptorRemoteTexture(mLastRemoteTextureId,
                                                  mRemoteTextureOwnerId);
+    // If something is querying the id, assume it is going to be composited.
+    mUsedRemoteTexture = true;
   } else {
     aDescriptor = SurfaceDescriptorRecorded(mTextureId);
   }

gfx/layers/client/TextureRecorded.h

@@ -62,6 +62,7 @@ class RecordedTextureData final : public TextureData {
   OpenMode mLockedMode;
   layers::RemoteTextureId mLastRemoteTextureId;
   layers::RemoteTextureOwnerId mRemoteTextureOwnerId;
+  bool mUsedRemoteTexture = false;
 };
 
 }  // namespace layers

gfx/layers/ipc/CanvasTranslator.cpp

@@ -806,11 +806,16 @@ TextureData* CanvasTranslator::LookupTextureData(int64_t aTextureId) {
 }
 
 bool CanvasTranslator::LockTexture(int64_t aTextureId, OpenMode aMode,
-                                   RemoteTextureId aId) {
+                                   RemoteTextureId aId,
+                                   RemoteTextureId aObsoleteId) {
   auto result = mTextureInfo.find(aTextureId);
   if (result == mTextureInfo.end()) {
     return false;
   }
+  RemoteTextureOwnerId ownerId = result->second.mRemoteTextureOwnerId;
+  if (ownerId.IsValid() && aObsoleteId.IsValid()) {
+    RemoteTextureMap::Get()->RemoveTexture(aObsoleteId, ownerId, mOtherPid);
+  }
   if (result->second.mDrawTarget &&
       result->second.mDrawTarget->GetBackendType() == gfx::BackendType::WEBGL) {
     gfx::DrawTargetWebgl* webgl =

gfx/layers/ipc/CanvasTranslator.h

@@ -181,7 +181,8 @@ class CanvasTranslator final : public gfx::InlineTranslator,
    */
   void RemoveTexture(int64_t aTextureId);
 
-  bool LockTexture(int64_t aTextureId, OpenMode aMode, RemoteTextureId aId);
+  bool LockTexture(int64_t aTextureId, OpenMode aMode, RemoteTextureId aId,
+                   RemoteTextureId aObsoleteId = RemoteTextureId());
   bool UnlockTexture(int64_t aTextureId, RemoteTextureId aId);
 
   bool PushRemoteTexture(TextureData* aData, RemoteTextureId aId,