webgl: optimize webgl.texSubImage2D(video) path.

webgl: optimize webgl.texSubImage2D(video) path.

Currently, texSubImage2D(video) requires glReadPixels to copy video texture to
target texture. This CL makes infra structure to copy video texture to target
texture directly. Blink will turn on the direct path.

In addition, it will reduce WebGL video test flakiness significantly.

TEST=video texture test on webgl_conformance
./content/test/gpu/run_gpu_test.py webgl_conformance --browser=release --story-filter=WebglConformance.conformance_textures_video_tex_image_and_sub_image_2d_with_video
BUG=349871, 504773
Committed: https://crrev.com/01542ac6d0fbec6aa78e33e6c7ec49a582072ea9
Cr-Commit-Position: refs/heads/master@{#356390}

[dshwang, 2015-09-04 09:12:52]

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

[dshwang, 2015-09-04 09:12:53]

piman, could you review content/?
dalecurtis, could you review media/?

[dshwang, 2015-09-04 11:05:54]

On 2015/09/04 09:12:53, dshwang_ooo_5.9-27.9 wrote:
> piman, could you review content/?
> dalecurtis, could you review media/?

This CL block Blink CL; https://codereview.chromium.org/1284603003/
kbr, after the Blink CL is landed, I think we can turn on webgl video test on
Android, although it's still a bit flaky (pretty better than now).

[DaleCurtis, 2015-09-04 17:08:58]

Haven't looked closely at the other CL yet, but this seems to add a ton of
boilerplate? Will we be down to one copy function per class after the other CL
lands and you can clean this up?

[piman, 2015-09-04 17:34:29]

RS LGTM for content, modulo Dale's review.

Agreed that there seems to be a fair amount of duplication, and I wonder if it's
really useful to have the sub-texture and full texture versions as separate
functions.

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
content/renderer/media/android/webmediaplayer_android.cc:743: if (is_full_copy)
{
nit: maybe DCHECK that xoffset/yoffset are 0 here?

[dshwang, 2015-09-28 11:14:19]

I come back from vacation. Thanks for reviewing.

About duplication, we can have one unified API for both full copy and sub copy,
if media owner wants.
How about passing following struct instead of tons of arguments?

struct CopyVideoTexture {
        enum CopyType copy_type;
        unsigned int target;
        unsigned int texture;
        unsigned int internal_format;
        unsigned int type;
        int level;
        int xoffset;
        int yoffset;
        bool premultiply_alpha;
        bool flip_y;
}

[DaleCurtis, 2015-09-28 20:16:07]

Without looking too closely, struct sgtm, we use that pattern elsewhere in WMPI
(see WebMediaPlayerParams)

[Ken Russell (switch to Gerrit), 2015-09-28 22:34:42]

On 2015/09/28 20:16:07, DaleCurtis wrote:
> Without looking too closely, struct sgtm, we use that pattern elsewhere in
WMPI
> (see WebMediaPlayerParams)

Sounds OK as long as there are enough arguments to clearly distinguish between
both cases. There are differences between the "copy" and "sub-copy" version --
for example, the sub-copy version wouldn't be able to handle an internal format
change -- so we should clearly distinguish in the struct which version we intend
to call, validate the struct, and fail fast if there are errors in the struct's
contents.

[Ken Russell (switch to Gerrit), 2015-10-01 05:08:04]

Also: now that Blink has been rolled into Chromium, this CL can be combined with
https://codereview.chromium.org/1284603003/ . Would you please do that?

[Ken Russell (switch to Gerrit), 2015-10-01 05:08:27]

+Olli as FYI

[oetuaho-nv, 2015-10-01 07:46:21]

oetuaho@nvidia.com changed reviewers:
+ oetuaho@nvidia.com

[oetuaho-nv, 2015-10-01 07:46:21]

Really nice to see this change! A few comments inline.

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
File content/renderer/media/android/webmediaplayer_android.h (right):

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
content/renderer/media/android/webmediaplayer_android.h:157: bool
copyVideoSubTextureToPlatformTexture(
I think that it needs to be clearer that this is copying the whole video to a
subregion of the target texture. Based on the function name it might as well be
copying a subregion of the video.

I'd at least change the function name to "copyVideoTextureToPlatformSubTexture"
and add a comment.

https://codereview.chromium.org/1315323006/diff/1/media/blink/skcanvas_video_...
File media/blink/skcanvas_video_renderer.h (right):

https://codereview.chromium.org/1315323006/diff/1/media/blink/skcanvas_video_...
media/blink/skcanvas_video_renderer.h:76: static void
CopySubVideoFrameSingleTextureToGLTexture(
Same naming issue here. Would rather use
"CopyVideoFrameSingleTextureToGLSubTexture".

[dshwang, 2015-10-21 16:12:16]

I'm sorry for taking so long to get to this too. I had long vacation and in
another project. Now let me get this job done. Thank you for reviewing!

New patch set uses proposed struct and unify two methods to one.

kbr, DaleCurtis, Olli, could you review again?

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
File content/renderer/media/android/webmediaplayer_android.h (right):

https://codereview.chromium.org/1315323006/diff/1/content/renderer/media/andr...
content/renderer/media/android/webmediaplayer_android.h:157: bool
copyVideoSubTextureToPlatformTexture(
On 2015/10/01 07:46:21, oetuaho-nv wrote:
> I think that it needs to be clearer that this is copying the whole video to a
> subregion of the target texture. Based on the function name it might as well
be
> copying a subregion of the video.
> 
> I'd at least change the function name to
"copyVideoTextureToPlatformSubTexture"
> and add a comment.

This method is removed as unifying both into copyVideoTextureToPlatformTexture.
I comment "copying the whole video to a subregion of the target texture." in the
new struct.

https://codereview.chromium.org/1315323006/diff/1/media/blink/skcanvas_video_...
File media/blink/skcanvas_video_renderer.h (right):

https://codereview.chromium.org/1315323006/diff/1/media/blink/skcanvas_video_...
media/blink/skcanvas_video_renderer.h:76: static void
CopySubVideoFrameSingleTextureToGLTexture(
On 2015/10/01 07:46:21, oetuaho-nv wrote:
> Same naming issue here. Would rather use
> "CopyVideoFrameSingleTextureToGLSubTexture".

ditto.

https://codereview.chromium.org/1315323006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/Extensions3DUtil.cpp
(right):

https://codereview.chromium.org/1315323006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/Extensions3DUtil.cpp:89: //
TODO(dshwang): support cube map. crbug.com/517548
As https://codereview.chromium.org/1275773003 was reverted, zero copy cube map
texture is not yet enabled. Let me do in another patch.

[DaleCurtis, 2015-10-21 18:35:50]

media/ lgtm % removal of potential of c++11 feature.

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:639:
{CopyVideoTextureParams::FullCopy, GL_TEXTURE_2D, textureId, GL_RGBA,
Hmm, does this work by way of a C++11 initializer list? I don't remember this
working otherwise. I think you need to use a temporary "BlahBlah a = {...}"
here.

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
File content/renderer/media/webmediaplayer_ms.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
content/renderer/media/webmediaplayer_ms.cc:373: {params.copyType ==
CopyVideoTextureParams::FullCopy
Ditto.

https://codereview.chromium.org/1315323006/diff/60001/media/blink/skcanvas_vi...
File media/blink/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer.cc:165:
{SkCanvasVideoRenderer::CopyFrameSingleTextureParams::FullCopy,
Ditto.

https://codereview.chromium.org/1315323006/diff/60001/media/blink/webmediapla...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/media/blink/webmediapla...
media/blink/webmediaplayer_impl.cc:653: {params.copyType ==
CopyVideoTextureParams::FullCopy
:)

[dshwang, 2015-10-21 19:31:07]

Thank you for reviewing!

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:639:
{CopyVideoTextureParams::FullCopy, GL_TEXTURE_2D, textureId, GL_RGBA,
On 2015/10/21 18:35:50, DaleCurtis wrote:
> Hmm, does this work by way of a C++11 initializer list? I don't remember this
> working otherwise. I think you need to use a temporary "BlahBlah a = {...}"
> here. 

Yes, it's by C++11 initializer list;
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Braced_Initi...
Is it banned in chromium?

As I remember, initializer list for std::vector is banned but initializer list
for struct might be fine...

[DaleCurtis, 2015-10-22 00:35:28]

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:639:
{CopyVideoTextureParams::FullCopy, GL_TEXTURE_2D, textureId, GL_RGBA,
On 2015/10/21 19:31:07, dshwang wrote:
> On 2015/10/21 18:35:50, DaleCurtis wrote:
> > Hmm, does this work by way of a C++11 initializer list? I don't remember
this
> > working otherwise. I think you need to use a temporary "BlahBlah a = {...}"
> > here. 
> 
> Yes, it's by C++11 initializer list;
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Braced_Initi...
> Is it banned in chromium?
> 
> As I remember, initializer list for std::vector is banned but initializer list
> for struct might be fine...

Yup, banned wholesale currently https://chromium-cpp.appspot.com/ -- see uniform
initialization syntax.

[dshwang, 2015-10-22 13:23:03]

kbr, could you review third_party/WebKit?
piman, could you conform again content/?

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/1315323006/diff/60001/content/renderer/media/...
content/renderer/media/android/webmediaplayer_android.cc:639:
{CopyVideoTextureParams::FullCopy, GL_TEXTURE_2D, textureId, GL_RGBA,
On 2015/10/22 00:35:28, DaleCurtis wrote:
> On 2015/10/21 19:31:07, dshwang wrote:
> > On 2015/10/21 18:35:50, DaleCurtis wrote:
> > > Hmm, does this work by way of a C++11 initializer list? I don't remember
> this
> > > working otherwise. I think you need to use a temporary "BlahBlah a =
{...}"
> > > here. 
> > 
> > Yes, it's by C++11 initializer list;
> >
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Braced_Initi...
> > Is it banned in chromium?
> > 
> > As I remember, initializer list for std::vector is banned but initializer
list
> > for struct might be fine...
> 
> Yup, banned wholesale currently https://chromium-cpp.appspot.com/ -- see
uniform
> initialization syntax.

Thanks for investigation. I replace it to constructor.

FYI, constructor in struct is allowed by coding style;
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Structs_vs._...

[piman, 2015-10-22 21:30:23]

lgtm

https://codereview.chromium.org/1315323006/diff/80001/content/renderer/media/...
File content/renderer/media/webmediaplayer_ms.cc (right):

https://codereview.chromium.org/1315323006/diff/80001/content/renderer/media/...
content/renderer/media/webmediaplayer_ms.cc:373:
media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams(
nit: you may help readability with a function-local typedef
media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams CopyParams and use
CopyParams instead of the wholesale
media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams

[dshwang, 2015-10-23 11:07:54]

Patchset #6 (id:100001) has been deleted

[dshwang, 2015-10-23 11:09:27]

kbr, could you review third_party/WebKit?

https://codereview.chromium.org/1315323006/diff/80001/content/renderer/media/...
File content/renderer/media/webmediaplayer_ms.cc (right):

https://codereview.chromium.org/1315323006/diff/80001/content/renderer/media/...
content/renderer/media/webmediaplayer_ms.cc:373:
media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams(
On 2015/10/22 21:30:23, piman (slow to review) wrote:
> nit: you may help readability with a function-local typedef
> media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams CopyParams and use
> CopyParams instead of the wholesale
> media::SkCanvasVideoRenderer::CopyFrameSingleTextureParams

Good idea! Done.

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

Thanks for this contribution. LGTM

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4710: if
(Extensions3DUtil::canUseCopyTextureCHROMIUM(target, GL_RGBA, type, level)) {
It would be great if more code could be shared between here and texImage2D,
above, since the main difference now seems to be the arguments to the
CopyVideoTextureParams constructor.

[dshwang, 2015-10-26 14:16:32]

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

[dshwang, 2015-10-26 14:17:32]

dpranke@, could you review WebKit/public? kbr reviewed WebKit part already.

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4710: if
(Extensions3DUtil::canUseCopyTextureCHROMIUM(target, GL_RGBA, type, level)) {
On 2015/10/25 01:08:40, Ken Russell wrote:
> It would be great if more code could be shared between here and texImage2D,
> above, since the main difference now seems to be the arguments to the
> CopyVideoTextureParams constructor.

I'll merge soon.

[dshwang, 2015-10-26 17:45:15]

dpranke@, could you review WebKit/public? kbr reviewed WebKit part already.

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/1315323006/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4710: if
(Extensions3DUtil::canUseCopyTextureCHROMIUM(target, GL_RGBA, type, level)) {
On 2015/10/25 01:08:40, Ken Russell wrote:
> It would be great if more code could be shared between here and texImage2D,
> above, since the main difference now seems to be the arguments to the
> CopyVideoTextureParams constructor.

Done.

[Dirk Pranke, 2015-10-26 18:50:35]

dpranke@chromium.org changed reviewers:
+ philipj@opera.com

[Dirk Pranke, 2015-10-26 18:51:12]

On 2015/10/26 17:45:15, dshwang wrote:
> dpranke@, could you review WebKit/public? kbr reviewed WebKit part already.

I'm not an OWNER for api changes in WebKit/public. I suggest maybe
philipj@opera.com for this?

[dshwang, 2015-10-26 19:12:21]

On 2015/10/26 18:51:12, Dirk Pranke wrote:
> On 2015/10/26 17:45:15, dshwang wrote:
> > dpranke@, could you review WebKit/public? kbr reviewed WebKit part already.
> 
> I'm not an OWNER for api changes in WebKit/public. I suggest maybe
> mailto:philipj@opera.com for this?

thank you for assigning proper owner.
philipj@, could you review WebKit/public? kbr reviewed WebKit part already.

[philipj_slow, 2015-10-27 16:02:17]

third_party/WebKit/public/platform/WebMediaPlayer.h LGTM

https://codereview.chromium.org/1315323006/diff/160001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/1315323006/diff/160001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebMediaPlayer.h:173:
CopyVideoTextureParams() = delete;
Is this actually needed, doesn't declaring a constructor with arguments
implicitly get rid of the default constructor?

[dshwang, 2015-10-27 16:18:01]

thank you for reviewing!

https://codereview.chromium.org/1315323006/diff/160001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/1315323006/diff/160001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebMediaPlayer.h:173:
CopyVideoTextureParams() = delete;
On 2015/10/27 16:02:17, philipj wrote:
> Is this actually needed, doesn't declaring a constructor with arguments
> implicitly get rid of the default constructor?

good point. it's not needed. I removed it in the next patch set as well as in
chromium side struct.

[dshwang, 2015-10-27 16:18:06]

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

[dshwang, 2015-10-27 16:18:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, piman@chromium.org, kbr@chromium.org, philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1315323006/#ps180001
(title: "remove redundant "CopyVideoTextureParams() = delete". rebase to ToT")

[commit-bot: I haz the power, 2015-10-27 16:18:19]

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

[commit-bot: I haz the power, 2015-10-27 16:19:33]

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

[commit-bot: I haz the power, 2015-10-27 16:19:33]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2015-10-27 16:45:36]

Patchset #10 (id:200001) has been deleted

[dshwang, 2015-10-27 16:46:10]

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

[dshwang, 2015-10-27 16:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
philipj@opera.com, piman@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1315323006/#ps220001
(title: "rebase to ToT")

[commit-bot: I haz the power, 2015-10-27 16:46:38]

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

[commit-bot: I haz the power, 2015-10-27 16:47:23]

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

[commit-bot: I haz the power, 2015-10-27 16:47:24]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2015-10-27 17:01:47]

Patchset #10 (id:220001) has been deleted

[dshwang, 2015-10-27 17:02:56]

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

[dshwang, 2015-10-27 17:02:57]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
philipj@opera.com, piman@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1315323006/#ps240001
(title: "rebase to ToT")

[commit-bot: I haz the power, 2015-10-27 17:03:49]

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

[commit-bot: I haz the power, 2015-10-27 17:52:55]

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

[commit-bot: I haz the power, 2015-10-27 17:52:55]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2015-10-27 19:35:03]

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

[commit-bot: I haz the power, 2015-10-27 19:35:22]

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

[commit-bot: I haz the power, 2015-10-27 20:43:50]

Committed patchset #10 (id:240001)

[commit-bot: I haz the power, 2015-10-27 20:45:11]

Patchset 10 (id:??) landed as
https://crrev.com/01542ac6d0fbec6aa78e33e6c7ec49a582072ea9
Cr-Commit-Position: refs/heads/master@{#356390}

[dcaiafa, 2015-10-27 22:27:06]

A revert of this CL (patchset #10 id:240001) has been created in
https://codereview.chromium.org/1418513016/ by dcaiafa@chromium.org.

The reason for reverting is: Reverting to address webgl_conformance_tests break
in:

https://build.chromium.org/p/chromium.gpu/builders/Android%20Debug%20%28Nexus....

[dshwang, 2015-10-28 14:49:35]

On 2015/10/27 22:27:06, dcaiafa wrote:
> A revert of this CL (patchset #10 id:240001) has been created in
> https://codereview.chromium.org/1418513016/ by mailto:dcaiafa@chromium.org.
> 
> The reason for reverting is: Reverting to address webgl_conformance_tests
break
> in:
> 
>
https://build.chromium.org/p/chromium.gpu/builders/Android%20Debug%20%28Nexus....

Thank you for reverting, and sorry for careless landing.

I reproduce it on my Nexus 5. My chromebook is fine with this patch. So android
code should have bug. Let me fix and re-land.