Compile shader upon deletion if attached.

Compile shader upon deletion if attached.

R=kbr@chromium.org, vmiura@chromium.org
BUG=459778
TEST=chromote locally with enableVideoDecodeRenderer

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

[vmiura, 2015-02-25 00:53:17]

This looks fine.  Let's include a gl_test that fails without the fix in the same
patch.

Thanks.

[David Yen, 2015-02-25 01:07:20]

On 2015/02/25 00:53:17, vmiura wrote:
> This looks fine.  Let's include a gl_test that fails without the fix in the
same
> patch.
> 
> Thanks.

Unit test has been added. I've verified that it fails without my changes, and
succeeds with my changes.

[Ken Russell (switch to Gerrit), 2015-02-25 01:10:03]

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

https://codereview.chromium.org/954073002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5137:
glDeleteShader(shader->service_id());
This won't work. The service_id will have been zeroed out and we won't actually
delete the underlying shader object. This is probably currently generating an
INVALID_VALUE error. Please test for that. We should also ensure that the
command decoder tests will catch the situation where we accidentally leak the
shader object as is currently being done here.

[Zhenyao Mo, 2015-02-25 01:15:37]

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

[Zhenyao Mo, 2015-02-25 01:15:37]

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

https://codereview.chromium.org/954073002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5137:
glDeleteShader(shader->service_id());
You could probably change MarkAsDeleted() to just Delete(), which combines both
the current MarkAsDeleted() and the actual glDeleteShader() call.

[David Yen, 2015-02-25 02:15:00]

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

https://codereview.chromium.org/954073002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5137:
glDeleteShader(shader->service_id());
On 2015/02/25 01:15:37, Zhenyao Mo wrote:
> You could probably change MarkAsDeleted() to just Delete(), which combines
both
> the current MarkAsDeleted() and the actual glDeleteShader() call.

Done. This is already being tested but I didn't run gpu_unittests locally.

[Zhenyao Mo, 2015-02-25 03:57:15]

LGTM

[David Yen, 2015-02-25 04:34:17]

The CQ bit was checked by dyen@chromium.org

[commit-bot: I haz the power, 2015-02-25 04:35:48]

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

[commit-bot: I haz the power, 2015-02-25 05:24:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-25 05:25:33]

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