Allow custom resources in the GrContext's cache

Allow custom resources in the GrContext's cache

Adds methods to GrContext for client code to store custom resources in
the cache.

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=14577

[bsalomon, 2014-04-30 20:19:52]

https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:906: * unref().
I think this has to ref the object to be safe. What if it gets dropped from the
cache after the find? Won't the caller be holding a dead ptr?

[Chris Dalton, 2014-04-30 21:11:29]

https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:906: * unref().
On 2014/04/30 20:19:52, bsalomon wrote:
> I think this has to ref the object to be safe. What if it gets dropped from
the
> cache after the find? Won't the caller be holding a dead ptr?

Right, that wording is confusing come to think of it. The usage I intended was
that findCachedResource doesn't ref the object, and if the caller plans to use
it at a future time, they are responsible to ref it. (That's the same behavior
as GrResourceCache::find.)

Does that seem reasonable, or would you rather ref the object for this method?

[bsalomon, 2014-05-01 13:03:16]

On 2014/04/30 21:11:29, Chris Dalton wrote:
> https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h
> File include/gpu/GrContext.h (right):
> 
>
https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h#newc...
> include/gpu/GrContext.h:906: * unref().
> On 2014/04/30 20:19:52, bsalomon wrote:
> > I think this has to ref the object to be safe. What if it gets dropped from
> the
> > cache after the find? Won't the caller be holding a dead ptr?
> 
> Right, that wording is confusing come to think of it. The usage I intended was
> that findCachedResource doesn't ref the object, and if the caller plans to use
> it at a future time, they are responsible to ref it. (That's the same behavior
> as GrResourceCache::find.)
> 
> Does that seem reasonable, or would you rather ref the object for this method?

I think we should ref it (findAndRefCachedResource()). It seems to easy to
introduce a hard to reproduce bug otherwise.

[Chris Dalton, 2014-05-01 19:17:43]

On 2014/05/01 13:03:16, bsalomon wrote:
> On 2014/04/30 21:11:29, Chris Dalton wrote:
> > https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h
> > File include/gpu/GrContext.h (right):
> > 
> >
>
https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h#newc...
> > include/gpu/GrContext.h:906: * unref().
> > On 2014/04/30 20:19:52, bsalomon wrote:
> > > I think this has to ref the object to be safe. What if it gets dropped
from
> > the
> > > cache after the find? Won't the caller be holding a dead ptr?
> > 
> > Right, that wording is confusing come to think of it. The usage I intended
was
> > that findCachedResource doesn't ref the object, and if the caller plans to
use
> > it at a future time, they are responsible to ref it. (That's the same
behavior
> > as GrResourceCache::find.)
> > 
> > Does that seem reasonable, or would you rather ref the object for this
method?
> 
> I think we should ref it (findAndRefCachedResource()). It seems to easy to
> introduce a hard to reproduce bug otherwise.

Agreed. New patch uploaded.

[bsalomon, 2014-05-01 20:31:38]

On 2014/05/01 19:17:43, Chris Dalton wrote:
> On 2014/05/01 13:03:16, bsalomon wrote:
> > On 2014/04/30 21:11:29, Chris Dalton wrote:
> > > https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h
> > > File include/gpu/GrContext.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/261593009/diff/1/include/gpu/GrContext.h#newc...
> > > include/gpu/GrContext.h:906: * unref().
> > > On 2014/04/30 20:19:52, bsalomon wrote:
> > > > I think this has to ref the object to be safe. What if it gets dropped
> from
> > > the
> > > > cache after the find? Won't the caller be holding a dead ptr?
> > > 
> > > Right, that wording is confusing come to think of it. The usage I intended
> was
> > > that findCachedResource doesn't ref the object, and if the caller plans to
> use
> > > it at a future time, they are responsible to ref it. (That's the same
> behavior
> > > as GrResourceCache::find.)
> > > 
> > > Does that seem reasonable, or would you rather ref the object for this
> method?
> > 
> > I think we should ref it (findAndRefCachedResource()). It seems to easy to
> > introduce a hard to reproduce bug otherwise.
> 
> Agreed. New patch uploaded.

lgtm

[Chris Dalton, 2014-05-05 19:10:05]

The CQ bit was checked by cdalton@nvidia.com

[commit-bot: I haz the power, 2014-05-05 19:10:30]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/cdalton@nvidia.com/261593009/40001

[commit-bot: I haz the power, 2014-05-05 19:21:18]

Change committed as 14577