Use GrResourceCache2 to service content key lookups

Use GrResourceCache2 to service content key lookups

BUG=skia:2889
Committed: https://skia.googlesource.com/skia/+/8b79d23f825cd9e0f9e3bf8aaa9e209940b17ef6

[bsalomon, 2014-11-07 19:26:32]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2014-11-07 19:26:45]

Patchset #1 (id:1) has been deleted

[bsalomon, 2014-11-07 19:26:52]

Patchset #1 (id:20001) has been deleted

[bsalomon, 2014-11-07 19:26:59]

Patchset #1 (id:40001) has been deleted

[bsalomon, 2014-11-07 19:27:08]

Patchset #2 (id:80001) has been deleted

[bsalomon, 2014-11-07 19:27:14]

Patchset #1 (id:60001) has been deleted

[robertphillips, 2014-11-07 20:32:13]

Painful but lgtm + some dox suggestions & questions.

https://codereview.chromium.org/707493002/diff/120001/include/gpu/GrGpuResour...
File include/gpu/GrGpuResource.h (right):

https://codereview.chromium.org/707493002/diff/120001/include/gpu/GrGpuResour...
include/gpu/GrGpuResource.h:174: 
// Returns true if the cache entry was successfully set; false otherwise.

?

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp#n...
src/gpu/GrContext.cpp:258: 
Call findAndRefCachedResource here similar to how findAndRefStencilBuffer works
?

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrGpuResource.cpp
File src/gpu/GrGpuResource.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrGpuResource.c...
src/gpu/GrGpuResource.cpp:89: 
Delete this next line ?

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:251: 
This comment doesn't seem right.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:271: 
// Invalidate 2 of the 3 ?

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:283: 
// Finally invalidate the third. ?

[bsalomon, 2014-11-07 21:48:44]

https://codereview.chromium.org/707493002/diff/120001/include/gpu/GrGpuResour...
File include/gpu/GrGpuResource.h (right):

https://codereview.chromium.org/707493002/diff/120001/include/gpu/GrGpuResour...
include/gpu/GrGpuResource.h:174: 
On 2014/11/07 20:32:12, robertphillips wrote:
> // Returns true if the cache entry was successfully set; false otherwise.
> 
> ?

The bool is removed in the next CL :)

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp#n...
src/gpu/GrContext.cpp:258: 
On 2014/11/07 20:32:12, robertphillips wrote:
> Call findAndRefCachedResource here similar to how findAndRefStencilBuffer
works
> ?

I think I got what you mean, can you check?

BTW I hope to soon remove findAndRefStencilBuffer and force all SBs to be
scratch without the special functionality.

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrGpuResource.cpp
File src/gpu/GrGpuResource.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrGpuResource.c...
src/gpu/GrGpuResource.cpp:89: 
On 2014/11/07 20:32:13, robertphillips wrote:
> Delete this next line ?

Done.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:251: 
On 2014/11/07 20:32:13, robertphillips wrote:
> This comment doesn't seem right.

Don't they delete each other (setDeleteWhenDestroyed below)? I guess since I no
longer allow multiple resources with the same key this aspect isn't interesting.
I'll remove the setDeleteWhenDestroyed() business.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:271: 
On 2014/11/07 20:32:13, robertphillips wrote:
> // Invalidate 2 of the 3 ?

Done.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:283: 
On 2014/11/07 20:32:13, robertphillips wrote:
> // Finally invalidate the third. ?

Done.

[robertphillips, 2014-11-10 13:28:26]

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/src/gpu/GrContext.cpp#n...
src/gpu/GrContext.cpp:258: 
On 2014/11/07 21:48:44, bsalomon wrote:
> On 2014/11/07 20:32:12, robertphillips wrote:
> > Call findAndRefCachedResource here similar to how findAndRefStencilBuffer
> works
> > ?
> 
> I think I got what you mean, can you check?
> 
> BTW I hope to soon remove findAndRefStencilBuffer and force all SBs to be
> scratch without the special functionality.

lgtm

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:137: 
Here.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:199: 
Here.

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:251: 
> Don't they delete each other (setDeleteWhenDestroyed below)? I guess since I
no
> longer allow multiple resources with the same key this aspect isn't
interesting.
> I'll remove the setDeleteWhenDestroyed() business.

Ooops - I put that comment on the wrong line - please see the other two
instances.

[bsalomon, 2014-11-10 14:14:58]

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/707493002/diff/120001/tests/ResourceCacheTest...
tests/ResourceCacheTest.cpp:251: 
On 2014/11/10 13:28:26, robertphillips wrote:
> > Don't they delete each other (setDeleteWhenDestroyed below)? I guess since I
> no
> > longer allow multiple resources with the same key this aspect isn't
> interesting.
> > I'll remove the setDeleteWhenDestroyed() business.
> 
> Ooops - I put that comment on the wrong line - please see the other two
> instances.

Gotcha, will update before submitting.

[bsalomon, 2014-11-10 16:14:24]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-10 16:14:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707493002/160001

[commit-bot: I haz the power, 2014-11-10 16:17:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-10 16:17:02]

Try jobs failed on following builders:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-Cl...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[bsalomon, 2014-11-10 18:00:08]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2014-11-10 18:00:11]

The CQ bit was unchecked by bsalomon@google.com

[bsalomon, 2014-11-10 18:00:22]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-10 18:00:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707493002/200001

[commit-bot: I haz the power, 2014-11-10 18:03:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-10 18:03:00]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)

[bsalomon, 2014-11-10 18:10:25]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-10 18:11:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707493002/220001

[commit-bot: I haz the power, 2014-11-10 18:19:10]

Committed patchset #7 (id:220001) as
https://skia.googlesource.com/skia/+/8b79d23f825cd9e0f9e3bf8aaa9e209940b17ef6