change backing store cache to be memory-based rather than count

change backing store cache to be memory-based rather than count

BUG=13763
TEST=none

[darin (slow to review), 2009-06-24 17:37:27]

Looks great overall!!

http://codereview.chromium.org/146095/diff/1/2
File base/gfx/size.h (right):

http://codereview.chromium.org/146095/diff/1/2#newcode32
Line 32: int area() const { return width_ * height_; }
since this is not a simple accessor (cheap as a member variable read), it should
be CamelCase named.  how about ComputeArea() or GetArea()?

http://codereview.chromium.org/146095/diff/1/3
File chrome/browser/renderer_host/backing_store.h (right):

http://codereview.chromium.org/146095/diff/1/3#newcode52
Line 52: // TODO(erikkay) 4 bytes per pixel isn't always accurate.
This should be pretty easy to fix.  We know the pixel depth (at least on
Windows) when we allocate the backingstore, so we should be able to make it
available here.

http://codereview.chromium.org/146095/diff/1/4
File chrome/browser/renderer_host/backing_store_manager.cc (right):

http://codereview.chromium.org/146095/diff/1/4#newcode29
Line 29: // Previously, the backing store cache was based on a set number of
dibs,
Speaking of "dibs" here is a bit misleading since on some platforms, like X
windows, they are not device independent.  Instead, they are device-dependent
pixmaps allocated by the X server.  Ideally, the backingstores should actually
be device dependent on all platforms.  Might be more future proof to just say
"bitmaps" or "backingstores" instead of dibs.

[Erik does not do reviews, 2009-06-24 18:07:56]

http://codereview.chromium.org/146095/diff/1/2
File base/gfx/size.h (right):

http://codereview.chromium.org/146095/diff/1/2#newcode32
Line 32: int area() const { return width_ * height_; }
On 2009/06/24 17:37:27, darin wrote:
> since this is not a simple accessor (cheap as a member variable read), it
should
> be CamelCase named.  how about ComputeArea() or GetArea()?

sure.

http://codereview.chromium.org/146095/diff/1/3
File chrome/browser/renderer_host/backing_store.h (right):

http://codereview.chromium.org/146095/diff/1/3#newcode52
Line 52: // TODO(erikkay) 4 bytes per pixel isn't always accurate.
On 2009/06/24 17:37:27, darin wrote:
> This should be pretty easy to fix.  We know the pixel depth (at least on
> Windows) when we allocate the backingstore, so we should be able to make it
> available here.

yeah, it was the multiplatform part that made me punt here.  linux wasn't clear
to me (if use_render_ is false...)  I took a swing at this and will ask a linux
and a mac person to verify it.

http://codereview.chromium.org/146095/diff/1/4
File chrome/browser/renderer_host/backing_store_manager.cc (right):

http://codereview.chromium.org/146095/diff/1/4#newcode29
Line 29: // Previously, the backing store cache was based on a set number of
dibs,
On 2009/06/24 17:37:27, darin wrote:
> Speaking of "dibs" here is a bit misleading since on some platforms, like X
> windows, they are not device independent.  Instead, they are device-dependent
> pixmaps allocated by the X server.  Ideally, the backingstores should actually
> be device dependent on all platforms.  Might be more future proof to just say
> "bitmaps" or "backingstores" instead of dibs.

fixed.

[Erik does not do reviews, 2009-06-24 18:42:06]

Amanda & Tony - please check out the platform-specific chunk (very small)

[Erik does not do reviews, 2009-06-24 18:43:47]

sorry, switch to agl for the linux/X part.

[Amanda Walker, 2009-06-24 18:47:27]

mac changes lgtm.

[agl, 2009-06-24 18:53:01]

LGTM

http://codereview.chromium.org/146095/diff/15/1007
File chrome/browser/renderer_host/backing_store_x.cc (right):

http://codereview.chromium.org/146095/diff/15/1007#newcode81
Line 81: if (use_render_)
!use_render_

http://codereview.chromium.org/146095/diff/15/1007#newcode84
Line 84: return size_.GetArea() * 4;  // TODO(port) is this correct?
Yes it is, remove TODO

[brettw, 2009-06-24 20:09:21]

I only skimmed this but it seems fine.

[darin (slow to review), 2009-06-24 22:21:24]

LGTM

http://codereview.chromium.org/146095/diff/15/1002
File chrome/browser/renderer_host/backing_store.h (right):

http://codereview.chromium.org/146095/diff/15/1002#newcode52
Line 52: // The number of bytes that this backing store consumes.  This should
rougly
nit: roughly