Added runtime checks for GetClientRect errors....

Added runtime checks for GetClientRect errors.

I noticed that if the window handle is invalid, the resulting rect will be uninitialized. This change makes it fail immediately.

[apatrick1, 2010-08-18 00:02:37]

Hi Daniel,

Can you take a look at this patch when you get some time? It's not high
priority.

Thanks,

Al

[dgkoch, 2010-08-23 19:01:16]

Have you actually managed to get into a situation with an invalid window handle?
 
I wouldn't expect this to be a good situation to be in...

http://codereview.appspot.com/2009042/diff/1/2
File src/libEGL/Surface.cpp (right):

http://codereview.appspot.com/2009042/diff/1/2#newcode97
src/libEGL/Surface.cpp:97: return error(EGL_FALSE);
EGL_FALSE is not a valid EGL error to set here

http://codereview.appspot.com/2009042/diff/1/2#newcode319
src/libEGL/Surface.cpp:319: 
This will make checkForWindowResize abort instead of failing (which might be
sufficient for your purposes) but the calling function doesn't actually check
the return value.

[apatrick1, 2010-08-23 22:03:07]

Thanks Daniel,

Responses inline...

Al

http://codereview.appspot.com/2009042/diff/1/2
File src/libEGL/Surface.cpp (right):

http://codereview.appspot.com/2009042/diff/1/2#newcode97
src/libEGL/Surface.cpp:97: return error(EGL_FALSE);
On 2010/08/23 19:01:17, dgkoch wrote:
> EGL_FALSE is not a valid EGL error to set here

I misread the spec. I changed it to an assertion.

http://codereview.appspot.com/2009042/diff/1/2#newcode319
src/libEGL/Surface.cpp:319: 
On 2010/08/23 19:01:17, dgkoch wrote:
> This will make checkForWindowResize abort instead of failing (which might be
> sufficient for your purposes) but the calling function doesn't actually check
> the return value. 

The intention is to prevent later code from doing things with an uninitialized
width and height in the event the window handle becomes invalid. This likely
indicates a bug in client code, since it should have destroyed the surface
before destroying the window.

I ran into this because of a bug in Chrome. I added an assertion to indicate
that this is not a legitimate runtime error.

[dgkoch, 2010-08-23 22:55:40]

Thanks for clarifying.

This version looks better. Approved.