cc: Fix aborted commits with evicted textures

cc: Fix aborted commits with evicted textures

If the prioritized resource manager evicts textures in response to a
SetMemory call but the main thread doesn't have any resources to update,
then the active tree will be stuck in a state where it is not be able to
draw.  This is because ContentsTexturesPurged() is true leading to
CanDraw() being false.  This causes the active tree to be "stuck" until
a non-aborted commit occurs.

There are two logic fixes here, both of which fix this bug
independently.

The first is that the prioritized resource manager reports that it has
evicted textures when it actually hasn't.  This is why the main thread
can abort; it doesn't see any evicted textures and so has no work to do.

The second is a catch-all fix to just reset the contents textures purged
flag when a commit is aborted.  Even if there are evicted textures, if
the main thread doesn't have anything to update, then there's no reason
to prevent drawing.

R=danakj@chromium.org
BUG=268290
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216047

[ccameron, 2013-08-06 19:35:31]

The PRM part lgtm.

For this part
> The second is a catch-all fix to just reset the contents textures purged 
> flag when a commit is aborted. Even if there are evicted textures, if 
> the main thread doesn't have anything to update, then there's no reason 
> to prevent drawing.
I didn't think that it was the case that an aborted commit implies that drawing
can necessarily occur (e.g, couldn't a commit be aborted due to being
backgrounded, then when it's made visible again, it still isn't ready to draw)?
I'll defer to you and danakj on that though, since I've been out of this code
for a while.

[danakj, 2013-08-06 19:36:53]

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4209: PostSetNeedsRedrawToMainThread();
nit: You're on the main thread, you can just LTH->SetNeedsRedraw()

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4226: impl->SetNeedsCommit();
why? didn't scrollby do this?

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4249: 
?

[enne (OOO), 2013-08-06 21:41:21]

On 2013/08/06 19:35:31, ccameron1 wrote:

> I didn't think that it was the case that an aborted commit implies that
drawing
> can necessarily occur (e.g, couldn't a commit be aborted due to being
> backgrounded, then when it's made visible again, it still isn't ready to
draw)?
> I'll defer to you and danakj on that though, since I've been out of this code
> for a while.

Commits only get aborted when the main thread determines that the commit is a
no-op.  In other words, the compositor thread already has all the information
that the main thread would have sent.  This can happen right now in two cases:
(1) dropped invalidations which are too far away to cause any chance in
resources or recording.  (2) handling scrolls from the compositor thread that
don't do anything but update scroll offsets to the same position that the
compositor thread already knows about.

In other words, an aborted commit says that the compositor thread is already in
the state that would happen if the commit went through.  Since a commit resets
the contents textures purged state, it seemed sane to do the same when a commit
would abort.

There is a trade-off here of different failure modes (in the presence of other
bugs).  If the texture state is always reset in abort, then if the main thread
is incorrect that it has no updates and purged textures are in use, then we'll
get graphical corruption.

On the other hand, if the texture state isn't reset then in cases where we set
the texture purge state but a commit wouldn't actually change the set of
textures in use (such as before this patch) we get freezes.  I couldn't convince
myself of the absence of this possibility after the above fix, so I went with
the "always reset" approach.

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4209: PostSetNeedsRedrawToMainThread();
On 2013/08/06 19:36:53, danakj wrote:
> nit: You're on the main thread, you can just LTH->SetNeedsRedraw()

Done.

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4226: impl->SetNeedsCommit();
On 2013/08/06 19:36:53, danakj wrote:
> why? didn't scrollby do this?

Maybe it should, but it doesn't.  This normally gets set by LTHI's scrolling
functions.

https://codereview.chromium.org/22469002/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest.cc:4249: 
On 2013/08/06 19:36:53, danakj wrote:
> ?

Presubmit style.

https://codereview.chromium.org/22469002/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest_animation.cc (right):

https://codereview.chromium.org/22469002/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest_animation.cc:274:
EXPECT_TRUE(host_impl->CanDraw());
This test started failing because of the changes to PrioritizedResourceManager. 
Previously SetVisible(false) => SetMemory => set resources evicted (even if
nothing had) => can draw is false => OnCanDrawChanged triggers animation tick.

Since only the visibility flag changed (but not CanDraw) with the
PrioritizedResourceManager changed, this wasn't ticking animations.  I changed
the test to verify that only visibility would cause animation ticks.

To make this start passing again, I modified both proxies to update animation
ticking when visibility changed.

[danakj, 2013-08-06 21:50:12]

LGTM

[danakj, 2013-08-06 21:50:28]

+ajuma fyi

[commit-bot: I haz the power, 2013-08-06 21:56:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/22469002/17001

[commit-bot: I haz the power, 2013-08-07 00:32:16]

Change committed as 216047