SkImage_Raster's pixels are always immutable.

To make this work, we tag their pixelrefs as temporarily immutable, allowing
ourselves to restore the pixels to mutability only when the image drops away.

This should allow us to wobble back and forth between writing to the Surface
and reading from the Image without a COW, with the Surface seeing mutable
pixels and the Image seeing immutable pixels.

The big idea is, Image doesn't need forever-immutable pixels, it just needs
pixels that are immutable as long as it's alive.

BUG=skia:

patch from issue 804523002 at patchset 40001 (http://crrev.com/804523002#ps40001)

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

7 files changed, 66 insertions, 17 deletions

diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 6d36915d9a..56bcc9a792 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -106,7 +106,7 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info)
     fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
-    fIsImmutable = false;
+    fMutability = kMutable;
     fPreLocked = false;
     fAddedToCache.store(false);
 }
@@ -125,7 +125,7 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex)
     fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
-    fIsImmutable = false;
+    fMutability = kMutable;
     fPreLocked = false;
     fAddedToCache.store(false);
 }
@@ -341,7 +341,7 @@ void SkPixelRef::callGenIDChangeListeners() {
 
 void SkPixelRef::notifyPixelsChanged() {
 #ifdef SK_DEBUG
-    if (fIsImmutable) {
+    if (this->isImmutable()) {
         SkDebugf("========== notifyPixelsChanged called on immutable pixelref");
     }
 #endif
@@ -355,7 +355,17 @@ void SkPixelRef::changeAlphaType(SkAlphaType at) {
 }
 
 void SkPixelRef::setImmutable() {
-    fIsImmutable = true;
+    fMutability = kImmutable;
+}
+void SkPixelRef::setTemporarilyImmutable() {
+    SkASSERT(fMutability != kImmutable);
+    fMutability = kTemporarilyImmutable;
+}
+
+void SkPixelRef::restoreMutability() {
+    SkASSERT(fMutability != kImmutable);
+    fMutability = kMutable;
+    this->notifyPixelsChanged();  // This is just precautionary.
 }
 
 bool SkPixelRef::readPixels(SkBitmap* dst, const SkIRect* subset) {
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index cde9d76172..081785f4b3 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -254,7 +254,7 @@ SkImage* SkImage::NewFromBitmap(const SkBitmap& bm) {
 #endif
 
     // This will check for immutable (share or copy)
-    return SkNewImageFromRasterBitmap(bm, false, nullptr, kUnlocked_SharedPixelRefMode);
+    return SkNewImageFromRasterBitmap(bm, nullptr, kUnlocked_SharedPixelRefMode);
 }
 
 bool SkImage::asLegacyBitmap(SkBitmap* bitmap, LegacyBitmapMode mode) const {
diff --git a/src/image/SkImagePriv.h b/src/image/SkImagePriv.h
index 876c4279b6..0626d6a3ce 100644
--- a/src/image/SkImagePriv.h
+++ b/src/image/SkImagePriv.h
@@ -39,8 +39,12 @@ enum SharedPixelRefMode {
     kLocked_SharedPixelRefMode,
     kUnlocked_SharedPixelRefMode
 };
-extern SkImage* SkNewImageFromRasterBitmap(const SkBitmap&, bool forceSharePixelRef,
-                                           const SkSurfaceProps*, SharedPixelRefMode);
+enum ForceCopyMode {
+    kNo_ForceCopyMode,
+    kYes_ForceCopyMode, // must copy the pixels even if the bitmap is immutable
+};
+extern SkImage* SkNewImageFromRasterBitmap(const SkBitmap&, const SkSurfaceProps*,
+                                           SharedPixelRefMode, ForceCopyMode = kNo_ForceCopyMode);
 
 static inline size_t SkImageMinRowBytes(const SkImageInfo& info) {
     size_t minRB = info.minRowBytes();
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 41198c09ed..bde4c347aa 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -87,10 +87,13 @@ public:
         if (lockPixels) {
             fBitmap.lockPixels();
         }
+        SkASSERT(fBitmap.isImmutable());
     }
 
 private:
-    SkImage_Raster() : INHERITED(0, 0, NULL) {}
+    SkImage_Raster() : INHERITED(0, 0, NULL) {
+        fBitmap.setImmutable();
+    }
 
     SkBitmap    fBitmap;
 
@@ -123,6 +126,7 @@ SkImage_Raster::SkImage_Raster(const Info& info, SkPixelRef* pr, const SkIPoint&
     fBitmap.setInfo(info, rowBytes);
     fBitmap.setPixelRef(pr, pixelRefOrigin);
     fBitmap.lockPixels();
+    SkASSERT(fBitmap.isImmutable());
 }
 
 SkImage_Raster::~SkImage_Raster() {}
@@ -232,8 +236,8 @@ SkImage* SkNewImageFromPixelRef(const SkImageInfo& info, SkPixelRef* pr,
     return SkNEW_ARGS(SkImage_Raster, (info, pr, pixelRefOrigin, rowBytes, props));
 }
 
-SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, bool forceSharePixelRef,
-                                    const SkSurfaceProps* props, SharedPixelRefMode mode) {
+SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, const SkSurfaceProps* props,
+                                    SharedPixelRefMode mode, ForceCopyMode forceCopy) {
     SkASSERT(NULL == bm.getTexture());
 
     if (!SkImage_Raster::ValidArgs(bm.info(), bm.rowBytes(), NULL, NULL)) {
@@ -241,9 +245,7 @@ SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, bool forceSharePixelRef,
     }
 
     SkImage* image = NULL;
-    if (forceSharePixelRef || bm.isImmutable()) {
-        image = SkNEW_ARGS(SkImage_Raster, (bm, props, kLocked_SharedPixelRefMode == mode));
-    } else {
+    if (kYes_ForceCopyMode == forceCopy || !bm.isImmutable()) {
         SkBitmap tmp(bm);
         tmp.lockPixels();
         if (tmp.getPixels()) {
@@ -255,6 +257,8 @@ SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, bool forceSharePixelRef,
         if (image && props) {
             as_IB(image)->initWithProps(*props);
         }
+    } else {
+        image = SkNEW_ARGS(SkImage_Raster, (bm, props, kLocked_SharedPixelRefMode == mode));
     }
     return image;
 }
diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index bfc019bd4f..8c356a8080 100644
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -99,7 +99,8 @@ void SkSurface_Base::aboutToDraw(ContentChangeMode mode) {
         // the surface may need to fork its backend, if its sharing it with
         // the cached image. Note: we only call if there is an outstanding owner
         // on the image (besides us).
-        if (!fCachedImage->unique()) {
+        bool unique = fCachedImage->unique();
+        if (!unique) {
             this->onCopyOnWrite(mode);
         }
 
@@ -107,6 +108,13 @@ void SkSurface_Base::aboutToDraw(ContentChangeMode mode) {
         // that the next request will get our new contents.
         fCachedImage->unref();
         fCachedImage = NULL;
+
+        if (unique) {
+            // Our content isn't held by any image now, so we can consider that content mutable.
+            // Raster surfaces need to be told it's safe to consider its pixels mutable again.
+            // We make this call after the ->unref() so the subclass can assert there are no images.
+            this->onRestoreBackingMutability();
+        }
     } else if (kDiscard_ContentChangeMode == mode) {
         this->onDiscard();
     }
diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h
index a52a2b44d9..8f0eef3b60 100644
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -68,9 +68,17 @@ public:
      */
     virtual void onCopyOnWrite(ContentChangeMode) = 0;
 
+    /**
+     *  Signal the surface to remind its backing store that it's mutable again.
+     *  Called only when we _didn't_ copy-on-write; we assume the copies start mutable.
+     */
+    virtual void onRestoreBackingMutability() {}
+
     inline SkCanvas* getCachedCanvas();
     inline SkImage* getCachedImage(Budgeted);
 
+    bool hasCachedImage() const { return fCachedImage != NULL; }
+
     // called by SkSurface to compute a new genID
     uint32_t newGenerationID();
 
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index d59aef7c75..d0a65530f0 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -27,6 +27,7 @@ public:
     SkImage* onNewImageSnapshot(Budgeted) override;
     void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override;
     void onCopyOnWrite(ContentChangeMode) override;
+    void onRestoreBackingMutability() override;
 
 private:
     SkBitmap    fBitmap;
@@ -118,10 +119,24 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
 }
 
 SkImage* SkSurface_Raster::onNewImageSnapshot(Budgeted) {
+    if (fWeOwnThePixels) {
+        // SkImage_raster requires these pixels are immutable for its full lifetime.
+        // We'll undo this via onRestoreBackingMutability() if we can avoid the COW.
+        if (SkPixelRef* pr = fBitmap.pixelRef()) {
+            pr->setTemporarilyImmutable();
+        }
+    }
     // Our pixels are in memory, so read access on the snapshot SkImage could be cheap.
     // Lock the shared pixel ref to ensure peekPixels() is usable.
-    return SkNewImageFromRasterBitmap(fBitmap, fWeOwnThePixels, &this->props(),
-                                      kLocked_SharedPixelRefMode);
+    return SkNewImageFromRasterBitmap(fBitmap, &this->props(), kLocked_SharedPixelRefMode,
+                                      fWeOwnThePixels ? kNo_ForceCopyMode : kYes_ForceCopyMode);
+}
+
+void SkSurface_Raster::onRestoreBackingMutability() {
+    SkASSERT(!this->hasCachedImage());  // Shouldn't be any snapshots out there.
+    if (SkPixelRef* pr = fBitmap.pixelRef()) {
+        pr->restoreMutability();
+    }
 }
 
 void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) {
@@ -158,7 +173,7 @@ SkSurface* SkSurface::NewRasterDirectReleaseProc(const SkImageInfo& info, void*
     if (NULL == pixels) {
         return NULL;
     }
-    
+
     return SkNEW_ARGS(SkSurface_Raster, (info, pixels, rb, releaseProc, context, props));
 }