Disallow active attrib aliasing at shader program link time.

Disallow active attrib aliasing at shader program link time.

BUG=415688
TEST=gpu_unittests, webgl_conformance_tests
R=kbr@chromium.org,bajones@chromium.org

Committed: https://crrev.com/460b593ef3155ddf691f68def702c74246bf516f
Cr-Commit-Position: refs/heads/master@{#299387}

[Zhenyao Mo, 2014-10-11 00:25:41]

Please take a look.

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/g...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/g...
gpu/command_buffer/service/gles2_cmd_decoder.cc:4869: // TODO(zmo): Get rid of
the following glBindAttribLocation call.
I didn't do this in this CL, but simply add a TODO to remind myself.

This is because 1) it's orthogonal to this CL; 2) it involves a bunch of test
changes (mostly test expectations).

[Zhenyao Mo, 2014-10-11 00:31:16]

Tested with a bunch of shadertoy shaders, maps.google.com, and a bunch of other
webgl contents.

No regression in WebGL conformance tests.

Two related bindAttribLocation conformance tests are passing with this CL.

[Ken Russell (switch to Gerrit), 2014-10-11 03:29:03]

lgtm

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/p...
File gpu/command_buffer/service/program_manager.cc (left):

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:496: if (mapped_name &&
*mapped_name != it->first)
Do you know why this test used to be there and what the effect is of taking it
out?

[Ken Russell (switch to Gerrit), 2014-10-11 03:29:51]

I would recommend announcing this behavioral change on public_webgl, and
pointing people at the new section of the spec and conformance tests, once it
reaches Canary.

[Zhenyao Mo, 2014-10-11 04:30:08]

Will post to the public mailing list about this change.

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/p...
File gpu/command_buffer/service/program_manager.cc (left):

https://codereview.chromium.org/651543003/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:496: if (mapped_name &&
*mapped_name != it->first)
On 2014/10/11 03:29:03, Ken Russell wrote:
> Do you know why this test used to be there and what the effect is of taking it
> out?

It's used to be there because we do call glBindAttribLocation in the
bindAttribLocation command, and if we don't turn on name hashing, then this
mapped_name should equal the original_name.  However, I plan to take out the
glBindAttribLocation call (see my other comment), so this will never be true in
that case.  This is never true anyway because we always do name hashing.

[Zhenyao Mo, 2014-10-13 17:48:11]

Also tested webgl conformance suite with --disable-shader-name-hashing, no
regressions with this CL.

bajones: please take a look.

[bajones, 2014-10-13 22:28:53]

On 2014/10/13 at 17:48:11, zmo wrote:
> Also tested webgl conformance suite with --disable-shader-name-hashing, no
regressions with this CL.
> 
> bajones: please take a look.

LGTM

[Zhenyao Mo, 2014-10-13 22:34:03]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2014-10-13 22:35:51]

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

[commit-bot: I haz the power, 2014-10-13 23:07:56]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-10-13 23:08:40]

Patchset 1 (id:??) landed as
https://crrev.com/460b593ef3155ddf691f68def702c74246bf516f
Cr-Commit-Position: refs/heads/master@{#299387}