Minor improvement in uniform block backing buffer validation.

Minor improvement in uniform block backing buffer validation.

BUG=429053
TEST=webgl2_conformance
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[Zhenyao Mo, 2016-11-01 22:13:39]

Description was changed from

==========
Minor improvement in uniform block backing buffer validation.

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

to

==========
Minor improvement in uniform block backing buffer validation.

BUG=429053
TEST=webgl2_conformance
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-11-01 22:18:55]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 22:19:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhenyao Mo, 2016-11-01 22:19:31]

piman: PTAL
Others: FYI (feel free to review)

[piman, 2016-11-01 22:26:39]

One DCHECK issue, but after that LGTM.

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

https://codereview.chromium.org/2471533003/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8702:
DCHECK(feature_info_->IsWebGL1OrES2Context());
Did you mean DCHECK(feature_info_->IsWebGL2OrES3Context()) ?

[Zhenyao Mo, 2016-11-01 22:33:40]

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

https://codereview.chromium.org/2471533003/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8702:
DCHECK(feature_info_->IsWebGL1OrES2Context());
On 2016/11/01 22:26:39, piman wrote:
> Did you mean DCHECK(feature_info_->IsWebGL2OrES3Context()) ?

Right. Thanks for catching this.

Let's see if any tests crash. I am sure they will.  If not, I'll look into
adding tests to catch this before landing.

[Zhenyao Mo, 2016-11-01 22:33:45]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 22:34:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 23:02:53]

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

[commit-bot: I haz the power, 2016-11-01 23:02:54]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Zhenyao Mo, 2016-11-01 23:07:16]

On 2016/11/01 22:33:40, Zhenyao Mo wrote:
>
https://codereview.chromium.org/2471533003/diff/20001/gpu/command_buffer/serv...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/2471533003/diff/20001/gpu/command_buffer/serv...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:8702:
> DCHECK(feature_info_->IsWebGL1OrES2Context());
> On 2016/11/01 22:26:39, piman wrote:
> > Did you mean DCHECK(feature_info_->IsWebGL2OrES3Context()) ?
> 
> Right. Thanks for catching this.
> 
> Let's see if any tests crash. I am sure they will.  If not, I'll look into
> adding tests to catch this before landing.

Yeah, some tests caught this bug.

[Zhenyao Mo, 2016-11-01 23:08:02]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2016-11-01 23:08:03]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2471533003/#ps40001
(title: "fix")

[commit-bot: I haz the power, 2016-11-01 23:08:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 00:33:10]

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

[commit-bot: I haz the power, 2016-11-02 00:33:11]

Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Ken Russell (switch to Gerrit), 2016-11-02 00:46:06]

On 2016/11/02 00:33:11, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

Is WebglConformance_conformance2_rendering_uniform_block_buffer_size buggy or
did it catch a bug here?

[Zhenyao Mo, 2016-11-02 18:50:04]

Yes, there is a bug in the code and gladly the tests catch it.  Also, on Mac OSX
it does crash the driver (seg fault) if uniform blocks do not have enough buffer
backings. Scary.

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

https://codereview.chromium.org/2471533003/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8717: uniform_block_sizes[index]
= static_cast<GLsizeiptr>(info.data_size);
This is the only change from previous patchset.  Everything else is rebase.

[Zhenyao Mo, 2016-11-02 18:50:14]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[Zhenyao Mo, 2016-11-02 18:50:53]

piman: can you take another look?  I added a comment in patch set 4 for where
the change is.

[commit-bot: I haz the power, 2016-11-02 18:50:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[piman, 2016-11-02 19:00:05]

lgtm

[Zhenyao Mo, 2016-11-02 20:05:56]

The CQ bit was unchecked by zmo@chromium.org

[Zhenyao Mo, 2016-11-02 20:06:02]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2016-11-02 20:06:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 20:15:12]

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

[commit-bot: I haz the power, 2016-11-02 20:15:14]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhenyao Mo, 2016-11-02 20:56:20]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2016-11-02 20:56:20]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2471533003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-02 20:57:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 22:14:57]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-02 22:17:08]

Description was changed from

==========
Minor improvement in uniform block backing buffer validation.

BUG=429053
TEST=webgl2_conformance
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Minor improvement in uniform block backing buffer validation.

BUG=429053
TEST=webgl2_conformance
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[commit-bot: I haz the power, 2016-11-02 22:17:09]

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