Fix imageSourceURL() for WebGL elements.

Fix imageSourceURL() for WebGL elements.

For saveImageAs, we want to save the latest frame that has 
been composited to the screen. So in this case, copy from the
frontBuffer in DrawingBuffer::copyToPlatformTexture.
    
BUG=374762
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176007

[hj.r.chung, 2014-06-03 10:28:09]

PTAL!

[Justin Novosad, 2014-06-03 14:41:43]

Please add a test that triggers the bug.

[Ken Russell (switch to Gerrit), 2014-06-05 01:24:53]

Sorry for the delay reviewing.

Agree with Justin -- can you please provide a test that triggers the failure? In
https://github.com/KhronosGroup/WebGL under sdk/tests/conformance/canvas/ there
is already a to-data-url test -- runnable at
http://www.khronos.org/registry/webgl/sdk/tests/conformance/canvas/to-data-ur...
. Is this test insufficient? Should another be added?

Is this only triggerable with the "Save canvas as..." right-click menu item?

[hj.r.chung, 2014-06-05 02:21:29]

On 2014/06/05 01:24:53, Ken Russell wrote:
> Sorry for the delay reviewing.
> 
> Agree with Justin -- can you please provide a test that triggers the failure?
In
> https://github.com/KhronosGroup/WebGL under sdk/tests/conformance/canvas/
there
> is already a to-data-url test -- runnable at
>
http://www.khronos.org/registry/webgl/sdk/tests/conformance/canvas/to-data-ur...
> . Is this test insufficient? Should another be added?
> 
> Is this only triggerable with the "Save canvas as..." right-click menu item?

Sure, I will check and update the commit message (or code/tests if necessary)

[hj.r.chung, 2014-06-05 04:54:00]

On 2014/06/05 02:21:29, hj.r.chung wrote:
> On 2014/06/05 01:24:53, Ken Russell wrote:
> > Sorry for the delay reviewing.
> > 
> > Agree with Justin -- can you please provide a test that triggers the
failure?
> In
> > https://github.com/KhronosGroup/WebGL under sdk/tests/conformance/canvas/
> there
> > is already a to-data-url test -- runnable at
> >
>
http://www.khronos.org/registry/webgl/sdk/tests/conformance/canvas/to-data-ur...
> > . Is this test insufficient? Should another be added?
> > 
> > Is this only triggerable with the "Save canvas as..." right-click menu item?
> 
> Sure, I will check and update the commit message (or code/tests if necessary)

Hi!
I've confirmed that the to-data-url test in the WebGL conformance suite doesn't
cover 
this case because it isn't animated. I'll add a canvas Layout test that triggers
the bug.
(And I also will fix the patch because it actually broke the conformance test :(
)

[hj.r.chung, 2014-06-05 12:15:41]

PATL.. 
Added a test and modified the patch so it doesn't break the to-data-url test

[Ken Russell (switch to Gerrit), 2014-06-07 03:27:37]

Thanks for the update.

Sorry, but this fix is not correct. It breaks WebGL's
preserveDrawingBuffer=false behavior, and breaks the WebGL conformance test
http://www.khronos.org/registry/webgl/sdk/tests/conformance/context/context-a...
. (Do you have the ability to send try jobs for your CLs? The win_gpu, linux_gpu
and mac_gpu bots should catch this failure.)

I haven't had time to step through the code but am confused why this patch
works. It looks like HTMLCanvasElement::toDataURLInternal() should be going down
the path which calls HTMLCanvasElement::getImageData() and
WebGLRenderingContextBase::paintRenderingResultsToImageData, not the
paintRenderingResultsToCanvas path.

Regardless, what is needed here is to read back non-destructively the results
presented to the compositor most recently. I think the printing path does this.

Could you please investigate a little more deeply and propose another patch?

[hj.r.chung, 2014-06-09 01:00:27]

On 2014/06/07 03:27:37, Ken Russell wrote:
> Thanks for the update.
> 
> Sorry, but this fix is not correct. It breaks WebGL's
> preserveDrawingBuffer=false behavior, and breaks the WebGL conformance test
>
http://www.khronos.org/registry/webgl/sdk/tests/conformance/context/context-a...
> . (Do you have the ability to send try jobs for your CLs? The win_gpu,
linux_gpu
> and mac_gpu bots should catch this failure.)
> 
No, I don't have the permission for try bots..

> I haven't had time to step through the code but am confused why this patch
> works. It looks like HTMLCanvasElement::toDataURLInternal() should be going
down
> the path which calls HTMLCanvasElement::getImageData() and
> WebGLRenderingContextBase::paintRenderingResultsToImageData, not the
> paintRenderingResultsToCanvas path.
> 
> Regardless, what is needed here is to read back non-destructively the results
> presented to the compositor most recently. I think the printing path does
this.
> 
> Could you please investigate a little more deeply and propose another patch?

Yes, I'll look deeper and find a right fix. Thanks for the info!

[hj.r.chung, 2014-06-10 11:36:01]

On 2014/06/09 01:00:27, hj.r.chung wrote:
> On 2014/06/07 03:27:37, Ken Russell wrote:
> > Thanks for the update.
> > 
> > Sorry, but this fix is not correct. It breaks WebGL's
> > preserveDrawingBuffer=false behavior, and breaks the WebGL conformance test
> >
>
http://www.khronos.org/registry/webgl/sdk/tests/conformance/context/context-a...
> > . (Do you have the ability to send try jobs for your CLs? The win_gpu,
> linux_gpu
> > and mac_gpu bots should catch this failure.)
> > 
> No, I don't have the permission for try bots..
> 
> > I haven't had time to step through the code but am confused why this patch
> > works. It looks like HTMLCanvasElement::toDataURLInternal() should be going
> down
> > the path which calls HTMLCanvasElement::getImageData() and
> > WebGLRenderingContextBase::paintRenderingResultsToImageData, not the
> > paintRenderingResultsToCanvas path.
> > 
> > Regardless, what is needed here is to read back non-destructively the
results
> > presented to the compositor most recently. I think the printing path does
> this.
> > 
> > Could you please investigate a little more deeply and propose another patch?
> 
> Yes, I'll look deeper and find a right fix. Thanks for the info!

The WebGLRenderingContextBase::paintRenderingResultsToImageData path is skipped
because
premultipliedAlpha is true for most WebGL Elements.

I've uploaded a patch that seems to pass the conformance test, but I don't think
I have enough permissions to use the try servers yet.

[Ken Russell (switch to Gerrit), 2014-06-11 21:55:26]

Thanks. This looks nice. LGTM

[hj.r.chung, 2014-06-12 04:49:22]

The CQ bit was checked by heejin.r.chung@samsung.com

[hj.r.chung, 2014-06-12 04:49:23]

The CQ bit was unchecked by heejin.r.chung@samsung.com

[hj.r.chung, 2014-06-12 04:49:27]

The CQ bit was checked by heejin.r.chung@samsung.com

[commit-bot: I haz the power, 2014-06-12 04:49:45]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/heejin.r.chung@samsung.com/309933004/6...

[hj.r.chung, 2014-06-12 04:49:53]

On 2014/06/11 21:55:26, Ken Russell wrote:
> Thanks. This looks nice. LGTM

Thanks for the review!

[commit-bot: I haz the power, 2014-06-12 06:49:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/11678)

[commit-bot: I haz the power, 2014-06-12 07:48:40]

Change committed as 176007