Bug 1279617 - When SurfacePipe::WritePixels() finishes early, zero out the rest of the surface. r=njn

image/SurfacePipe.h

@@ -185,10 +185,7 @@ public:
             return WriteState::NEED_MORE_DATA;
 
           case WriteState::FINISHED:
-            // Make sure that IsSurfaceFinished() returns true so the caller
-            // can't write anything else to the pipeline.
-            mRowPointer = nullptr;
-            mCol = 0;
+            ZeroOutRestOfSurface<PixelType>();
             return WriteState::FINISHED;
 
           case WriteState::FAILURE:
@@ -444,6 +441,13 @@ protected:
   }
 
 private:
+
+  template <typename PixelType>
+  void ZeroOutRestOfSurface()
+  {
+    WritePixels<PixelType>([]{ return AsVariant(PixelType(0)); });
+  }
+
   gfx::IntSize mInputSize;  /// The size of the input this filter expects.
   uint8_t* mRowPointer;     /// Pointer to the current row or null if finished.
   int32_t mCol;             /// The current column we're writing to. (0-indexed)

image/test/gtest/TestSurfaceSink.cpp

@@ -213,7 +213,10 @@ TEST(ImageSurfaceSink, SurfaceSinkWritePixelsFinish)
     });
     EXPECT_EQ(WriteState::FINISHED, result);
     EXPECT_EQ(1u, count);
-    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
 
     // Attempt to write more and make sure that nothing gets written.
     count = 0;
@@ -232,6 +235,81 @@ TEST(ImageSurfaceSink, SurfaceSinkWritePixelsFinish)
   });
 }
 
+TEST(ImageSurfaceSink, SurfaceSinkWritePixelsEarlyExit)
+{
+  auto checkEarlyExit =
+    [](Decoder* aDecoder, SurfaceSink* aSink, WriteState aState) {
+    // Write half a row of green pixels and then exit early with |aState|. If
+    // the lambda keeps getting called, we'll write red pixels, which will cause
+    // the test to fail.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint32_t>([&]() -> NextPixel<uint32_t> {
+      if (count == 50) {
+        return AsVariant(aState);
+      }
+      return count++ < 50 ? AsVariant(BGRAColor::Green().AsPixel())
+                          : AsVariant(BGRAColor::Red().AsPixel());
+    });
+
+    EXPECT_EQ(aState, result);
+    EXPECT_EQ(50u, count);
+    CheckGeneratedImage(aDecoder, IntRect(0, 0, 50, 1));
+
+    if (aState != WriteState::FINISHED) {
+      // We should still be able to write more at this point.
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+
+      // Verify that we can resume writing. We'll finish up the same row.
+      count = 0;
+      result = aSink->WritePixels<uint32_t>([&]() -> NextPixel<uint32_t> {
+        if (count == 50) {
+          return AsVariant(WriteState::NEED_MORE_DATA);
+        }
+        ++count;
+        return AsVariant(BGRAColor::Green().AsPixel());
+      });
+
+      EXPECT_EQ(WriteState::NEED_MORE_DATA, result);
+      EXPECT_EQ(50u, count);
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+      CheckGeneratedImage(aDecoder, IntRect(0, 0, 100, 1));
+
+      return;
+    }
+
+    // We should've finished the surface at this point.
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint32_t>([&]{
+      count++;
+      return AsVariant(BGRAColor::Red().AsPixel());
+    });
+
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is still correct.
+    CheckGeneratedImage(aDecoder, IntRect(0, 0, 50, 1));
+  };
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::NEED_MORE_DATA);
+  });
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FAILURE);
+  });
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FINISHED);
+  });
+}
+
 TEST(ImageSurfaceSink, SurfaceSinkWriteBuffer)
 {
   WithSurfaceSink<Orient::NORMAL>([](Decoder* aDecoder, SurfaceSink* aSink) {
@@ -809,6 +887,115 @@ TEST(ImageSurfaceSink, PalettedSurfaceSinkWritePixelsFor75_75_50_50)
   });
 }
 
+TEST(ImageSurfaceSink, PalettedSurfaceSinkWritePixelsFinish)
+{
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    // Write nothing into the surface; just finish immediately.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint8_t>([&]{
+      count++;
+      return AsVariant(WriteState::FINISHED);
+    });
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(1u, count);
+
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint8_t>([&]() {
+      count++;
+      return AsVariant(uint8_t(128));
+    });
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is correct.
+    EXPECT_TRUE(IsSolidPalettedColor(aDecoder, 0));
+  });
+}
+
+TEST(ImageSurfaceSink, PalettedSurfaceSinkWritePixelsEarlyExit)
+{
+  auto checkEarlyExit =
+    [](Decoder* aDecoder, PalettedSurfaceSink* aSink, WriteState aState) {
+    // Write half a row of green pixels and then exit early with |aState|. If
+    // the lambda keeps getting called, we'll write red pixels, which will cause
+    // the test to fail.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint8_t>([&]() -> NextPixel<uint8_t> {
+      if (count == 50) {
+        return AsVariant(aState);
+      }
+      return count++ < 50 ? AsVariant(uint8_t(255)) : AsVariant(uint8_t(128));
+    });
+
+    EXPECT_EQ(aState, result);
+    EXPECT_EQ(50u, count);
+    CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 50, 1));
+
+    if (aState != WriteState::FINISHED) {
+      // We should still be able to write more at this point.
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+
+      // Verify that we can resume writing. We'll finish up the same row.
+      count = 0;
+      result = aSink->WritePixels<uint8_t>([&]() -> NextPixel<uint8_t> {
+        if (count == 50) {
+          return AsVariant(WriteState::NEED_MORE_DATA);
+        }
+        ++count;
+        return AsVariant(uint8_t(255));
+      });
+
+      EXPECT_EQ(WriteState::NEED_MORE_DATA, result);
+      EXPECT_EQ(50u, count);
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+      CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 100, 1));
+
+      return;
+    }
+
+    // We should've finished the surface at this point.
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint8_t>([&]{
+      count++;
+      return AsVariant(uint8_t(128));
+    });
+
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is still correct.
+    CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 50, 1));
+  };
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::NEED_MORE_DATA);
+  });
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FAILURE);
+  });
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FINISHED);
+  });
+}
+
 TEST(ImageSurfaceSink, PalettedSurfaceSinkWriteBuffer)
 {
   WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),