Fix WebGL tex-image-and-sub-image-2d-with-video-rgb{565,4444,5551} tests on Android.

Fix WebGL tex-image-and-sub-image-2d-with-video-rgb{565,4444,5551} tests on Android.

A subtle change in behavior in the fix for Issue 443151 caused all of
the tex-image-and-sub-image-2d-with-video* tests to start failing on
Android. This went unnoticed because the only Android bot on the
waterfall at the time could not run these tests, so the tests were
marked as expected failures.

This fixes three of the four tests reliably. The fourth,
tex-image-and-sub-image-2d-with-video.html, is flaky and will be fixed
separately.

BUG=499555
TBR=dongseong.hwang@intel.com,bajones@chromium.org,zmo@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197449

[Ken Russell (switch to Gerrit), 2015-06-19 05:01:34]

I apologize, but am CQ'ing this TBR'd because I am in a rush to re-enable these
tests on the waterfall and prevent further regressions.

Any reviews of course appreciated.

Tested in content_shell on a Nexus 5.

[Ken Russell (switch to Gerrit), 2015-06-19 05:01:54]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2015-06-19 05:02:42]

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

[Sami, 2015-06-19 06:15:57]

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

[Sami, 2015-06-19 06:15:58]

lgtm.

https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:3930: if (GL_TEXTURE_2D ==
target) {
I assume the intention is that we want to try using the secondary YUV conversion
path for all TEXTURE_2D video textures instead of just ones that support
copyVideoTextureToPlatformTexture? Any idea why the software fallback wasn't
working?

[commit-bot: I haz the power, 2015-06-19 06:58:52]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197449

[Ken Russell (switch to Gerrit), 2015-06-19 17:44:18]

On 2015/06/19 06:15:58, Sami wrote:
> lgtm.
> 
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):
> 
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:3930: if (GL_TEXTURE_2D
==
> target) {
> I assume the intention is that we want to try using the secondary YUV
conversion
> path for all TEXTURE_2D video textures instead of just ones that support
> copyVideoTextureToPlatformTexture? Any idea why the software fallback wasn't
> working?

I don't know why the software fallback wasn't working. I found that
https://codereview.chromium.org/1152733008 caused the tests to start failing and
that this slight change was the culprit. It's definitely suspicious and warrants
further investigation once the tests are passing on the waterfall.

[dshwang, 2015-06-22 11:44:17]

https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:3949: if
(imageBuffer->copyToPlatformTexture(webContext(), texture->object(),
internalformat, type,
Thank you for fixing the bug I created.
I couldn't understand how this change fixes the bug because this line is
supposed to fail if "Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
|false|.
I think software fallback always goes through if
"Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
|false|, no matter this CL.
The only difference between this CL and previous is that this CL make code try
to go through hardware path and fail and then try software fallback. Trying
hardware path makes some difference (side effect). I feel root bug is still
there.

[Ken Russell (switch to Gerrit), 2015-06-23 04:55:09]

On 2015/06/22 11:44:17, dshwang wrote:
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):
> 
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:3949: if
> (imageBuffer->copyToPlatformTexture(webContext(), texture->object(),
> internalformat, type,
> Thank you for fixing the bug I created.
> I couldn't understand how this change fixes the bug because this line is
> supposed to fail if "Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
> |false|.
> I think software fallback always goes through if
> "Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
> |false|, no matter this CL.
> The only difference between this CL and previous is that this CL make code try
> to go through hardware path and fail and then try software fallback. Trying
> hardware path makes some difference (side effect). I feel root bug is still
> there.

I agree that there is still some underlying bug and would appreciate any help
you can offer in tracking it down. I'm completely swamped dealing with other
issues and haven't had time to dive into this more deeply.

[dshwang, 2015-06-24 16:25:54]

On 2015/06/23 04:55:09, Ken Russell wrote:
> On 2015/06/22 11:44:17, dshwang wrote:
> >
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> > File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1192283002/diff/1/Source/core/html/canvas/Web...
> > Source/core/html/canvas/WebGLRenderingContextBase.cpp:3949: if
> > (imageBuffer->copyToPlatformTexture(webContext(), texture->object(),
> > internalformat, type,
> > Thank you for fixing the bug I created.
> > I couldn't understand how this change fixes the bug because this line is
> > supposed to fail if "Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
> > |false|.
> > I think software fallback always goes through if
> > "Extensions3DUtil::canUseCopyTextureCHROMIUM" return 
> > |false|, no matter this CL.
> > The only difference between this CL and previous is that this CL make code
try
> > to go through hardware path and fail and then try software fallback. Trying
> > hardware path makes some difference (side effect). I feel root bug is still
> > there.
> 
> I agree that there is still some underlying bug and would appreciate any help
> you can offer in tracking it down. I'm completely swamped dealing with other
> issues and haven't had time to dive into this more deeply.

Yes, I'll dig into. However, I'm stuck in zero-copy on chromeos, so I'll be
slow.