Bug 1876506 - Check for CopyToSwapChain failure. r=aosmond

CopyToSwapChain was silently failing, causing no texture to get pushed
to RemoteTextureMap, so that when a wait on it was occurring, it would
timeout.

The failure occurred in DrawTargetWebgl::FlushFromSkia, because the
DT's size actually exceeded the value of the texture limit pref when
it was attempting to allocate a temporary texture to blend back a
Skia layer to the WebGL framebuffer. This is fixed by allowing layering
to bypass this limit, as it is always expected that layer blending
succeed.

To guard against future instances of this bug, CopyToSwapChain now returns
a boolean result so that it is fallible and can signal to CanvasTranslator
that it needs to take appropriate fallback measures on failure.

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

dom/canvas/DrawTargetWebgl.cpp

@@ -1008,18 +1008,25 @@ bool DrawTargetWebgl::HasDataSnapshot() const {
   return (mSkiaValid && !mSkiaLayer) || (mSnapshot && mSnapshot->HasReadData());
 }
 
-void DrawTargetWebgl::PrepareData() {
+bool DrawTargetWebgl::PrepareSkia() {
   if (!mSkiaValid) {
     ReadIntoSkia();
   } else if (mSkiaLayer) {
     FlattenSkia();
   }
+  return mSkiaValid;
 }
 
+bool DrawTargetWebgl::EnsureDataSnapshot() {
+  return HasDataSnapshot() || PrepareSkia();
+}
+
+void DrawTargetWebgl::PrepareShmem() { PrepareSkia(); }
+
 // Borrow a snapshot that may be used by another thread for composition. Only
 // Skia snapshots are safe to pass around.
 already_AddRefed<SourceSurface> DrawTargetWebgl::GetDataSnapshot() {
-  PrepareData();
+  PrepareSkia();
   return mSkia->Snapshot(mFormat);
 }
 
@@ -2168,9 +2175,12 @@ bool SharedContextWebgl::DrawRectAccel(
 
   // Check if the drawing options and the pattern support acceleration. Also
   // ensure the framebuffer is prepared for drawing. If not, fall back to using
-  // the Skia target.
-  if (!SupportsDrawOptions(aOptions) || !SupportsPattern(aPattern) ||
-      aStrokeOptions || !mCurrentTarget->MarkChanged()) {
+  // the Skia target. When we need to forcefully update a texture, we must be
+  // careful to override any pattern limits, as the caller ensures the pattern
+  // is otherwise a supported type.
+  if (!SupportsDrawOptions(aOptions) ||
+      (!aForceUpdate && !SupportsPattern(aPattern)) || aStrokeOptions ||
+      !mCurrentTarget->MarkChanged()) {
     // If only accelerated drawing was requested, bail out without software
     // drawing fallback.
     if (!aAccelOnly) {
@@ -4690,11 +4700,11 @@ void DrawTargetWebgl::EndFrame() {
   mSharedContext->ClearCachesIfNecessary();
 }
 
-void DrawTargetWebgl::CopyToSwapChain(
+bool DrawTargetWebgl::CopyToSwapChain(
     layers::RemoteTextureId aId, layers::RemoteTextureOwnerId aOwnerId,
     layers::RemoteTextureOwnerClient* aOwnerClient) {
   if (!mWebglValid && !FlushFromSkia()) {
-    return;
+    return false;
   }
 
   // Copy and swizzle the WebGL framebuffer to the swap chain front buffer.
@@ -4709,8 +4719,8 @@ void DrawTargetWebgl::CopyToSwapChain(
   const RefPtr<layers::ImageBridgeChild> imageBridge =
       layers::ImageBridgeChild::GetSingleton();
   auto texType = layers::TexTypeForWebgl(imageBridge);
-  mSharedContext->mWebgl->CopyToSwapChain(mFramebuffer, texType, options,
-                                          aOwnerClient);
+  return mSharedContext->mWebgl->CopyToSwapChain(mFramebuffer, texType, options,
+                                                 aOwnerClient);
 }
 
 already_AddRefed<DrawTarget> DrawTargetWebgl::CreateSimilarDrawTarget(

dom/canvas/DrawTargetWebgl.h

@@ -457,7 +457,8 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr {
   }
 
   bool HasDataSnapshot() const;
-  void PrepareData();
+  bool EnsureDataSnapshot();
+  void PrepareShmem();
   already_AddRefed<SourceSurface> GetDataSnapshot();
   already_AddRefed<SourceSurface> Snapshot() override;
   already_AddRefed<SourceSurface> GetOptimizedSnapshot(DrawTarget* aTarget);
@@ -571,7 +572,7 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr {
   void SetTransform(const Matrix& aTransform) override;
   void* GetNativeSurface(NativeSurfaceType aType) override;
 
-  void CopyToSwapChain(
+  bool CopyToSwapChain(
       layers::RemoteTextureId aId, layers::RemoteTextureOwnerId aOwnerId,
       layers::RemoteTextureOwnerClient* aOwnerClient = nullptr);
 
@@ -630,6 +631,7 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr {
 
   bool ReadIntoSkia();
   void FlattenSkia();
+  bool PrepareSkia();
   bool FlushFromSkia();
 
   void MarkSkiaChanged(bool aOverwrite = false);

dom/canvas/WebGLContext.cpp

@@ -1082,19 +1082,23 @@ void WebGLContext::Present(WebGLFramebuffer* const xrFb,
   }
 }
 
-void WebGLContext::CopyToSwapChain(
+bool WebGLContext::CopyToSwapChain(
     WebGLFramebuffer* const srcFb, const layers::TextureType consumerType,
     const webgl::SwapChainOptions& options,
     layers::RemoteTextureOwnerClient* ownerClient) {
   const FuncScope funcScope(*this, "<CopyToSwapChain>");
-  if (IsContextLost()) return;
+  if (IsContextLost()) {
+    return false;
+  }
 
   OnEndOfFrame();
 
-  if (!srcFb) return;
+  if (!srcFb) {
+    return false;
+  }
   const auto* info = srcFb->GetCompletenessInfo();
   if (!info) {
-    return;
+    return false;
   }
   gfx::IntSize size(info->width, info->height);
 
@@ -1108,8 +1112,8 @@ void WebGLContext::CopyToSwapChain(
   // read back the WebGL framebuffer into and push it as a remote texture.
   if (useAsync && srcFb->mSwapChain.mFactory->GetConsumerType() ==
                       layers::TextureType::Unknown) {
-    PushRemoteTexture(srcFb, srcFb->mSwapChain, nullptr, options, ownerClient);
-    return;
+    return PushRemoteTexture(srcFb, srcFb->mSwapChain, nullptr, options,
+                             ownerClient);
   }
 
   {
@@ -1119,7 +1123,7 @@ void WebGLContext::CopyToSwapChain(
     if (!presenter) {
       GenerateWarning("Swap chain surface creation failed.");
       LoseContext();
-      return;
+      return false;
     }
 
     const ScopedFBRebinder saveFB(this);
@@ -1131,9 +1135,11 @@ void WebGLContext::CopyToSwapChain(
   }
 
   if (useAsync) {
-    PushRemoteTexture(srcFb, srcFb->mSwapChain, srcFb->mSwapChain.FrontBuffer(),
-                      options, ownerClient);
+    return PushRemoteTexture(srcFb, srcFb->mSwapChain,
+                             srcFb->mSwapChain.FrontBuffer(), options,
+                             ownerClient);
   }
+  return true;
 }
 
 bool WebGLContext::PushRemoteTexture(

dom/canvas/WebGLContext.h

@@ -499,7 +499,7 @@ class WebGLContext : public VRefCounted, public SupportsWeakPtr {
   // may differ subject to available format conversion options. Since this
   // operation uses an explicit copy, it inherently preserves the framebuffer
   // without need to set the preserveDrawingBuffer option.
-  void CopyToSwapChain(
+  bool CopyToSwapChain(
       WebGLFramebuffer*, layers::TextureType,
       const webgl::SwapChainOptions& options = webgl::SwapChainOptions(),
       layers::RemoteTextureOwnerClient* ownerClient = nullptr);

gfx/layers/RemoteTextureMap.cpp

@@ -808,6 +808,7 @@ void RemoteTextureMap::NotifyContextLost(
     const RemoteTextureOwnerIdSet& aOwnerIds, const base::ProcessId aForPid) {
   MonitorAutoLock lock(mMonitor);
 
+  bool changed = false;
   for (const auto& id : aOwnerIds) {
     const auto key = std::pair(aForPid, id);
     auto it = mTextureOwners.find(key);
@@ -816,16 +817,22 @@ void RemoteTextureMap::NotifyContextLost(
       continue;
     }
     auto& owner = it->second;
-    owner->mIsContextLost = true;
+    if (!owner->mIsContextLost) {
+      owner->mIsContextLost = true;
+      changed = true;
+    }
   }
 
-  mMonitor.Notify();
+  if (changed) {
+    mMonitor.Notify();
+  }
 }
 
 void RemoteTextureMap::NotifyContextRestored(
     const RemoteTextureOwnerIdSet& aOwnerIds, const base::ProcessId aForPid) {
   MonitorAutoLock lock(mMonitor);
 
+  bool changed = false;
   for (const auto& id : aOwnerIds) {
     const auto key = std::pair(aForPid, id);
     auto it = mTextureOwners.find(key);
@@ -834,10 +841,15 @@ void RemoteTextureMap::NotifyContextRestored(
       continue;
     }
     auto& owner = it->second;
-    owner->mIsContextLost = false;
+    if (owner->mIsContextLost) {
+      owner->mIsContextLost = false;
+      changed = true;
+    }
   }
 
-  mMonitor.Notify();
+  if (changed) {
+    mMonitor.Notify();
+  }
 }
 
 /* static */
@@ -965,6 +977,10 @@ bool RemoteTextureMap::GetRemoteTexture(
       const TimeDuration timeout = TimeDuration::FromMilliseconds(10000);
       while (!owner || (!owner->mLatestTextureHost &&
                         owner->mWaitingTextureDataHolders.empty())) {
+        if (owner && owner->mIsContextLost) {
+          // If the context was lost, no further updates are expected.
+          return false;
+        }
         CVStatus status = mMonitor.Wait(timeout);
         if (status == CVStatus::Timeout) {
           return false;

gfx/layers/ipc/CanvasTranslator.cpp

@@ -417,6 +417,28 @@ inline gfx::DrawTargetWebgl* CanvasTranslator::TextureInfo::GetDrawTargetWebgl(
   return nullptr;
 }
 
+bool CanvasTranslator::TryDrawTargetWebglFallback(
+    int64_t aTextureId, gfx::DrawTargetWebgl* aWebgl) {
+  NotifyRequiresRefresh(aTextureId);
+
+  // An existing data snapshot is required for fallback, as we have to avoid
+  // trying to touch the WebGL context, which is assumed to be invalid and not
+  // suitable for readback.
+  if (!aWebgl->HasDataSnapshot()) {
+    return false;
+  }
+
+  const auto& info = mTextureInfo[aTextureId];
+  if (RefPtr<gfx::DrawTarget> dt = CreateFallbackDrawTarget(
+          info.mRefPtr, aTextureId, info.mRemoteTextureOwnerId,
+          aWebgl->GetSize(), aWebgl->GetFormat())) {
+    aWebgl->CopyToFallback(dt);
+    AddDrawTarget(info.mRefPtr, dt);
+    return true;
+  }
+  return false;
+}
+
 void CanvasTranslator::ForceDrawTargetWebglFallback() {
   // This looks for any DrawTargetWebgls that have a cached data snapshot that
   // can be used to recover a fallback TextureData in the event of a context
@@ -425,22 +447,14 @@ void CanvasTranslator::ForceDrawTargetWebglFallback() {
   for (const auto& entry : mTextureInfo) {
     const auto& info = entry.second;
     if (gfx::DrawTargetWebgl* webgl = info.GetDrawTargetWebgl()) {
-      NotifyRequiresRefresh(entry.first);
-      if (webgl->HasDataSnapshot()) {
-        if (RefPtr<gfx::DrawTarget> dt = CreateFallbackDrawTarget(
-                info.mRefPtr, entry.first, info.mRemoteTextureOwnerId,
-                webgl->GetSize(), webgl->GetFormat())) {
-          webgl->CopyToFallback(dt);
-          AddDrawTarget(info.mRefPtr, dt);
-          continue;
+      if (!TryDrawTargetWebglFallback(entry.first, webgl)) {
+        // No fallback could be created, so we need to notify the compositor the
+        // texture won't be pushed.
+        if (mRemoteTextureOwner &&
+            mRemoteTextureOwner->IsRegistered(info.mRemoteTextureOwnerId)) {
+          lost.insert(info.mRemoteTextureOwnerId);
         }
       }
-      // No fallback could be created, so we need to notify the compositor the
-      // texture won't be pushed.
-      if (mRemoteTextureOwner &&
-          mRemoteTextureOwner->IsRegistered(info.mRemoteTextureOwnerId)) {
-        lost.insert(info.mRemoteTextureOwnerId);
-      }
     }
   }
   if (!lost.empty()) {
@@ -815,7 +829,7 @@ void CanvasTranslator::PrepareShmem(int64_t aTextureId) {
       }
     } else {
       // Otherwise, just ensure the software framebuffer is up to date.
-      webgl->PrepareData();
+      webgl->PrepareShmem();
     }
   }
 }
@@ -829,9 +843,7 @@ void CanvasTranslator::ClearCachedResources() {
     mSharedContext->OnMemoryPressure();
     for (auto const& entry : mTextureInfo) {
       if (gfx::DrawTargetWebgl* webgl = entry.second.GetDrawTargetWebgl()) {
-        if (!webgl->HasDataSnapshot()) {
-          webgl->PrepareData();
-        }
+        webgl->EnsureDataSnapshot();
       }
     }
   }
@@ -1021,15 +1033,22 @@ bool CanvasTranslator::PresentTexture(int64_t aTextureId, RemoteTextureId aId) {
   RemoteTextureOwnerId ownerId = info.mRemoteTextureOwnerId;
   if (gfx::DrawTargetWebgl* webgl = info.GetDrawTargetWebgl()) {
     EnsureRemoteTextureOwner(ownerId);
-    // Check for context loss to avoid CopyToSwapChain becoming a no-op.
-    if (webgl->IsValid()) {
-      webgl->CopyToSwapChain(aId, ownerId, mRemoteTextureOwner);
-      if (webgl->IsValid()) {
-        return true;
+    if (webgl->CopyToSwapChain(aId, ownerId, mRemoteTextureOwner)) {
+      return true;
+    }
+    if (mSharedContext && mSharedContext->IsContextLost()) {
+      // If the context was lost, try to create a fallback to push instead.
+      EnsureSharedContextWebgl();
+    } else {
+      // CopyToSwapChain failed for an unknown reason other than context loss.
+      // Try to read into fallback data if possible to recover, otherwise force
+      // the loss of the individual texture.
+      webgl->EnsureDataSnapshot();
+      if (!TryDrawTargetWebglFallback(aTextureId, webgl)) {
+        RemoteTextureOwnerIdSet lost = {info.mRemoteTextureOwnerId};
+        mRemoteTextureOwner->NotifyContextLost(&lost);
       }
     }
-    // If the context was lost, try to create a fallback to push instead.
-    EnsureSharedContextWebgl();
   }
   if (TextureData* data = info.mTextureData.get()) {
     PushRemoteTexture(aTextureId, data, aId, ownerId);

gfx/layers/ipc/CanvasTranslator.h

@@ -289,6 +289,8 @@ class CanvasTranslator final : public gfx::InlineTranslator,
 
   void Deactivate();
 
+  bool TryDrawTargetWebglFallback(int64_t aTextureId,
+                                  gfx::DrawTargetWebgl* aWebgl);
   void ForceDrawTargetWebglFallback();
 
   void BlockCanvas();