Enable removing uploads to evicted textures from texture upload queues.

Allow deletion of some (as opposed to all) textures on the impl thread.  

Enable removing uploads to evicted textures from texture upload queues.  Track whether not a CCPrioritizedTexture::Backing has been evicted in the structure itself.  Purge texture upload queues of evicted backings instead of clearing the entire queue.

Added CCTextureUpdateControllerTest.ClearUploadsToEvictedResources to test the new eviction logic.

BUG=134750
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157662

[jamesr, 2012-09-19 18:56:37]

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.cpp
File cc/CCPrioritizedTexture.cpp (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.cpp#n...
cc/CCPrioritizedTexture.cpp:139: void
CCPrioritizedTexture::Backing::deleteResource(CCResourceProvider*
resourceProvider)
could we at least in debug verify that the provider used to delete the resource
is the same that was used to create it? perhaps keep a pointer around in #ifndef
NDEBUG?

why the asymmetry between creating the resource and deleting it? the manager
always creates and most of the time deletes the resource, but in this path
deletion is through the backing. would it be cleaner to leave all
CCResourceProvider interactions in the manager and have the backing be more of a
container of ids/properties?

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.h
File cc/CCPrioritizedTexture.h (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.h#new...
cc/CCPrioritizedTexture.h:55: bool backingResourceWasEvicted() const { return
m_backing ? m_backing->resourceHasBeenDeleted() : false; }
a ternary is about the place where I'd prefer putting the function definition in
the cpp

https://codereview.chromium.org/10947017/diff/1/cc/CCTextureUpdateControllerT...
File cc/CCTextureUpdateControllerTest.cpp (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCTextureUpdateControllerT...
cc/CCTextureUpdateControllerTest.cpp:70: TextureForUploadTest() :
LayerTextureUpdater::Texture(adoptPtr<CCPrioritizedTexture>(0)),
m_evicted(false) { }
one initialization per line, please. I think this should go:

TextureForUploadTest()
    : LayerTexture...()
    , m_evicted(..)
{
}

[ccameron, 2012-09-19 20:52:02]

Thanks!

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.cpp
File cc/CCPrioritizedTexture.cpp (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.cpp#n...
cc/CCPrioritizedTexture.cpp:139: void
CCPrioritizedTexture::Backing::deleteResource(CCResourceProvider*
resourceProvider)
On 2012/09/19 18:56:37, jamesr wrote:
> could we at least in debug verify that the provider used to delete the
resource
> is the same that was used to create it? perhaps keep a pointer around in
#ifndef
> NDEBUG?

Added.

> why the asymmetry between creating the resource and deleting it? the manager
> always creates and most of the time deletes the resource, but in this path
> deletion is through the backing. would it be cleaner to leave all
> CCResourceProvider interactions in the manager and have the backing be more of
a
> container of ids/properties?

It's asymmetric because we always create a backing structure and its resource at
the same time, while we will delete the backing structure and its resource at
different times (this, in turn, is because we can delete the resource while the
main thread is running, but we cannot unlink backings while the main thread is
running).  All deletion paths go through the sequence
"evictBackingsToReduceMemory calls evictBackingResource calls deleteResource" at
the end.

I played around with separating the creation into two steps to be symmetric, but
the result was more confusing to read (it then appeared to support or use
behavior that we didn't).

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.h
File cc/CCPrioritizedTexture.h (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCPrioritizedTexture.h#new...
cc/CCPrioritizedTexture.h:55: bool backingResourceWasEvicted() const { return
m_backing ? m_backing->resourceHasBeenDeleted() : false; }
On 2012/09/19 18:56:37, jamesr wrote:
> a ternary is about the place where I'd prefer putting the function definition
in
> the cpp

Fixed

https://codereview.chromium.org/10947017/diff/1/cc/CCTextureUpdateControllerT...
File cc/CCTextureUpdateControllerTest.cpp (right):

https://codereview.chromium.org/10947017/diff/1/cc/CCTextureUpdateControllerT...
cc/CCTextureUpdateControllerTest.cpp:70: TextureForUploadTest() :
LayerTextureUpdater::Texture(adoptPtr<CCPrioritizedTexture>(0)),
m_evicted(false) { }
On 2012/09/19 18:56:37, jamesr wrote:
> one initialization per line, please. I think this should go:
> 
> TextureForUploadTest()
>     : LayerTexture...()
>     , m_evicted(..)
> {
> }

Fixed.

[jamesr, 2012-09-19 20:54:37]

OK, that makes sense. Could you leave a comment in/around
Backing::deleteResource to explain what's going on? lgtm

https://codereview.chromium.org/10947017/diff/8003/cc/CCTextureUpdateControll...
File cc/CCTextureUpdateControllerTest.cpp (right):

https://codereview.chromium.org/10947017/diff/8003/cc/CCTextureUpdateControll...
cc/CCTextureUpdateControllerTest.cpp:71: :
LayerTextureUpdater::Texture(adoptPtr<CCPrioritizedTexture>(0))
protip: you can just type "nullptr" instead of adoptPtr<T>(0)

[commit-bot: I haz the power, 2012-09-19 21:41:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/10947017/12001

[commit-bot: I haz the power, 2012-09-19 23:59:02]

Change committed as 157662