Do not give GPU memory to backgrounded compositors.

Do not give GPU memory to backgrounded compositors.

This has already been disabled on all platforms except Windows and
Chrome OS to be more conservative about GPU memory.

All platforms are moving to a delegated renderer where, if keeping
around resources for a fast tab-switch is desired, it can be
accomplished by not releasing those resources in the browser process.

This feature was originally requested in crbug.com/134750.

Also remove the infrastructure for preserving backbuffers of
backgrounded renderers. This feature was never enabled.

BUG=276559
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232324

[ccameron, 2013-10-31 03:30:12]

This torches a lot of lines of code and removes a lot of complexity from the
GpuMemoryManager.

There's still more complexity that I think should go (evicting textures on the
impl thread in cc, for instance, to the extent that that is a relevant codepath
anymore).

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

https://codereview.chromium.org/53633003/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest_damage.cc:27: class
LayerTreeHostDamageTestSetVisibleDoesNotDraw
I wasn't sure what to do with this test.

It's not clear that the behavior that the test is testing is still present. The
test relies on the ability to have visibility change without any resources being
evicted (gone) and without the backbuffer being discarded (also gone).

The results (see the comments in the diff itself) are also not what I was
expecting in terms of behavior. I did verify that this test requires the below
changes, without any of the other changes in this CL if (1) the
SetManagedMemoryPolicy call below is removed and (2) the test changes its
behavior to discard backbuffers when made non-visible (so if the results aren't
expected, it's not because of this CL).

https://codereview.chromium.org/53633003/diff/1/content/common/gpu/gpu_memory...
File content/common/gpu/gpu_memory_manager_unittest.cc (right):

https://codereview.chromium.org/53633003/diff/1/content/common/gpu/gpu_memory...
content/common/gpu/gpu_memory_manager_unittest.cc:179: bool
IsAllocationForegroundForSurfaceYes(
The tests that use these functions are getting to be of limited utility -- they
should probably be pared down or rewritten after this patch.

[piman, 2013-10-31 04:00:19]

LGTM, I support this change. I tried on the Pixel and it seems to work fine.

+tsepez security OWNER for the gpu_messages.h change (removing fields)

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

https://codereview.chromium.org/53633003/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest_damage.cc:27: class
LayerTreeHostDamageTestSetVisibleDoesNotDraw
On 2013/10/31 03:30:13, ccameron1 wrote:
> I wasn't sure what to do with this test.
> 
> It's not clear that the behavior that the test is testing is still present.
The
> test relies on the ability to have visibility change without any resources
being
> evicted (gone) and without the backbuffer being discarded (also gone).
> 
> The results (see the comments in the diff itself) are also not what I was
> expecting in terms of behavior. I did verify that this test requires the below
> changes, without any of the other changes in this CL if (1) the
> SetManagedMemoryPolicy call below is removed and (2) the test changes its
> behavior to discard backbuffers when made non-visible (so if the results
aren't
> expected, it's not because of this CL).

Do we know if this test was introduced with a particular CL to fix a particular
bug? Can we check the behavior wrt that bug?

At the very least we should rename the test, because it tests the opposite of
what it says :)

[Tom Sepez, 2013-10-31 16:59:39]

rubberstamp LGTM on deleting members.

[danakj, 2013-10-31 17:11:23]

LGTM!

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

https://codereview.chromium.org/53633003/diff/1/cc/trees/layer_tree_host_unit...
cc/trees/layer_tree_host_unittest_damage.cc:27: class
LayerTreeHostDamageTestSetVisibleDoesNotDraw
On 2013/10/31 04:00:25, piman wrote:
> On 2013/10/31 03:30:13, ccameron1 wrote:
> > I wasn't sure what to do with this test.
> > 
> > It's not clear that the behavior that the test is testing is still present.
> The
> > test relies on the ability to have visibility change without any resources
> being
> > evicted (gone) and without the backbuffer being discarded (also gone).
> > 
> > The results (see the comments in the diff itself) are also not what I was
> > expecting in terms of behavior. I did verify that this test requires the
below
> > changes, without any of the other changes in this CL if (1) the
> > SetManagedMemoryPolicy call below is removed and (2) the test changes its
> > behavior to discard backbuffers when made non-visible (so if the results
> aren't
> > expected, it's not because of this CL).
> 
> Do we know if this test was introduced with a particular CL to fix a
particular
> bug? Can we check the behavior wrt that bug?
> 
> At the very least we should rename the test, because it tests the opposite of
> what it says :)

I think delete this test. It did not come out of a bug, but out of me
experimenting trying to repro/explore behaviour. It's becoming way too awkward
to be useful now IMO.

[ccameron, 2013-10-31 18:26:29]

Thanks!!

I ended up removing test. I'll should also go through the GpuMemoryManager tests
and clean them up (most of the functionality they test has been removed).

[commit-bot: I haz the power, 2013-10-31 18:38:57]

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

[commit-bot: I haz the power, 2013-10-31 19:13:28]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-10-31 20:22:02]

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

[commit-bot: I haz the power, 2013-10-31 23:47:18]

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

[commit-bot: I haz the power, 2013-11-01 09:53:38]

Change committed as 232324