Adds a TransportDIB::Id value that is explicitly invalid and use it when compositing

Adds a TransportDIB::Id value that is explicitly invalid and use it when compositing

BUG=76001
TEST=

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

[jamesr, 2011-03-17 00:46:30]

Interested in feedback - this isn't necessarily ready to commit yet as I haven't
figured out a reasonable way to write a test.  I can manually test this by
loading a specially-crafted test and checking that I don't trigger the bad
message handler.

[brettw, 2011-03-17 03:31:00]

http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
File app/surface/transport_dib_win.cc (right):

http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
app/surface/transport_dib_win.cc:65: return Id(DefaultHandleValue(), 0);
It seems like this should just use the default constructor so we always have the
default object == InvalidId. We should change the default constructor if it's
not appropriate.

http://codereview.chromium.org/6665029/diff/2001/chrome/browser/renderer_host...
File chrome/browser/renderer_host/browser_render_process_host.cc (right):

http://codereview.chromium.org/6665029/diff/2001/chrome/browser/renderer_host...
chrome/browser/renderer_host/browser_render_process_host.cc:925: if (dib_id ==
TransportDIB::InvalidId())
We should be sure to comment the valid cases where this may happen.

[jamesr, 2011-03-17 04:17:31]

On 2011/03/17 03:31:00, brettw wrote:
>
http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
> File app/surface/transport_dib_win.cc (right):
> 
>
http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
> app/surface/transport_dib_win.cc:65: return Id(DefaultHandleValue(), 0);
> It seems like this should just use the default constructor so we always have
the
> default object == InvalidId. We should change the default constructor if it's
> not appropriate.

Id is typedef'd to a primitive on mac and linux so the default constructor will
always return 0 on those platforms and I couldn't find any sign that 0 was
invalid on those platforms. I could change it from being just a typedef to a
real class but I'm not sure that is better.
>
http://codereview.chromium.org/6665029/diff/2001/chrome/browser/renderer_host...
> File chrome/browser/renderer_host/browser_render_process_host.cc (right):
> 
>
http://codereview.chromium.org/6665029/diff/2001/chrome/browser/renderer_host...

Definitely will comment this.
> chrome/browser/renderer_host/browser_render_process_host.cc:925: if (dib_id ==
> TransportDIB::InvalidId())
> We should be sure to comment the valid cases where this may happen.

[brettw, 2011-03-17 04:35:01]

On 2011/03/17 04:17:31, jamesr wrote:
> On 2011/03/17 03:31:00, brettw wrote:
> >
>
http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
> > File app/surface/transport_dib_win.cc (right):
> > 
> >
>
http://codereview.chromium.org/6665029/diff/2001/app/surface/transport_dib_wi...
> > app/surface/transport_dib_win.cc:65: return Id(DefaultHandleValue(), 0);
> > It seems like this should just use the default constructor so we always have
> the
> > default object == InvalidId. We should change the default constructor if
it's
> > not appropriate.
> 
> Id is typedef'd to a primitive on mac and linux so the default constructor
will
> always return 0 on those platforms and I couldn't find any sign that 0 was
> invalid on those platforms. I could change it from being just a typedef to a
> real class but I'm not sure that is better.

I'm just saying in the case of the class that it's weird the default constructor
and this code are different. I think they should be the same at least when it's
a class.

[jamesr, 2011-03-23 23:33:55]

This ensures that dib_id is invalid if created like this:
  TransportDIB::Id dib_id;

on all platforms without needing an InvalidId() factory.  It turns out this was
already the case for mac and windows, so this only changes linux where I made Id
a struct with a default c'tor to do the right thing.  I also had to add a way to
check whether a given dib ID was invalid (as opposed to just checking if a
handle was invalid).

[brettw, 2011-03-24 23:54:34]

LGTM

http://codereview.chromium.org/6665029/diff/6019/app/surface/transport_dib.h
File app/surface/transport_dib.h (right):

http://codereview.chromium.org/6665029/diff/6019/app/surface/transport_dib.h#...
app/surface/transport_dib.h:103: bool operator< (const Id& other) const {
I think normal style would be no space afer the <

http://codereview.chromium.org/6665029/diff/6019/content/common/common_param_...
File content/common/common_param_traits.h (right):

http://codereview.chromium.org/6665029/diff/6019/content/common/common_param_...
content/common/common_param_traits.h:394: return (ReadParam(m, iter,
&r->shmkey));
I wouldn't do the extra parens around ReadParam.