Bug 1877429 - Prevent offscreen canvas2d updates from racing with compositing. r=gfx-reviewers,lsalzman

When OffscreenCanvas::CommitFrameToCompositor uses the non-remote
texture canvas path with Skia, it uses ImageBridgeChild for compositing.
When ImageContainer::SetCurrentImages is called, there was an
intermediate state where the relevant textures were not yet marked as
read only for the compositor's consumption, because the event to do so
was dispatched asynchronously to the ImageBridgeChild thread. If the
owning thread of the canvas (main or DOM worker) ran immediately after
CommitFrameToCompositor, then we could run into texture reuse since
nothing marked the texture yet as being used for compositing. This had
the end result of sometimes displaying back buffer textures currently
being used for drawing on the display pipeline.

This patch makes it so that we mark OffscreenCanvas textures as read
only for the compositor before dispatching, and releasing the lock
either when we swap the images in the ImageContainer (winning the race
with ImageBridgeChild), or after the compositor has finished with it
(losing the race, if any, with ImageBridgeChild).

Additionally, to handle better the case where we run out of buffers, we
need to implement ImageBridgeChild::SyncWithCompositor, to be analogous
to how WebRenderBridgeChild::SyncWithCompositor works. We achieve this
by calling from ImageBridgeChild back into the appropriate
WebRenderBridgeChild based on the window ID associated with the canvas,

It also adds a new pref, gfx.offscreencanvas.shared-provider, which
allows one to switch between PersistentBufferProviderShared and Basic.
The latter of which is used if we fallback from using shared buffers if
it takes too long to get the shared buffers back from the compositor.

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

dom/canvas/CanvasRenderingContext2D.cpp

@@ -1492,8 +1492,7 @@ bool CanvasRenderingContext2D::BorrowTarget(const IntRect& aPersistedRect,
 bool CanvasRenderingContext2D::EnsureTarget(const gfx::Rect* aCoveredRect,
                                             bool aWillClear) {
   if (AlreadyShutDown()) {
-    gfxCriticalErrorOnce()
+    gfxCriticalNoteOnce << "Attempt to render into a Canvas2d after shutdown.";
-        << "Attempt to render into a Canvas2d after shutdown.";
     SetErrorState();
     return false;
   }
@@ -1726,6 +1725,10 @@ bool CanvasRenderingContext2D::TrySharedTarget(
         GetSize(), GetSurfaceFormat(),
         !mAllowAcceleration || GetEffectiveWillReadFrequently());
   } else if (mOffscreenCanvas) {
+    if (!StaticPrefs::gfx_offscreencanvas_shared_provider()) {
+      return false;
+    }
+
     RefPtr<layers::ImageBridgeChild> imageBridge =
         layers::ImageBridgeChild::GetSingleton();
     if (NS_WARN_IF(!imageBridge)) {
@@ -1734,7 +1737,8 @@ bool CanvasRenderingContext2D::TrySharedTarget(
 
     aOutProvider = layers::PersistentBufferProviderShared::Create(
         GetSize(), GetSurfaceFormat(), imageBridge,
-        !mAllowAcceleration || GetEffectiveWillReadFrequently());
+        !mAllowAcceleration || GetEffectiveWillReadFrequently(),
+        mOffscreenCanvas->GetWindowID());
   }
 
   if (!aOutProvider) {

dom/canvas/OffscreenCanvas.cpp

@@ -281,6 +281,19 @@ OffscreenCanvas::CreateContext(CanvasContextType aContextType) {
   return ret.forget();
 }
 
+Maybe<uint64_t> OffscreenCanvas::GetWindowID() {
+  if (NS_IsMainThread()) {
+    if (nsIGlobalObject* global = GetOwnerGlobal()) {
+      if (auto* window = global->GetAsInnerWindow()) {
+        return Some(window->WindowID());
+      }
+    }
+  } else if (auto* workerPrivate = GetCurrentThreadWorkerPrivate()) {
+    return Some(workerPrivate->WindowID());
+  }
+  return Nothing();
+}
+
 void OffscreenCanvas::UpdateDisplayData(
     const OffscreenCanvasDisplayData& aData) {
   if (!mDisplay) {

dom/canvas/OffscreenCanvas.h

@@ -111,6 +111,8 @@ class OffscreenCanvas final : public DOMEventTargetHelper,
                                    JS::Handle<JS::Value> aParams,
                                    ErrorResult& aRv);
 
+  Maybe<uint64_t> GetWindowID();
+
   nsICanvasRenderingContextInternal* GetContext() const {
     return mCurrentContext;
   }

gfx/layers/ImageContainer.cpp

@@ -319,6 +319,9 @@ void ImageContainer::SetCurrentImageInternal(
     if (aImages[0].mProducerID != mCurrentProducerID) {
       mCurrentProducerID = aImages[0].mProducerID;
     }
+    for (auto& img : mCurrentImages) {
+      img.mImage->OnAbandonForwardToHost();
+    }
   }
 
   nsTArray<OwningImage> newImages;
@@ -347,6 +350,7 @@ void ImageContainer::SetCurrentImageInternal(
         break;
       }
     }
+    img->mImage->OnPrepareForwardToHost();
   }
 
   mCurrentImages = std::move(newImages);

gfx/layers/ImageContainer.h

@@ -129,6 +129,9 @@ class Image {
   bool IsDRM() const { return mIsDRM; }
   virtual void SetIsDRM(bool aIsDRM) { mIsDRM = aIsDRM; }
 
+  virtual void OnPrepareForwardToHost() {}
+  virtual void OnAbandonForwardToHost() {}
+
   virtual already_AddRefed<gfx::SourceSurface> GetAsSourceSurface() = 0;
 
   enum class BuildSdbFlags : uint8_t {

gfx/layers/PersistentBufferProvider.cpp

@@ -266,7 +266,8 @@ already_AddRefed<PersistentBufferProviderShared>
 PersistentBufferProviderShared::Create(gfx::IntSize aSize,
                                        gfx::SurfaceFormat aFormat,
                                        KnowsCompositor* aKnowsCompositor,
-                                       bool aWillReadFrequently) {
+                                       bool aWillReadFrequently,
+                                       const Maybe<uint64_t>& aWindowID) {
   if (!aKnowsCompositor || !aKnowsCompositor->GetTextureForwarder() ||
       !aKnowsCompositor->GetTextureForwarder()->IPCOpen()) {
     return nullptr;
@@ -290,19 +291,21 @@ PersistentBufferProviderShared::Create(gfx::IntSize aSize,
 
   RefPtr<PersistentBufferProviderShared> provider =
       new PersistentBufferProviderShared(aSize, aFormat, aKnowsCompositor,
-                                         texture, aWillReadFrequently);
+                                         texture, aWillReadFrequently,
+                                         aWindowID);
   return provider.forget();
 }
 
 PersistentBufferProviderShared::PersistentBufferProviderShared(
     gfx::IntSize aSize, gfx::SurfaceFormat aFormat,
     KnowsCompositor* aKnowsCompositor, RefPtr<TextureClient>& aTexture,
-    bool aWillReadFrequently)
+    bool aWillReadFrequently, const Maybe<uint64_t>& aWindowID)
     : mSize(aSize),
       mFormat(aFormat),
       mKnowsCompositor(aKnowsCompositor),
       mFront(Nothing()),
-      mWillReadFrequently(aWillReadFrequently) {
+      mWillReadFrequently(aWillReadFrequently),
+      mWindowID(aWindowID) {
   MOZ_ASSERT(aKnowsCompositor);
   if (mTextures.append(aTexture)) {
     mBack = Some<uint32_t>(0);
@@ -481,7 +484,7 @@ PersistentBufferProviderShared::BorrowDrawTarget(
       // especially when switching between layer managers (during tab-switch).
       // To make sure we don't get too far ahead of the compositor, we send a
       // sync ping to the compositor thread...
-      mKnowsCompositor->SyncWithCompositor();
+      mKnowsCompositor->SyncWithCompositor(mWindowID);
       // ...and try again.
       for (uint32_t i = 0; i < mTextures.length(); ++i) {
         if (!mTextures[i]->IsReadLocked()) {

gfx/layers/PersistentBufferProvider.h

@@ -206,7 +206,8 @@ class PersistentBufferProviderShared : public PersistentBufferProvider,
 
   static already_AddRefed<PersistentBufferProviderShared> Create(
       gfx::IntSize aSize, gfx::SurfaceFormat aFormat,
-      KnowsCompositor* aKnowsCompositor, bool aWillReadFrequently = false);
+      KnowsCompositor* aKnowsCompositor, bool aWillReadFrequently = false,
+      const Maybe<uint64_t>& aWindowID = Nothing());
 
   bool IsShared() const override { return true; }
 
@@ -237,7 +238,8 @@ class PersistentBufferProviderShared : public PersistentBufferProvider,
   PersistentBufferProviderShared(gfx::IntSize aSize, gfx::SurfaceFormat aFormat,
                                  KnowsCompositor* aKnowsCompositor,
                                  RefPtr<TextureClient>& aTexture,
-                                 bool aWillReadFrequently);
+                                 bool aWillReadFrequently,
+                                 const Maybe<uint64_t>& aWindowID);
 
   ~PersistentBufferProviderShared();
 
@@ -262,6 +264,8 @@ class PersistentBufferProviderShared : public PersistentBufferProvider,
   Maybe<uint32_t> mFront;
   // Whether to avoid acceleration.
   bool mWillReadFrequently = false;
+  // Owning window ID of the buffer provider.
+  Maybe<uint64_t> mWindowID;
 
   RefPtr<gfx::DrawTarget> mDrawTarget;
   RefPtr<gfx::SourceSurface> mSnapshot;

gfx/layers/TextureWrapperImage.cpp

@@ -45,5 +45,13 @@ TextureClient* TextureWrapperImage::GetTextureClient(
   return mTextureClient;
 }
 
+void TextureWrapperImage::OnPrepareForwardToHost() {
+  mTextureClient->OnPrepareForwardToHost();
+}
+
+void TextureWrapperImage::OnAbandonForwardToHost() {
+  mTextureClient->OnAbandonForwardToHost();
+}
+
 }  // namespace layers
 }  // namespace mozilla

gfx/layers/TextureWrapperImage.h

@@ -25,6 +25,8 @@ class TextureWrapperImage final : public Image {
   gfx::IntRect GetPictureRect() const override;
   already_AddRefed<gfx::SourceSurface> GetAsSourceSurface() override;
   TextureClient* GetTextureClient(KnowsCompositor* aKnowsCompositor) override;
+  void OnPrepareForwardToHost() override;
+  void OnAbandonForwardToHost() override;
 
  private:
   gfx::IntRect mPictureRect;

gfx/layers/client/TextureClient.cpp

@@ -839,30 +839,77 @@ void TextureClient::EnableReadLock() {
   }
 }
 
+void TextureClient::OnPrepareForwardToHost() {
+  if (!ShouldReadLock()) {
+    return;
+  }
+
+  MutexAutoLock lock(mMutex);
+  if (NS_WARN_IF(!mReadLock)) {
+    MOZ_ASSERT(!mAllocator->IPCOpen(), "Should have created readlock already!");
+    MOZ_ASSERT(!mIsPendingForwardReadLocked);
+    return;
+  }
+
+  if (mIsPendingForwardReadLocked) {
+    return;
+  }
+
+  mReadLock->ReadLock();
+  mIsPendingForwardReadLocked = true;
+}
+
+void TextureClient::OnAbandonForwardToHost() {
+  if (!ShouldReadLock()) {
+    return;
+  }
+
+  MutexAutoLock lock(mMutex);
+  if (!mReadLock || !mIsPendingForwardReadLocked) {
+    return;
+  }
+
+  mReadLock->ReadUnlock();
+  mIsPendingForwardReadLocked = false;
+}
+
 bool TextureClient::OnForwardedToHost() {
   if (mData) {
     mData->OnForwardedToHost();
   }
 
-  if (!ShouldReadLock() || !mUpdated) {
+  if (!ShouldReadLock()) {
     return false;
   }
 
-  {
+  MutexAutoLock lock(mMutex);
-    MutexAutoLock lock(mMutex);
+  EnsureHasReadLock();
-    EnsureHasReadLock();
 
-    if (NS_WARN_IF(!mReadLock)) {
+  if (NS_WARN_IF(!mReadLock)) {
-      MOZ_ASSERT(!mAllocator->IPCOpen());
+    MOZ_ASSERT(!mAllocator->IPCOpen());
-      return false;
+    return false;
+  }
+
+  if (!mUpdated) {
+    if (mIsPendingForwardReadLocked) {
+      mIsPendingForwardReadLocked = false;
+      mReadLock->ReadUnlock();
     }
-
+    return false;
-    // Take a read lock on behalf of the TextureHost. The latter will unlock
-    // after the shared data is available again for drawing.
-    mReadLock->ReadLock();
   }
 
   mUpdated = false;
+
+  if (mIsPendingForwardReadLocked) {
+    // We have successfully forwarded, just clear the flag and let the
+    // TextureHost be responsible for unlocking.
+    mIsPendingForwardReadLocked = false;
+  } else {
+    // Otherwise we did not need to readlock in advance, so do so now. We do
+    // this on behalf of the TextureHost.
+    mReadLock->ReadLock();
+  }
+
   return true;
 }
 

gfx/layers/client/TextureClient.h

@@ -677,6 +677,8 @@ class TextureClient : public AtomicRefCountedWithFinalize<TextureClient> {
 
   void SetUpdated() { mUpdated = true; }
 
+  void OnPrepareForwardToHost();
+  void OnAbandonForwardToHost();
   bool OnForwardedToHost();
 
   // Mark that the TextureClient will be used by the paint thread, and should
@@ -762,6 +764,7 @@ class TextureClient : public AtomicRefCountedWithFinalize<TextureClient> {
 #endif
   bool mIsLocked;
   bool mIsReadLocked MOZ_GUARDED_BY(mMutex);
+  bool mIsPendingForwardReadLocked MOZ_GUARDED_BY(mMutex) = false;
   // This member tracks that the texture was written into until the update
   // is sent to the compositor. We need this remember to lock mReadLock on
   // behalf of the compositor just before sending the notification.

gfx/layers/ipc/ImageBridgeChild.cpp

@@ -33,10 +33,12 @@
 #include "mozilla/mozalloc.h"  // for operator new, etc
 #include "transport/runnable_utils.h"
 #include "nsContentUtils.h"
+#include "nsGlobalWindowInner.h"
 #include "nsISupportsImpl.h"         // for ImageContainer::AddRef, etc
 #include "nsTArray.h"                // for AutoTArray, nsTArray, etc
 #include "nsTArrayForwardDeclare.h"  // for AutoTArray
 #include "nsThreadUtils.h"           // for NS_IsMainThread
+#include "WindowRenderer.h"
 
 #if defined(XP_WIN)
 #  include "mozilla/gfx/DeviceManagerDx.h"
@@ -395,6 +397,43 @@ void ImageBridgeChild::FlushAllImages(ImageClient* aClient,
   task.Wait();
 }
 
+void ImageBridgeChild::SyncWithCompositor(const Maybe<uint64_t>& aWindowID) {
+  if (NS_WARN_IF(InImageBridgeChildThread())) {
+    MOZ_ASSERT_UNREACHABLE("Cannot call on ImageBridge thread!");
+    return;
+  }
+
+  if (!aWindowID) {
+    return;
+  }
+
+  const auto fnSyncWithWindow = [&]() {
+    if (auto* window = nsGlobalWindowInner::GetInnerWindowWithId(*aWindowID)) {
+      if (auto* widget = window->GetNearestWidget()) {
+        if (auto* renderer = widget->GetWindowRenderer()) {
+          if (auto* kc = renderer->AsKnowsCompositor()) {
+            kc->SyncWithCompositor();
+          }
+        }
+      }
+    }
+  };
+
+  if (NS_IsMainThread()) {
+    fnSyncWithWindow();
+    return;
+  }
+
+  SynchronousTask task("SyncWithCompositor Lock");
+  RefPtr<Runnable> runnable =
+      NS_NewRunnableFunction("ImageBridgeChild::SyncWithCompositor", [&]() {
+        AutoCompleteTask complete(&task);
+        fnSyncWithWindow();
+      });
+  NS_DispatchToMainThread(runnable.forget());
+  task.Wait();
+}
+
 void ImageBridgeChild::BeginTransaction() {
   MOZ_ASSERT(CanSend());
   MOZ_ASSERT(mTxn->Finished(), "uncommitted txn?");

gfx/layers/ipc/ImageBridgeChild.h

@@ -166,6 +166,9 @@ class ImageBridgeChild final : public PImageBridgeChild,
 
   base::ProcessId GetParentPid() const override { return OtherPid(); }
 
+  void SyncWithCompositor(
+      const Maybe<uint64_t>& aWindowID = Nothing()) override;
+
   PTextureChild* AllocPTextureChild(
       const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock,
       const LayersBackend& aLayersBackend, const TextureFlags& aFlags,

gfx/layers/ipc/KnowsCompositor.cpp

@@ -46,8 +46,9 @@ LayersIPCActor* KnowsCompositorMediaProxy::GetLayersIPCActor() {
   return mThreadSafeAllocator->GetLayersIPCActor();
 }
 
-void KnowsCompositorMediaProxy::SyncWithCompositor() {
+void KnowsCompositorMediaProxy::SyncWithCompositor(
-  mThreadSafeAllocator->SyncWithCompositor();
+    const Maybe<uint64_t>& aWindowID) {
+  mThreadSafeAllocator->SyncWithCompositor(aWindowID);
 }
 
 bool IsSurfaceDescriptorValid(const SurfaceDescriptor& aSurface) {

gfx/layers/ipc/KnowsCompositor.h

@@ -175,7 +175,10 @@ class KnowsCompositor {
    * content process accumulates resource allocations that the compositor is not
    * consuming and releasing.
    */
-  virtual void SyncWithCompositor() { MOZ_ASSERT_UNREACHABLE("Unimplemented"); }
+  virtual void SyncWithCompositor(
+      const Maybe<uint64_t>& aWindowID = Nothing()) {
+    MOZ_ASSERT_UNREACHABLE("Unimplemented");
+  }
 
   /**
    * Helpers for finding other related interface. These are infallible.
@@ -216,7 +219,8 @@ class KnowsCompositorMediaProxy : public KnowsCompositor {
 
   LayersIPCActor* GetLayersIPCActor() override;
 
-  void SyncWithCompositor() override;
+  void SyncWithCompositor(
+      const Maybe<uint64_t>& aWindowID = Nothing()) override;
 
  protected:
   virtual ~KnowsCompositorMediaProxy();

gfx/layers/wr/WebRenderBridgeChild.cpp

@@ -347,7 +347,8 @@ LayersIPCActor* WebRenderBridgeChild::GetLayersIPCActor() {
   return static_cast<LayersIPCActor*>(GetCompositorBridgeChild());
 }
 
-void WebRenderBridgeChild::SyncWithCompositor() {
+void WebRenderBridgeChild::SyncWithCompositor(
+    const Maybe<uint64_t>& aWindowID) {
   if (!IPCOpen()) {
     return;
   }

gfx/layers/wr/WebRenderBridgeChild.h

@@ -93,7 +93,8 @@ class WebRenderBridgeChild final : public PWebRenderBridgeChild,
   // KnowsCompositor
   TextureForwarder* GetTextureForwarder() override;
   LayersIPCActor* GetLayersIPCActor() override;
-  void SyncWithCompositor() override;
+  void SyncWithCompositor(
+      const Maybe<uint64_t>& aWindowID = Nothing()) override;
 
   void AddPipelineIdForCompositable(const wr::PipelineId& aPipelineId,
                                     const CompositableHandle& aHandle,

modules/libpref/init/StaticPrefList.yaml

@@ -6195,6 +6195,11 @@
   value: true
   mirror: always
 
+- name: gfx.offscreencanvas.shared-provider
+  type: RelaxedAtomicBool
+  value: true
+  mirror: always
+
 - name: gfx.omta.background-color
   type: bool
   value: true