command_buffer: Guard on extension and profiles in the decoder

command_buffer: Guard on extension and profiles in the decoder

Previously the command buffer generator would only extension and
profile guards for the context state implementation. This caused
HandleHint to incorrectly call glHint with GL_GENERATE_MIPMAP_HINT on
a core profile, resulting in an OpenGL error.

BUG=614504
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/dc1c9eb2fa2588cbe5003251683f09f0c50226f4
Cr-Commit-Position: refs/heads/master@{#396093}

[cwallez, 2016-05-24 21:33:43]

Description was changed from

==========
command_buffer: Guard on extension and profiles in the decoder

Previously the command buffer generator would only extension and
profile guards for the context state implementation. This caused
HandleHint to incorrectly call glHint with GL_GENERATE_MIPMAP_HINT on
a core profile, resulting in an OpenGL error.

BUG=614504
==========

to

==========
command_buffer: Guard on extension and profiles in the decoder

Previously the command buffer generator would only extension and
profile guards for the context state implementation. This caused
HandleHint to incorrectly call glHint with GL_GENERATE_MIPMAP_HINT on
a core profile, resulting in an OpenGL error.

BUG=614504
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Corentin Wallez, 2016-05-24 21:34:35]

cwallez@chromium.org changed reviewers:
+ cwallez@chromium.org, geofflang@chromium.org, kbr@chromium.org,
zmo@chromium.org

[Corentin Wallez, 2016-05-24 21:34:35]

PTAL, this fixes deqp/functional/gles3/integerstatequery.html on OSX.

[Ken Russell (switch to Gerrit), 2016-05-24 21:52:25]

Thanks for tracking down and fixing this. It looks good overall but I'm not an
OWNER. Let's make sure any gpu_unittests still work, and are updated as
necessary to cover this change.

[Zhenyao Mo, 2016-05-24 22:02:32]

Thanks for tracking this down.  The code looks good.

Can you address kbr's comment about gpu_unittests?

[Zhenyao Mo, 2016-05-25 20:40:03]

lgtm if all unit tests are passing

[Ken Russell (switch to Gerrit), 2016-05-25 20:43:56]

Note: cwallez@ and I talked offline about this and it seems very hard to add
unit tests that mock the situation where the core profile is enabled, since the
test expectations (which are very large) were never updated for that
configuration. That having been said, let's get this in ASAP and perhaps add a
TODO about adding a test which mocks the fact that the core profile is enabled.

[Corentin Wallez, 2016-05-25 21:05:28]

On 2016/05/25 at 20:43:56, kbr wrote:
> Note: cwallez@ and I talked offline about this and it seems very hard to add
unit tests that mock the situation where the core profile is enabled, since the
test expectations (which are very large) were never updated for that
configuration. That having been said, let's get this in ASAP and perhaps add a
TODO about adding a test which mocks the fact that the core profile is enabled.

PTAL again, one of the changes required to make the desktop core profile working
was like the ones already done in the build_gles2_cmd_buffer.py script so
bundling everything in one commit is probably simpler.

This new patch adds the unittests, and fixes the mock expectations to work with
a core profile.

[Ken Russell (switch to Gerrit), 2016-05-25 21:07:46]

Great. Thanks for adding the test. LGTM again.

[Zhenyao Mo, 2016-05-25 21:42:38]

lgtm with one suggestion (if bots are green)

https://codereview.chromium.org/2009073002/diff/20001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc
(right):

https://codereview.chromium.org/2009073002/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc:463:
EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
You should check no GL error here.

[Corentin Wallez, 2016-05-25 22:06:02]

On 2016/05/25 at 21:42:38, zmo wrote:
> lgtm with one suggestion (if bots are green)
> 
>
https://codereview.chromium.org/2009073002/diff/20001/gpu/command_buffer/serv...
> File gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc
(right):
> 
>
https://codereview.chromium.org/2009073002/diff/20001/gpu/command_buffer/serv...
> gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc:463:
EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
> You should check no GL error here.

Thanks all for the reviews, let's see what the bots think.

[Corentin Wallez, 2016-05-25 22:06:04]

The CQ bit was checked by cwallez@chromium.org

[Corentin Wallez, 2016-05-25 22:06:05]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/2009073002/#ps40001
(title: "Check for GL errors in the unittest")

[commit-bot: I haz the power, 2016-05-25 22:06:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009073002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009073002/40001

[commit-bot: I haz the power, 2016-05-26 00:56:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-26 00:56:43]

Try jobs failed on following builders:
  win_optional_gpu_tests_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[Ken Russell (switch to Gerrit), 2016-05-26 01:32:44]

On 2016/05/26 00:56:43, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   win_optional_gpu_tests_rel on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

The GPU process hung while running that test. See http://crbug.com/609252 .
Trying again.

[Ken Russell (switch to Gerrit), 2016-05-26 01:32:50]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2016-05-26 01:33:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009073002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009073002/40001

[commit-bot: I haz the power, 2016-05-26 03:12:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-26 03:13:51]

Description was changed from

==========
command_buffer: Guard on extension and profiles in the decoder

Previously the command buffer generator would only extension and
profile guards for the context state implementation. This caused
HandleHint to incorrectly call glHint with GL_GENERATE_MIPMAP_HINT on
a core profile, resulting in an OpenGL error.

BUG=614504
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
command_buffer: Guard on extension and profiles in the decoder

Previously the command buffer generator would only extension and
profile guards for the context state implementation. This caused
HandleHint to incorrectly call glHint with GL_GENERATE_MIPMAP_HINT on
a core profile, resulting in an OpenGL error.

BUG=614504
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/dc1c9eb2fa2588cbe5003251683f09f0c50226f4
Cr-Commit-Position: refs/heads/master@{#396093}
==========

[commit-bot: I haz the power, 2016-05-26 03:13:52]

Patchset 3 (id:??) landed as
https://crrev.com/dc1c9eb2fa2588cbe5003251683f09f0c50226f4
Cr-Commit-Position: refs/heads/master@{#396093}