Some cleanup of backing_store_win.cc...

Some cleanup of backing_store_win.cc

- Fix a small leak (solved by re-selecting the initial bitmap into the dc at dtor time)
- Remove one set of GetDC(NULL) + GetDeviceCaps() + ReleaseDC()
- Remove some dead code

The removal of GetDC(NULL) might speed up Vista drawing a little bit, as is rummored that
this operation is expensive when DWM + Aero is enabled.

The integer overflow check is no longer possible. I'll file a bug so it is not lost.


TEST= existing test suffice


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

[darin (slow to review), 2009-02-19 19:38:34]

please coordinate with agl on this.  he already changed backing_store_win.cc
considerably.  his patch is already LGTM'd, so i think he should probably get to
land first :)

[darin (slow to review), 2009-02-19 20:00:24]

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

http://codereview.chromium.org/21516/diff/1/2#newcode61
Line 61: // Handle to the backing store dib.
why remove the whitespace?

http://codereview.chromium.org/21516/diff/1/3
File chrome/browser/renderer_host/backing_store_win.cc (right):

http://codereview.chromium.org/21516/diff/1/3#newcode12
Line 12: namespace {
nit: add a newline

http://codereview.chromium.org/21516/diff/1/3#newcode49
Line 49: if (original_bitmap_)
what was wrong with the old order?  deleting the DC before deleting the object
allows you to not have to hold onto the original object.

http://codereview.chromium.org/21516/diff/1/3#newcode76
Line 76: NOTREACHED();
i wish we had a way to know if this were ever reached in the field.  should you
log something to UMA?  or CHECK(false)?  doing nothing seems bad.

[cpu_(ooo_6.6-7.5), 2009-02-20 00:03:27]

Changes made please look again.

http://codereview.chromium.org/21516/diff/1/3
File chrome/browser/renderer_host/backing_store_win.cc (right):

http://codereview.chromium.org/21516/diff/1/3#newcode12
Line 12: namespace {
On 2009/02/19 20:00:24, darin wrote:
> nit: add a newline

Done.

http://codereview.chromium.org/21516/diff/1/3#newcode49
Line 49: if (original_bitmap_)
On 2009/02/19 20:00:24, darin wrote:
> what was wrong with the old order?  deleting the DC before deleting the object
> allows you to not have to hold onto the original object.

I think the old way leaks original_bitmap_

http://codereview.chromium.org/21516/diff/1/3#newcode76
Line 76: NOTREACHED();
On 2009/02/19 20:00:24, darin wrote:
> i wish we had a way to know if this were ever reached in the field.  should
you
> log something to UMA?  or CHECK(false)?  doing nothing seems bad.

I like the CHECK(false)

[darin (slow to review), 2009-02-20 16:01:23]

LGTM

[darin (slow to review), 2009-02-26 22:09:52]

can this issue be closed now?

[cpu_(ooo_6.6-7.5), 2009-03-03 22:24:20]

Sorry for the delay. I had to redo the CL given's agl checkin. Is pretty much a
subset of the already LGTMed change so I am going to go ahead and commit.


On 2009/02/26 22:09:52, darin wrote:
> can this issue be closed now?