[WIP] Add Context to SkDrawLooper.

src/core/SkTemplatesPriv.h

Index: src/core/SkTemplatesPriv.h
diff --git a/src/core/SkTemplatesPriv.h b/src/core/SkTemplatesPriv.h
index 79ae6093358993917df7cf3261ef4be93a6063e8..c7d61da7cf80e917e5708774b8f43c22763725db 100644
--- a/src/core/SkTemplatesPriv.h
+++ b/src/core/SkTemplatesPriv.h
@@ -20,6 +20,12 @@
 
     #define SK_PLACEMENT_NEW_ARGS(result, classname, storage, storageSize, args)    \
         result = SkNEW_ARGS(classname, args)
+
+    #define SK_PLACEMENT_SAFE_NEW(result, classname, storage, storageSize)   \
+        result = SkNEW(classname)

[scroggo, 2014/02/11 15:51:29]

Yikes! This will result in a memory leak if the caller is not careful. Would it
make sense to have some kind of SK_PLACEMENT_SAFE_DELETE that handles the delete
the way you do your delete?

I see this is already what we do on WIN32 for normal placement new (which I
cannot imagine the caller accounting for). Does placement new not work on WIN32?

(The history of SK_PLACEMENT_NEW on WIN32 is lost to time - it was pulled from
Android in 2008.)

[Dominik Grewe, 2014/02/11 16:04:31]

I guess the same is true for the normal placement method. No matter which one is
used, you always have to be careful about freeing memory.

Having SK_PLACEMENT_DELETE would make sense; I don't think we need a special
SK_PLACEMENT_SAFE_DELETE but we could add it for "symmetry".
Tbh, I'm not sure why we need this special case for WIN32, I just assumed we do
because that's what we do the normal placement functions. I can have a look if
it's still needed.

[scroggo, 2014/02/11 16:14:26]

Well, this one is actually safer than the normal placement method. When calling
this, you (should) know that you have to special case the delete, but if someone
called SK_PLACEMENT_NEW I would expect them to call ~T() instead of delete,
which would be the appropriate place everywhere except WIN32, where we'll leak
the memory. (Sorry to derail your CL a little bit - this problem wasn't
introduced by you!)

On another note, what about using SkAutoSMalloc (in SkTypes.h)? This class
handles choosing whether to use malloc and free (although you then still have to
do the SK_PLACEMENT_NEW yourself).

+
+    #define SK_PLACEMENT_SAFE_NEW_ARGS(result, classname, storage, storageSize, args)    \
+        result = SkNEW_ARGS(classname, args)
 #else
     #include <new>
     #define SK_PLACEMENT_NEW(result, classname, storage, storagesize)       \

[bungeman-skia, 2014/02/11 17:06:21]

Every current user of this does the same dance where they have to do a check to
see what result was and either delete or destroy. The name is also entirely
misleading, as this obviously isn't interchangeable with SkNEW_PLACEMENT. In
other words, the name is terrible and doesn't do what it says it does. Let's not
perpetuate that.

@@ -43,6 +49,22 @@
         else                                                                            \
             result = SkNEW_ARGS(classname, args);                                       \
     } while (0)
+
+    #define SK_PLACEMENT_SAFE_NEW(result, classname, storage, storagesize)  \

[bungeman-skia, 2014/02/11 17:06:21]

Why are we multiplying entities without necessity here? Why the new name? Why
not just replace the content of the SK_PLACEMENT_NEW with these new bodies?
Comparing against the constant sizeof or against 0 here seems a rather small
difference in performance, if that's what we're worrying about.

+    do {                                                                    \
+        if (storagesize >= sizeof(classname))                               \
+            result = new(storage) classname;                                \
+        else                                                                \
+            result = SkNEW(classname);                                      \
+    } while (0)
+
+    #define SK_PLACEMENT_SAFE_NEW_ARGS(result, classname, storage, storagesize, args)   \
+    do {                                                                                \
+        if (storagesize >= sizeof(classname))                                           \
+            result = new(storage) classname args;                                       \
+        else                                                                            \
+            result = SkNEW_ARGS(classname, args);                                       \
+    } while (0)
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////