Reland of "gpu: workaround force_cube_map_positive_x_allocation fixes Android crash."

Reland of "gpu: workaround force_cube_map_positive_x_allocation fixes Android crash."

Original CL: https://codereview.chromium.org/1280163004
Revert: https://codereview.chromium.org/1298263002

False revert for Linux Msan crash

BUG=517548
Committed: https://crrev.com/9e8922bb4c140b5efd5e14d5d6561717c512d4b7
Cr-Commit-Position: refs/heads/master@{#346622}

[dshwang, 2015-08-19 07:58:02]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2015-08-19 07:58:19]

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

[dshwang, 2015-08-19 07:59:07]

The CQ bit was unchecked by dongseong.hwang@intel.com

[dshwang, 2015-08-19 08:10:39]

sievers, could you review again? because I change some code.

https://codereview.chromium.org/1286193008/diff/20001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc (right):

https://codereview.chromium.org/1286193008/diff/20001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc:125:
glCheckFramebufferStatus(GL_FRAMEBUFFER));
FBO should be complete even if texture isn't cube complete. So change this part.

section 4.4.4 "Framebuffer Completeness" in the ES 3.0 spec. The specific text
is:

"If the value of FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is TEXTURE and the value of
FRAMEBUFFER_ATTACHMENT_OBJECT_NAME does not name an immutable-format texture and
the value of FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL is not level_base, then the
texture must be mipmap complete, and if FRAMEBUFFER_ATTACHMENT_OBJECT_NAME names
a cubemap texture, the texture must also be cube complete."

https://codereview.chromium.org/1286193008/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/1286193008/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_list_json.cc:1512: "description": "ANGLE crash on
binding incomplete cube map texture to FBO",
Add ANGLE to workaround. It will be removed soon because we can fix ANGLE easily
compared to other drivers.

[dshwang, 2015-08-19 10:25:26]

dongseong.hwang@intel.com changed reviewers:
+ kbr@chromium.org

[dshwang, 2015-08-19 10:26:38]

ok, I narrow the reason why https://codereview.chromium.org/1275773003/ fails in
Nvidia bot. The difference between Patch Set 1 and 2 is the reason. Let me see.

[dshwang, 2015-08-19 10:35:09]

nvidia driver fails glReadPixels if the bound texture is not cube complete.
ANGLE, Android Adreno, ATI requires positive x face for all kind of FBO
operation. Nvidia requires cube complete for only glReadPixels. It's different
bug so need different workaround.

[dshwang, 2015-08-20 17:13:51]

dongseong.hwang@intel.com changed reviewers:
+ piman@chromium.org

[dshwang, 2015-08-20 17:15:36]

sievers, could you review this workaround again? now this CL handles ANGLE and
NVidia also.

[no sievers, 2015-08-21 19:30:30]

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
File gpu/command_buffer/service/texture_manager.cc (right):

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
gpu/command_buffer/service/texture_manager.cc:1945: if (texture->target() ==
GL_TEXTURE_CUBE_MAP) {
This breaks all cubemap uploads that don't go through one of the workarounds,
right?
Because of the return in line 1984. We should also avoid the alloc (for |zero|)
below when not needed.

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
gpu/command_buffer/service/texture_manager.cc:1951: if
(!texture->GetLevelSize(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i,
Can't GetLevelSize() return true even though width and height are zero?

[dshwang, 2015-08-27 04:48:54]

Patchset #9 (id:160001) has been deleted

[dshwang, 2015-08-27 06:09:53]

now all trybots pass.
sievers, could you review again?

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
File gpu/command_buffer/service/texture_manager.cc (right):

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
gpu/command_buffer/service/texture_manager.cc:1945: if (texture->target() ==
GL_TEXTURE_CUBE_MAP) {
On 2015/08/21 19:30:30, sievers wrote:
> This breaks all cubemap uploads that don't go through one of the workarounds,
> right?
> Because of the return in line 1984. We should also avoid the alloc (for
|zero|)
> below when not needed.

yes, I made a mistake. Fixed.

https://codereview.chromium.org/1286193008/diff/120001/gpu/command_buffer/ser...
gpu/command_buffer/service/texture_manager.cc:1951: if
(!texture->GetLevelSize(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i,
On 2015/08/21 19:30:30, sievers wrote:
> Can't GetLevelSize() return true even though width and height are zero?

this routine check if the face is defined, not size. |width| and |height| are
output, not input.

[Ken Russell (switch to Gerrit), 2015-08-27 21:40:47]

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

[Ken Russell (switch to Gerrit), 2015-08-27 21:40:48]

+zmo as alternate reviewer

[Zhenyao Mo, 2015-08-28 22:41:58]

LGTM

https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
gpu/config/gpu_driver_bug_list_json.cc:22: "version": "8.23",
Update the version here.

[dshwang, 2015-08-31 08:50:45]

thanks for reviewing.
strangely, zmo is not owner for gpu/gpu.gyp and gpu/BUILD.gn

sievers, piman, could you review gpu/gpu.gyp and gpu/BUILD.gn ?

https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
gpu/config/gpu_driver_bug_list_json.cc:22: "version": "8.23",
On 2015/08/28 22:41:58, Zhenyao Mo wrote:
> Update the version here.

Done. I missed. Thx.

[Zhenyao Mo, 2015-08-31 17:43:25]

On 2015/08/31 08:50:45, dshwang_ooo_5.9-27.9 wrote:
> thanks for reviewing.
> strangely, zmo is not owner for gpu/gpu.gyp and gpu/BUILD.gn
> 
> sievers, piman, could you review gpu/gpu.gyp and gpu/BUILD.gn ?
> 
>
https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
> File gpu/config/gpu_driver_bug_list_json.cc (right):
> 
>
https://codereview.chromium.org/1286193008/diff/220001/gpu/config/gpu_driver_...
> gpu/config/gpu_driver_bug_list_json.cc:22: "version": "8.23",
> On 2015/08/28 22:41:58, Zhenyao Mo wrote:
> > Update the version here.
> 
> Done. I missed. Thx.

LGTM again

[dshwang, 2015-08-31 18:55:33]

On 2015/08/31 17:43:25, Zhenyao Mo wrote:
> On 2015/08/31 08:50:45, dshwang_ooo_5.9-27.9 wrote:
> > thanks for reviewing.
> > strangely, zmo is not owner for gpu/gpu.gyp and gpu/BUILD.gn
> > 
> > sievers, piman, could you review gpu/gpu.gyp and gpu/BUILD.gn ?
> > 
> LGTM again

Thx for reviewing again!

sievers, piman, could you review gpu/gpu.gyp and gpu/BUILD.gn ?

[piman, 2015-08-31 23:26:29]

lgtm

[dshwang, 2015-09-01 07:50:39]

On 2015/08/31 23:26:29, piman (OOO back 2015-08-31) wrote:
> lgtm

thx for reviewing!

[dshwang, 2015-09-01 07:50:43]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2015-09-01 07:50:48]

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

[commit-bot: I haz the power, 2015-09-01 11:04:57]

Committed patchset #12 (id:240001)

[commit-bot: I haz the power, 2015-09-01 11:05:32]

Patchset 12 (id:??) landed as
https://crrev.com/9e8922bb4c140b5efd5e14d5d6561717c512d4b7
Cr-Commit-Position: refs/heads/master@{#346622}