support scaledimagecache instantiable using discardablememory

support scaledimagecache instantiable using discardablememory

Add this to your build/gyp system to use discardable instead of malloc+budget

#define SK_USE_DISCARDABLE_SCALEDIMAGECACHE

R=halcanary@google.com

Committed: https://code.google.com/p/skia/source/detail?r=12588

[hal.canary, 2013-12-09 18:23:27]

needs tests.

Will Chrome need to switch between DM allocators at runtime or always use
SkDiscardableMemory::Create?

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
File src/core/SkScaledImageCache.cpp (right):

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:18: #   define
SK_DISCARDABLEMEMORY_SCALEDIMAGECACHE_COUNT_LIMIT   1024
I assume you expect clients to set this number larger.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:204: virtual Factory getFactory() const
SK_OVERRIDE { return NULL; }
Use SK_DECLARE_UNFLATTENABLE_OBJECT, since that is your intent.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:210: size_t               fRB;
readability: I'd prefer fRowBytes

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:255: fFactory = factory;
Either assert != NULL or explicitly allow NULL (default to
SkDiscardableMemory::Create).   I like having a sensible default.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:271: SkDiscardableMemory* dm = fFactory(size);
    SkDiscardableMemory* dm;
    if (fFactory != NULL) {
        dm = fFactory(size);
    } else {
        dm = SkDiscardableMemory::Create(size);
    }

[reed1, 2013-12-09 18:43:48]

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
File src/core/SkScaledImageCache.cpp (right):

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:18: #   define
SK_DISCARDABLEMEMORY_SCALEDIMAGECACHE_COUNT_LIMIT   1024
On 2013/12/09 18:23:27, Hal Canary wrote:
> I assume you expect clients to set this number larger.

Or smaller

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:204: virtual Factory getFactory() const
SK_OVERRIDE { return NULL; }
On 2013/12/09 18:23:27, Hal Canary wrote:
> Use SK_DECLARE_UNFLATTENABLE_OBJECT, since that is your intent.

Done.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:210: size_t               fRB;
On 2013/12/09 18:23:27, Hal Canary wrote:
> readability: I'd prefer fRowBytes

Will conflict with the SkPixelRef when that CL lands, and all subclasses in that
CL use fRB for this clarity-purpose.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:255: fFactory = factory;
On 2013/12/09 18:23:27, Hal Canary wrote:
> Either assert != NULL or explicitly allow NULL (default to
> SkDiscardableMemory::Create).   I like having a sensible default.

Defaults are harder to read sometimes, and I don't see the value in this case,
since it is a private api. I will add the assert.

https://codereview.chromium.org/105933003/diff/20001/src/core/SkScaledImageCa...
src/core/SkScaledImageCache.cpp:271: SkDiscardableMemory* dm = fFactory(size);
On 2013/12/09 18:23:27, Hal Canary wrote:
>     SkDiscardableMemory* dm;
>     if (fFactory != NULL) {
>         dm = fFactory(size);
>     } else {
>         dm = SkDiscardableMemory::Create(size);
>     }

asserting in constructor.

[reed1, 2013-12-09 20:09:25]

ptal

[hal.canary, 2013-12-09 20:21:14]

It looks good so far, but I think ImageCacheTest needs to be modified to test
the new code.

[reed1, 2013-12-09 21:54:42]

test added -- ptal

[hal.canary, 2013-12-09 22:00:20]

lgtm

[reed1, 2013-12-09 22:29:36]

Committed patchset #5 manually as r12588 (presubmit successful).