Make the canvas draw looper setup update the canvas save count

Image filter in a paint would leave save count in wrong state
for normal draws. This could be observed through the canvas
references during the draw call. An example of this is
inspecting the canvas during a draw looper.

patch from issue 993863002 at patchset 20001 (http://crrev.com/993863002#ps20001)

BUG=skia:
TBR=kkinnunen@nvidia.com

Review URL: https://codereview.chromium.org/1034033004

gyp/tests.gypi

@@ -90,7 +90,8 @@
     '../tests/DocumentTest.cpp',
     '../tests/DrawBitmapRectTest.cpp',
     '../tests/DrawPathTest.cpp',
-    '../tests/DrawTextTest.cpp',
+    '../tests/DrawTextTest.cpp', 
+    '../tests/DrawLooperTest.cpp',
     '../tests/DynamicHashTest.cpp',
     '../tests/EmptyPathTest.cpp',
     '../tests/ErrorTest.cpp',

src/core/SkCanvas.cpp

@@ -795,7 +795,6 @@ void SkCanvas::restore() {
         if (fMCStack.count() > 1) {
             this->willRestore();
             SkASSERT(fSaveCount > 1);
-            fSaveCount -= 1;
             this->internalRestore();
             this->didRestore();
         }
@@ -879,7 +878,6 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint) {
         bounds = NULL;
     }
     SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag);
-    fSaveCount += 1;
     this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, strategy);
     return this->getSaveCount() - 1;
 }
@@ -889,7 +887,6 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags fl
         bounds = NULL;
     }
     SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags);
-    fSaveCount += 1;
     this->internalSaveLayer(bounds, paint, flags, strategy);
     return this->getSaveCount() - 1;
 }
@@ -900,6 +897,8 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav
     flags |= kClipToLayer_SaveFlag;
 #endif
 
+    fSaveCount += 1;
+
     // do this before we create the layer. We don't call the public save() since
     // that would invoke a possibly overridden virtual
     this->internalSave();
@@ -978,6 +977,8 @@ int SkCanvas::saveLayerAlpha(const SkRect* bounds, U8CPU alpha,
 void SkCanvas::internalRestore() {
     SkASSERT(fMCStack.count() != 0);
 
+    fSaveCount -= 1;
+
     fDeviceCMDirty = true;
     fCachedLocalClipBoundsDirty = true;
 

tests/DrawLooperTest.cpp

@@ -0,0 +1,92 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkCanvas.h"
+#include "SkColorFilter.h"
+#include "SkColorFilterImageFilter.h"
+#include "SkDrawLooper.h"
+#include "Test.h"
+
+/* Tests for SkDrawLooper -related APIs and implementations. */
+
+namespace {
+/*
+ *  Subclass that caused an assert at the time of writing.
+ */
+class GetSaveCountAssertLooper : public SkDrawLooper {
+public:
+
+    SkDrawLooper::Context* createContext(SkCanvas*, void* storage) const override {
+        return SkNEW_PLACEMENT(storage, GetSaveCountAssertLooperContext);
+    }
+
+    size_t contextSize() const override { return sizeof(GetSaveCountAssertLooperContext); }
+
+#ifndef SK_IGNORE_TO_STRING
+    void toString(SkString* str) const override {
+        str->append("GetSaveCountAssertLooper:");
+    }
+#endif
+
+    SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(GetSaveCountAssertLooper);
+
+private:
+    class GetSaveCountAssertLooperContext : public SkDrawLooper::Context {
+    public:
+        GetSaveCountAssertLooperContext() : fOnce(0) {}
+        bool next(SkCanvas* canvas, SkPaint* p) override {
+            // Getting the save count would assert in SkCanvas at the time of writing.
+            canvas->getSaveCount();
+
+            SkASSERT(p->getColor() == SK_ColorRED);
+            // Set the color green so the test knows payload was run. We use this color in order to
+            // try to express the expectation that Skia can not away the color filter. Due to
+            // non-pm-to-pm-to-non-pm conversions, this probably is not exactly correct.
+            p->setColor(SkColorSetARGB(255, 0, 254, 0));
+            return fOnce++ < 1;
+        }
+    private:
+        int fOnce;
+    };
+};
+
+SkFlattenable* GetSaveCountAssertLooper::CreateProc(SkReadBuffer&) {
+    return SkNEW(GetSaveCountAssertLooper);
+}
+
+}
+
+DEF_TEST(SkCanvas_GetSaveCountInDrawLooperAssert, reporter) {
+    SkBitmap dst;
+    dst.allocN32Pixels(10, 10);
+    dst.eraseColor(SK_ColorTRANSPARENT);
+
+    SkCanvas canvas(dst);
+    SkPaint  paint;
+    {
+        SkAutoTUnref<SkColorFilter> addGreenCF(
+            SkColorFilter::CreateModeFilter(SkColorSetARGB(64, 0, 4, 0), SkXfermode::kPlus_Mode));
+        SkAutoTUnref<SkImageFilter> addGreenIF(
+            SkColorFilterImageFilter::Create(addGreenCF));
+
+        // This would trigger the assert upon a draw. It is a color filter that adds (roughly) 1 to
+        // the green component.
+        paint.setImageFilter(addGreenIF);
+
+        SkAutoTUnref<SkDrawLooper> looper(SkNEW(GetSaveCountAssertLooper));
+        paint.setLooper(looper);
+    }
+    paint.setColor(SK_ColorRED);
+    paint.setStrokeWidth(1);
+    paint.setStyle(SkPaint::kStroke_Style);
+    canvas.drawPoint(0, 0, paint);
+
+    uint32_t pixel = 0;
+    SkImageInfo info = SkImageInfo::Make(1, 1, kBGRA_8888_SkColorType, kUnpremul_SkAlphaType);
+    canvas.readPixels(info, &pixel, 4, 0, 0);
+    REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+}