Add glClientWaitSync to GPU command buffer.

Add glClientWaitSync to GPU command buffer.

Note that Chromium won't allow any non-zero value for |timeout| in WebGL 2.

If Nacl decides differently on this, we can revisit the code in the future.

This command has a GLuint64.  Instead of upgrading command buffer to be 64 bit,
we use a Union to decode the 64bit value to two 32bit values on the client side,
and encode them back into one 64bit value on the service side.

BUG=429053
TEST=gpu_unittests
R=piman@chromium.org

Committed: https://crrev.com/3366957e282c020cd471e23ef6bffb253edf790f
Cr-Commit-Position: refs/heads/master@{#316917}

[Zhenyao Mo, 2015-02-18 01:38:18]

zmo@chromium.org changed reviewers:
+ piman@chromium.org

[Zhenyao Mo, 2015-02-18 01:38:49]

piman: please take a look.
Others: FYI (feel free to review)

[piman, 2015-02-18 16:22:07]

https://codereview.chromium.org/932963003/diff/60001/gpu/command_buffer/servi...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/932963003/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_decoder.cc:12071: // In Chromium we only
allow |timeout| to be 0.
We should allow it here, and check this in the webgl code instead
(WebGLRenderingContext).

[Zhenyao Mo, 2015-02-18 16:49:46]

On 2015/02/18 16:22:07, piman (Very slow to review) wrote:
>
https://codereview.chromium.org/932963003/diff/60001/gpu/command_buffer/servi...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/932963003/diff/60001/gpu/command_buffer/servi...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:12071: // In Chromium we only
> allow |timeout| to be 0.
> We should allow it here, and check this in the webgl code instead
> (WebGLRenderingContext).

Will it be a concern that a malicious command will stall the GPU process for a
really long time?  Maybe not, as it can't do more damage than just stalling it.

[Zhenyao Mo, 2015-02-18 17:32:30]

Updated.  Removed the limitation of timeout == 0 from service side.  Also
modified the union a bit to pass win builders.

[piman, 2015-02-18 18:38:00]

I'm not really concerned about the client being able to block the GPU process
for a while, it can already do that with glFinish.
The only thing is that validation needs to make sure that the command that
enqueues the sync was flushed if the flag bit isn't set, but I guess that's
ANGLE's responsibility.

https://codereview.chromium.org/932963003/diff/120001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gles2_cmd_format.h (right):

https://codereview.chromium.org/932963003/diff/120001/gpu/command_buffer/comm...
gpu/command_buffer/common/gles2_cmd_format.h:81: union GLuint64Union {
Oh, actually, I missed the fact that this was a union.
Writing a member of the union and reading another one is undefined behavior in
C++.

Why does this need to be a union at all exactly? We can extract higher and lower
uints using shifts and masks, and reconstruct similarly (you can make helper
functions).

[Zhenyao Mo, 2015-02-18 19:10:37]

Removed union and used helper functions instead.

https://codereview.chromium.org/932963003/diff/120001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gles2_cmd_format.h (right):

https://codereview.chromium.org/932963003/diff/120001/gpu/command_buffer/comm...
gpu/command_buffer/common/gles2_cmd_format.h:81: union GLuint64Union {
On 2015/02/18 18:38:00, piman (Very slow to review) wrote:
> Oh, actually, I missed the fact that this was a union.
> Writing a member of the union and reading another one is undefined behavior in
> C++.
> 
> Why does this need to be a union at all exactly? We can extract higher and
lower
> uints using shifts and masks, and reconstruct similarly (you can make helper
> functions).

Thanks for catching this.  My understanding of union was apparently incorrect.

[piman, 2015-02-18 22:51:15]

LGTM, thanks

[Zhenyao Mo, 2015-02-18 23:34:26]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2015-02-18 23:35:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/932963003/140001

[commit-bot: I haz the power, 2015-02-18 23:40:48]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-02-18 23:42:13]

Patchset 8 (id:??) landed as
https://crrev.com/3366957e282c020cd471e23ef6bffb253edf790f
Cr-Commit-Position: refs/heads/master@{#316917}