Backed out 2 changesets (bug 1516011) for build bustge in /build/build/src/gfx/layers/wr/WebRenderCommandBuilder.cpp

Backed out changeset 3c5fcd8a2f4a (bug 1516011)
Backed out changeset c74c81fe8dcb (bug 1516011)

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -308,21 +308,6 @@ SharedSurfacesChild::AsSourceSurfaceSharedData(SourceSurface* aSurface) {
   return rv;
 }
 
-/* static */ nsresult SharedSurfacesChild::Share(
-    SourceSurface* aSurface, RenderRootStateManager* aManager,
-    wr::IpcResourceUpdateQueue& aResources, wr::ImageKey& aKey) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aSurface);
-  MOZ_ASSERT(aManager);
-
-  auto sharedSurface = AsSourceSurfaceSharedData(aSurface);
-  if (!sharedSurface) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
-  return Share(sharedSurface, aManager, aResources, aKey);
-}
-
 /* static */ nsresult SharedSurfacesChild::Share(
     ImageContainer* aContainer, RenderRootStateManager* aManager,
     wr::IpcResourceUpdateQueue& aResources, wr::ImageKey& aKey) {

gfx/layers/ipc/SharedSurfacesChild.h

@@ -71,16 +71,6 @@ class SharedSurfacesChild final {
                         wr::IpcResourceUpdateQueue& aResources,
                         wr::ImageKey& aKey);
 
-  /**
-   * Request that the surface be mapped into the compositor thread's memory
-   * space, and a valid ImageKey be generated for it for use with WebRender.
-   * This must be called from the main thread.
-   */
-  static nsresult Share(gfx::SourceSurface* aSurface,
-                        RenderRootStateManager* aManager,
-                        wr::IpcResourceUpdateQueue& aResources,
-                        wr::ImageKey& aKey);
-
   /**
    * Request that the first surface in the image container's current images be
    * mapped into the compositor thread's memory space, and a valid ImageKey be

gfx/layers/wr/WebRenderCommandBuilder.cpp

@@ -8,7 +8,6 @@
 
 #include "BasicLayers.h"
 #include "mozilla/AutoRestore.h"
-#include "mozilla/DebugOnly.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Logging.h"
 #include "mozilla/gfx/Types.h"
@@ -185,14 +184,18 @@ static void TakeExternalSurfaces(
   aRecorder->TakeExternalSurfaces(aExternalSurfaces);
 
   for (auto& surface : aExternalSurfaces) {
+    if (surface->GetType() != SurfaceType::DATA_SHARED) {
+      MOZ_ASSERT_UNREACHABLE("External surface that is not a shared surface!");
+      continue;
+    }
+
     // While we don't use the image key with the surface, because the blob image
     // renderer doesn't have easy access to the resource set, we still want to
     // ensure one is generated. That will ensure the surface remains alive until
     // at least the last epoch which the blob image could be used in.
     wr::ImageKey key;
-    DebugOnly<nsresult> rv =
-        SharedSurfacesChild::Share(surface, aManager, aResources, key);
-    MOZ_ASSERT(rv != NS_ERROR_NOT_IMPLEMENTED);
+    auto sharedSurface = static_cast<SourceSurfaceSharedData*>(surface.get());
+    SharedSurfacesChild::Share(sharedSurface, aManager, aResources, key);
   }
 }
 

image/AnimationFrameBuffer.cpp

@@ -352,7 +352,9 @@ void AnimationFrameRecyclingQueue::AdvanceInternal() {
 
   // If we are allowed to recycle the frame, then we should save it before the
   // base class's AdvanceInternal discards it.
-  newEntry.mFrame = std::move(front);
+  if (front->ShouldRecycle()) {
+    newEntry.mFrame = std::move(front);
+  }
 
   // Even if the frame itself isn't saved, we want the dirty rect to calculate
   // the recycle rect for future recycled frames.
@@ -391,9 +393,11 @@ bool AnimationFrameRecyclingQueue::ResetInternal() {
   // existing mRecycle entries, because that will happen in RecycleFrame when
   // we try to pull out a frame to redecode the first frame.
   for (RefPtr<imgFrame>& frame : mDisplay) {
-    RecycleEntry newEntry(mFirstFrameRefreshArea);
-    newEntry.mFrame = std::move(frame);
-    mRecycle.push_back(std::move(newEntry));
+    if (frame->ShouldRecycle()) {
+      RecycleEntry newEntry(mFirstFrameRefreshArea);
+      newEntry.mFrame = std::move(frame);
+      mRecycle.push_back(std::move(newEntry));
+    }
   }
 
   return AnimationFrameDiscardingQueue::ResetInternal();

image/Decoder.cpp

@@ -350,7 +350,8 @@ RawAccessFrameRef Decoder::AllocateFrameInternal(
       // animation parameters elsewhere. For now we just drop it.
       bool blocked = ref.get() == mRestoreFrame.get();
       if (!blocked) {
-        blocked = NS_FAILED(ref->InitForDecoderRecycle(aAnimParams.ref()));
+        nsresult rv = ref->InitForDecoderRecycle(aAnimParams.ref());
+        blocked = NS_WARN_IF(NS_FAILED(rv));
       }
 
       if (blocked) {

image/imgFrame.cpp

@@ -305,12 +305,7 @@ nsresult imgFrame::InitForDecoderRecycle(const AnimationParams& aAnimParams) {
   MOZ_ASSERT(mIsFullFrame);
   MOZ_ASSERT(mLockCount > 0);
   MOZ_ASSERT(mLockedSurface);
-
-  if (!mShouldRecycle) {
-    // This frame either was never marked as recyclable, or the flag was cleared
-    // for a caller which does not support recycling.
-    return NS_ERROR_NOT_AVAILABLE;
-  }
+  MOZ_ASSERT(mShouldRecycle);
 
   if (mRecycleLockCount > 0) {
     if (NS_IsMainThread()) {
@@ -629,8 +624,8 @@ bool imgFrame::Draw(gfxContext* aContext, const ImageRegion& aRegion,
     // this frame is the current frame of the animation. Since we can only
     // advance on the main thread, we know nothing else will try to use it.
     DrawTarget* drawTarget = aContext->GetDrawTarget();
-    bool recording = drawTarget->GetBackendType() == BackendType::RECORDING;
-    bool temporary = !drawTarget->IsCaptureDT() && !recording;
+    bool temporary = !drawTarget->IsCaptureDT() &&
+                     drawTarget->GetBackendType() != BackendType::RECORDING;
     RefPtr<SourceSurface> surf = GetSourceSurfaceInternal(temporary);
     if (!surf) {
       return false;
@@ -640,14 +635,6 @@ bool imgFrame::Draw(gfxContext* aContext, const ImageRegion& aRegion,
                   !(aImageFlags & imgIContainer::FLAG_CLAMP);
 
     surfaceResult = SurfaceForDrawing(doPartialDecode, doTile, region, surf);
-
-    // If we are recording, then we cannot recycle the surface. The blob
-    // rasterizer is not properly synchronized for recycling in the compositor
-    // process. The easiest thing to do is just mark the frames it consumes as
-    // non-recyclable.
-    if (recording && surfaceResult.IsValid()) {
-      mShouldRecycle = false;
-    }
   }
 
   if (surfaceResult.IsValid()) {

image/imgFrame.h

@@ -202,6 +202,8 @@ class imgFrame {
   bool GetCompositingFailed() const;
   void SetCompositingFailed(bool val);
 
+  bool ShouldRecycle() const { return mShouldRecycle; }
+
   void SetOptimizable();
 
   void FinalizeSurface();

image/test/gtest/TestAnimationFrameBuffer.cpp

@@ -31,17 +31,6 @@ static already_AddRefed<imgFrame> CreateEmptyFrame(
   return frame.forget();
 }
 
-static bool ReinitForRecycle(RawAccessFrameRef& aFrame) {
-  if (!aFrame) {
-    return false;
-  }
-
-  AnimationParams animParams{aFrame->GetRect(), FrameTimeout::Forever(),
-                             /* aFrameNum */ 1, BlendMethod::OVER,
-                             DisposalMethod::NOT_SPECIFIED};
-  return NS_SUCCEEDED(aFrame->InitForDecoderRecycle(animParams));
-}
-
 static void PrepareForDiscardingQueue(AnimationFrameRetainedBuffer& aQueue) {
   ASSERT_EQ(size_t(0), aQueue.Size());
   ASSERT_LT(size_t(1), aQueue.Batch());
@@ -111,7 +100,11 @@ static void VerifyAdvance(AnimationFrameBuffer& aQueue, size_t aExpectedFrame,
     EXPECT_TRUE(
         queue.Recycle().back().mDirtyRect.Contains(oldFrame->GetDirtyRect()));
     EXPECT_EQ(totalRecycled + 1, queue.Recycle().size());
-    EXPECT_EQ(oldFrame.get(), queue.Recycle().back().mFrame.get());
+    if (oldFrame->ShouldRecycle()) {
+      EXPECT_EQ(oldFrame.get(), queue.Recycle().back().mFrame.get());
+    } else {
+      EXPECT_EQ(nullptr, queue.Recycle().back().mFrame.get());
+    }
   }
 }
 
@@ -571,14 +564,13 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingLoop) {
     RawAccessFrameRef gotFrame = buffer.RecycleFrame(gotRect);
     EXPECT_EQ(expectedFrame.get(), gotFrame.get());
     EXPECT_EQ(expectedRect, gotRect);
-    EXPECT_TRUE(ReinitForRecycle(gotFrame));
   }
 
   // Trying to pull a recycled frame when we have nothing should be safe too.
   IntRect gotRect;
   RawAccessFrameRef gotFrame = buffer.RecycleFrame(gotRect);
   EXPECT_TRUE(gotFrame.get() == nullptr);
-  EXPECT_FALSE(ReinitForRecycle(gotFrame));
+  EXPECT_TRUE(gotRect.IsEmpty());
 }
 
 static void TestDiscardingQueueReset(AnimationFrameDiscardingQueue& aQueue,
@@ -753,13 +745,7 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingResetBeforeComplete) {
   EXPECT_TRUE(restartDecoding);
 
   // None of the buffers were recyclable.
-  EXPECT_FALSE(buffer.Recycle().empty());
-  while (!buffer.Recycle().empty()) {
-    IntRect recycleRect;
-    RawAccessFrameRef frameRef = buffer.RecycleFrame(recycleRect);
-    EXPECT_TRUE(frameRef);
-    EXPECT_FALSE(ReinitForRecycle(frameRef));
-  }
+  EXPECT_TRUE(buffer.Recycle().empty());
 
   // Reinsert the first two frames as recyclable and reset again.
   frame = CreateEmptyFrame(kImageSize, kImageRect, true);
@@ -787,25 +773,28 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   const size_t kBatch = 2;
   const size_t kStartFrame = 0;
   const IntSize kImageSize(100, 100);
-  const IntRect kImageRect(IntPoint(0, 0), kImageSize);
   AnimationFrameRetainedBuffer retained(kThreshold, kBatch, kStartFrame);
 
   // Let's get to the recycling state while marking all of the frames as not
   // recyclable, just like AnimationFrameBuffer / the decoders would do.
   RefPtr<imgFrame> frame;
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   AnimationFrameBuffer::InsertStatus status = retained.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
 
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   status = retained.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
 
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   status = retained.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
 
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   status = retained.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
 
@@ -813,7 +802,8 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   VerifyAdvance(retained, 2, true);
   VerifyAdvance(retained, 3, false);
 
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   status = retained.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::DISCARD_CONTINUE, status);
 
@@ -826,8 +816,8 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   IntRect recycleRect;
   EXPECT_FALSE(buffer.Recycle().empty());
   RawAccessFrameRef frameRef = buffer.RecycleFrame(recycleRect);
-  EXPECT_TRUE(frameRef);
-  EXPECT_FALSE(ReinitForRecycle(frameRef));
+  EXPECT_FALSE(frameRef);
+  EXPECT_TRUE(recycleRect.IsEmpty());
   EXPECT_TRUE(buffer.Recycle().empty());
 
   // Insert a recyclable partial frame. Its dirty rect shouldn't matter since
@@ -839,8 +829,8 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   VerifyAdvance(buffer, 5, true);
   EXPECT_FALSE(buffer.Recycle().empty());
   frameRef = buffer.RecycleFrame(recycleRect);
-  EXPECT_TRUE(frameRef);
-  EXPECT_FALSE(ReinitForRecycle(frameRef));
+  EXPECT_FALSE(frameRef);
+  EXPECT_TRUE(recycleRect.IsEmpty());
   EXPECT_TRUE(buffer.Recycle().empty());
 
   // Insert a recyclable partial frame. Its dirty rect should match the recycle
@@ -853,7 +843,6 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   EXPECT_FALSE(buffer.Recycle().empty());
   frameRef = buffer.RecycleFrame(recycleRect);
   EXPECT_TRUE(frameRef);
-  EXPECT_TRUE(ReinitForRecycle(frameRef));
   EXPECT_EQ(IntRect(25, 0, 50, 50), recycleRect);
   EXPECT_TRUE(buffer.Recycle().empty());
 
@@ -871,14 +860,14 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   EXPECT_FALSE(buffer.Recycle().empty());
   frameRef = buffer.RecycleFrame(recycleRect);
   EXPECT_TRUE(frameRef);
-  EXPECT_TRUE(ReinitForRecycle(frameRef));
   EXPECT_EQ(IntRect(0, 0, 75, 50), recycleRect);
   EXPECT_TRUE(buffer.Recycle().empty());
 
   // Now let's reinsert the first frame. The recycle rect should still be the
   // first frame refresh area instead of the dirty rect of the first frame (e.g.
   // the full frame).
-  frame = CreateEmptyFrame(kImageSize, kImageRect, false);
+  frame =
+      CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
   status = buffer.Insert(std::move(frame));
   EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
 
@@ -886,7 +875,6 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingRect) {
   EXPECT_FALSE(buffer.Recycle().empty());
   frameRef = buffer.RecycleFrame(recycleRect);
   EXPECT_TRUE(frameRef);
-  EXPECT_TRUE(ReinitForRecycle(frameRef));
   EXPECT_EQ(IntRect(0, 0, 75, 50), recycleRect);
   EXPECT_TRUE(buffer.Recycle().empty());
 }