Guard against most unintentionally ephemeral SkAutoFoo instantiations.

I think I applied the trick everywhere possible.  Limitations:
    - can't be used with templated classes
    - all constructors and destructors must be defined inline

A couple of the SkAutoFoo were unused in Skia, Chromium, and Android, so I
deleted them.  This change caught the same bugs Cary found in SkPath, plus one
more in SampleApp.

BUG=
R=reed@google.com, caryclark@google.com

Author: mtklein@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@12301 2bbb7eff-a529-9590-31e7-b0007b416f81

22 files changed, 61 insertions, 69 deletions

diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index dd2d2cfd7f..d9749f04c4 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -730,6 +730,7 @@ private:
     const SkBitmap& fBitmap;
     bool            fDidLock;
 };
+#define SkAutoLockPixels(...) SK_REQUIRE_LOCAL_VAR(SkAutoLockPixels)
 
 /** Helper class that performs the lock/unlockColors calls on a colortable.
     The destructor will call unlockColors(false) if it has a bitmap's colortable
@@ -783,6 +784,7 @@ private:
     SkColorTable*    fCTable;
     const SkPMColor* fColors;
 };
+#define SkAutoLockColors(...) SK_REQUIRE_LOCAL_VAR(SkAutoLockColors)
 
 ///////////////////////////////////////////////////////////////////////////////
 
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index c963edcecc..d831a5c11f 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1158,6 +1158,7 @@ private:
     SkCanvas*   fCanvas;
     int         fSaveCount;
 };
+#define SkAutoCanvasRestore(...) SK_REQUIRE_LOCAL_VAR(SkAutoCanvasRestore)
 
 /** Stack helper class to automatically open and close a comment block
  */
@@ -1179,5 +1180,6 @@ public:
 private:
     SkCanvas* fCanvas;
 };
+#define SkAutoCommentBlock(...) SK_REQUIRE_LOCAL_VAR(SkAutoCommentBlock)
 
 #endif
diff --git a/include/core/SkMask.h b/include/core/SkMask.h
index 71551842bb..5cfef970c5 100644
--- a/include/core/SkMask.h
+++ b/include/core/SkMask.h
@@ -157,5 +157,6 @@ public:
 private:
     uint8_t* fImage;
 };
+#define SkAutoMaskFreeImage(...) SK_REQUIRE_LOCAL_VAR(SkAutoMaskFreeImage)
 
 #endif
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index fa1cc4954d..e371c4369d 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -253,26 +253,6 @@ private:
     typedef SkRefCnt INHERITED;
 };
 
-class SkAutoPictureRecord : SkNoncopyable {
-public:
-    SkAutoPictureRecord(SkPicture* pict, int width, int height,
-                        uint32_t recordingFlags = 0) {
-        fPicture = pict;
-        fCanvas = pict->beginRecording(width, height, recordingFlags);
-    }
-    ~SkAutoPictureRecord() {
-        fPicture->endRecording();
-    }
-
-    /** Return the canvas to draw into for recording into the picture.
-    */
-    SkCanvas* getRecordingCanvas() const { return fCanvas; }
-
-private:
-    SkPicture*  fPicture;
-    SkCanvas*   fCanvas;
-};
-
 /**
  *  Subclasses of this can be passed to canvas.drawPicture. During the drawing
  *  of the picture, this callback will periodically be invoked. If its
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index 99df0c1256..b010faf760 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -237,11 +237,13 @@ public:
 private:
     T*  fObj;
 };
+// Can't use the #define trick below to guard a bare SkAutoTUnref(...) because it's templated. :(
 
 class SkAutoUnref : public SkAutoTUnref<SkRefCnt> {
 public:
     SkAutoUnref(SkRefCnt* obj) : SkAutoTUnref<SkRefCnt>(obj) {}
 };
+#define SkAutoUnref(...) SK_REQUIRE_LOCAL_VAR(SkAutoUnref)
 
 class SkAutoRef : SkNoncopyable {
 public:
@@ -250,6 +252,7 @@ public:
 private:
     SkRefCnt* fObj;
 };
+#define SkAutoRef(...) SK_REQUIRE_LOCAL_VAR(SkAutoRef)
 
 /** Wrapper class for SkRefCnt pointers. This manages ref/unref of a pointer to
     a SkRefCnt (or subclass) object.
diff --git a/include/core/SkString.h b/include/core/SkString.h
index 5d975330f7..5629cc0464 100644
--- a/include/core/SkString.h
+++ b/include/core/SkString.h
@@ -235,24 +235,6 @@ private:
     static Rec* RefRec(Rec*);
 };
 
-class SkAutoUCS2 {
-public:
-    SkAutoUCS2(const char utf8[]);
-    ~SkAutoUCS2();
-
-    /** This returns the number of ucs2 characters
-    */
-    int count() const { return fCount; }
-
-    /** This returns a null terminated ucs2 string
-    */
-    const uint16_t* getUCS2() const { return fUCS2; }
-
-private:
-    int         fCount;
-    uint16_t*   fUCS2;
-};
-
 /// Creates a new string and writes into it using a printf()-style format.
 SkString SkStringPrintf(const char* format, ...);
 
diff --git a/include/core/SkThread.h b/include/core/SkThread.h
index 4a2499a203..487c2bdf9e 100644
--- a/include/core/SkThread.h
+++ b/include/core/SkThread.h
@@ -64,5 +64,6 @@ public:
 private:
     SkBaseMutex* fMutex;
 };
+#define SkAutoMutexAcquire(...) SK_REQUIRE_LOCAL_VAR(SkAutoMutexAcquire)
 
 #endif
diff --git a/include/core/SkTime.h b/include/core/SkTime.h
index 7f3c27053e..51616d41c7 100644
--- a/include/core/SkTime.h
+++ b/include/core/SkTime.h
@@ -60,5 +60,6 @@ private:
     SkMSec      fNow;
     SkMSec      fMinToDump;
 };
+#define SkAutoTime(...) SK_REQUIRE_LOCAL_VAR(SkAutoTime)
 
 #endif
diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 219e51fbe9..c30a8a2d66 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -149,6 +149,32 @@ struct SkCompileAssert {
  */
 #define SK_MACRO_APPEND_LINE(name)  SK_MACRO_CONCAT(name, __LINE__)
 
+/**
+ * For some classes, it's almost always an error to instantiate one without a name, e.g.
+ *   {
+ *       SkAutoMutexAcquire(&mutex);
+ *       <some code>
+ *   }
+ * In this case, the writer meant to hold mutex while the rest of the code in the block runs,
+ * but instead the mutex is acquired and then immediately released.  The correct usage is
+ *   {
+ *       SkAutoMutexAcquire lock(&mutex);
+ *       <some code>
+ *   }
+ *
+ * To prevent callers from instantiating your class without a name, use SK_REQUIRE_LOCAL_VAR
+ * like this:
+ *   class classname {
+ *       <your class>
+ *   };
+ *   #define classname(...) SK_REQUIRE_LOCAL_VAR(classname)
+ *
+ * This won't work with templates, and you must inline the class' constructors and destructors.
+ * Take a look at SkAutoFree and SkAutoMalloc in this file for examples.
+ */
+#define SK_REQUIRE_LOCAL_VAR(classname) \
+    SK_COMPILE_ASSERT(false, missing_name_for_##classname)
+
 ///////////////////////////////////////////////////////////////////////
 
 /**
@@ -432,6 +458,7 @@ private:
     SkAutoFree(const SkAutoFree&);
     SkAutoFree& operator=(const SkAutoFree&);
 };
+#define SkAutoFree(...) SK_REQUIRE_LOCAL_VAR(SkAutoFree)
 
 /**
  *  Manage an allocated block of heap memory. This object is the sole manager of
@@ -518,6 +545,7 @@ private:
     void*   fPtr;
     size_t  fSize;  // can be larger than the requested size (see kReuse)
 };
+#define SkAutoMalloc(...) SK_REQUIRE_LOCAL_VAR(SkAutoMalloc)
 
 /**
  *  Manage an allocated block of memory. If the requested size is <= kSize, then
@@ -604,6 +632,7 @@ private:
     size_t      fSize;  // can be larger than the requested size (see kReuse)
     uint32_t    fStorage[(kSize + 3) >> 2];
 };
+// Can't guard the constructor because it's a template class.
 
 #endif /* C++ */
 
diff --git a/include/core/SkUtils.h b/include/core/SkUtils.h
index e29367dc22..c2dcad950f 100644
--- a/include/core/SkUtils.h
+++ b/include/core/SkUtils.h
@@ -113,5 +113,6 @@ public:
 private:
     const char* fLabel;
 };
+#define SkAutoTrace(...) SK_REQUIRE_LOCAL_VAR(SkAutoTrace)
 
 #endif
diff --git a/samplecode/SampleLayers.cpp b/samplecode/SampleLayers.cpp
index f72c519faf..483943d5d4 100644
--- a/samplecode/SampleLayers.cpp
+++ b/samplecode/SampleLayers.cpp
@@ -61,7 +61,7 @@ static void test_fade(SkCanvas* canvas) {
     SkPaint p;
     p.setAlpha(0x88);
 
-    SkAutoCanvasRestore(canvas, false);
+    SkAutoCanvasRestore ar2(canvas, false);
 
     // create the layers
 
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 7012cf3141..dc7946a4bd 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -780,6 +780,7 @@ private:
     void*   fObj;
     Proc    fProc;
 };
+#define SkAutoCallProc(...) SK_REQUIRE_LOCAL_VAR(SkAutoCallProc)
 
 static void destroy_blitter(void* blitter) {
     ((SkBlitter*)blitter)->~SkBlitter();
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 2efea724da..0d5fccb91f 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -459,6 +459,7 @@ public:
 private:
     SkBounder*  fBounder;
 };
+#define SkAutoBounderCommit(...) SK_REQUIRE_LOCAL_VAR(SkAutoBounderCommit)
 
 #include "SkColorPriv.h"
 
diff --git a/src/core/SkComposeShader.cpp b/src/core/SkComposeShader.cpp
index 206608a4e0..0d2d68717f 100644
--- a/src/core/SkComposeShader.cpp
+++ b/src/core/SkComposeShader.cpp
@@ -59,6 +59,7 @@ private:
     SkPaint*    fPaint;
     uint8_t     fAlpha;
 };
+#define SkAutoAlphaRestore(...) SK_REQUIRE_LOCAL_VAR(SkAutoAlphaRestore)
 
 void SkComposeShader::flatten(SkFlattenableWriteBuffer& buffer) const {
     this->INHERITED::flatten(buffer);
diff --git a/src/core/SkDescriptor.h b/src/core/SkDescriptor.h
index 79b086f614..e71ff41b5c 100644
--- a/src/core/SkDescriptor.h
+++ b/src/core/SkDescriptor.h
@@ -159,6 +159,7 @@ private:
     SkDescriptor*   fDesc;
     uint32_t        fStorage[(kStorageSize + 3) >> 2];
 };
+#define SkAutoDescriptor(...) SK_REQUIRE_LOCAL_VAR(SkAutoDescriptor)
 
 
 #endif
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index 112451920f..aa91df3df5 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -65,7 +65,13 @@ public:
                                      fStorage, sizeof(fStorage), drawCoverage);
     }
 
-    ~SkAutoBlitterChoose();
+    ~SkAutoBlitterChoose() {
+        if ((void*)fBlitter == (void*)fStorage) {
+            fBlitter->~SkBlitter();
+        } else {
+            SkDELETE(fBlitter);
+        }
+    }
 
     SkBlitter*  operator->() { return fBlitter; }
     SkBlitter*  get() const { return fBlitter; }
@@ -81,14 +87,7 @@ private:
     SkBlitter*  fBlitter;
     uint32_t    fStorage[kBlitterStorageLongCount];
 };
-
-SkAutoBlitterChoose::~SkAutoBlitterChoose() {
-    if ((void*)fBlitter == (void*)fStorage) {
-        fBlitter->~SkBlitter();
-    } else {
-        SkDELETE(fBlitter);
-    }
-}
+#define SkAutoBlitterChoose(...) SK_REQUIRE_LOCAL_VAR(SkAutoBlitterChoose)
 
 /**
  *  Since we are providing the storage for the shader (to avoid the perf cost
@@ -128,6 +127,7 @@ private:
     SkPaint     fPaint; // copy of caller's paint (which we then modify)
     uint32_t    fStorage[kBlitterStorageLongCount];
 };
+#define SkAutoBitmapShaderInstall(...) SK_REQUIRE_LOCAL_VAR(SkAutoBitmapShaderInstall)
 
 ///////////////////////////////////////////////////////////////////////////////
 
diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h
index 8447337374..52a8132f72 100644
--- a/src/core/SkGlyphCache.h
+++ b/src/core/SkGlyphCache.h
@@ -273,5 +273,6 @@ private:
 
     static bool DetachProc(const SkGlyphCache*, void*);
 };
+#define SkAutoGlyphCache(...) SK_REQUIRE_LOCAL_VAR(SkAutoGlyphCache)
 
 #endif
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 5f53ce8915..d25ec3c1df 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -56,6 +56,7 @@ private:
     SkPath* fPath;
     bool    fSaved;
 };
+#define SkAutoDisableOvalCheck(...) SK_REQUIRE_LOCAL_VAR(SkAutoDisableOvalCheck)
 
 class SkAutoDisableDirectionCheck {
 public:
@@ -71,6 +72,7 @@ private:
     SkPath*              fPath;
     SkPath::Direction    fSaved;
 };
+#define SkAutoDisableDirectionCheck(...) SK_REQUIRE_LOCAL_VAR(SkAutoDisableDirectionCheck)
 
 /*  This guy's constructor/destructor bracket a path editing operation. It is
     used when we know the bounds of the amount we are going to add to the path
@@ -125,6 +127,7 @@ private:
         fDegenerate = is_degenerate(*path);
     }
 };
+#define SkAutoPathBoundsUpdate(...) SK_REQUIRE_LOCAL_VAR(SkAutoPathBoundsUpdate)
 
 ////////////////////////////////////////////////////////////////////////////
 
diff --git a/src/core/SkRasterClip.h b/src/core/SkRasterClip.h
index e58a23b38b..0c2723314c 100644
--- a/src/core/SkRasterClip.h
+++ b/src/core/SkRasterClip.h
@@ -112,6 +112,7 @@ public:
 private:
     const SkRasterClip& fRC;
 };
+#define SkAutoRasterClipValidate(...) SK_REQUIRE_LOCAL_VAR(SkAutoRasterClipValidate)
 
 #ifdef SK_DEBUG
     #define AUTO_RASTERCLIP_VALIDATE(rc)    SkAutoRasterClipValidate arcv(rc)
diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 4e5e204e8c..7b3c265314 100644
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -626,27 +626,6 @@ void SkString::swap(SkString& other) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkAutoUCS2::SkAutoUCS2(const char utf8[]) {
-    size_t len = strlen(utf8);
-    fUCS2 = (uint16_t*)sk_malloc_throw((len + 1) * sizeof(uint16_t));
-
-    uint16_t* dst = fUCS2;
-    for (;;) {
-        SkUnichar uni = SkUTF8_NextUnichar(&utf8);
-        *dst++ = SkToU16(uni);
-        if (uni == 0) {
-            break;
-        }
-    }
-    fCount = (int)(dst - fUCS2);
-}
-
-SkAutoUCS2::~SkAutoUCS2() {
-    sk_free(fUCS2);
-}
-
-///////////////////////////////////////////////////////////////////////////////
-
 SkString SkStringPrintf(const char* format, ...) {
     SkString formattedOutput;
     char buffer[kBufferSize];
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index be762eead3..7416cb22fa 100755
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -2153,6 +2153,7 @@ private:
     HFONT fFont;
     HFONT fSavefont;
 };
+#define SkAutoHDC(...) SK_REQUIRE_LOCAL_VAR(SkAutoHDC)
 
 int LogFontTypeface::onCharsToGlyphs(const void* chars, Encoding encoding,
                                      uint16_t userGlyphs[], int glyphCount) const
diff --git a/src/utils/mac/SkCreateCGImageRef.cpp b/src/utils/mac/SkCreateCGImageRef.cpp
index e931901b39..0677b7bc30 100644
--- a/src/utils/mac/SkCreateCGImageRef.cpp
+++ b/src/utils/mac/SkCreateCGImageRef.cpp
@@ -174,6 +174,7 @@ public:
 private:
     CGPDFDocumentRef fDoc;
 };
+#define SkAutoPDFRelease(...) SK_REQUIRE_LOCAL_VAR(SkAutoPDFRelease)
 
 static void CGDataProviderReleaseData_FromMalloc(void*, const void* data,
                                                  size_t size) {