Allow creation of ImageBitmaps from WebGL canvases.

Allow creation of ImageBitmaps from WebGL canvases.

BUG=260936, 166658
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156506

[arbesfeld, 2013-08-14 19:54:28]

ping!

[Justin Novosad, 2013-08-19 20:20:51]

https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
File Source/core/page/ImageBitmap.cpp (right):

https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
Source/core/page/ImageBitmap.cpp:76: if (!sourceContext ||
!sourceContext->isAccelerated() || !sourceContext->is2d())
Do we really need to call makeRenderingResultsAvailable() in all these cases? 
If it is just needed for WebGL, shouldn't the condition be:
if (sourceContext && !sourceContext->is2d())
If the other conditions are necessary, what use cases do they fix?  Do we have
tests for those use cases?

[arbesfeld, 2013-08-20 14:27:55]

On 2013/08/19 20:20:51, junov wrote:
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> File Source/core/page/ImageBitmap.cpp (right):
> 
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> Source/core/page/ImageBitmap.cpp:76: if (!sourceContext ||
> !sourceContext->isAccelerated() || !sourceContext->is2d())
> Do we really need to call makeRenderingResultsAvailable() in all these cases? 
> If it is just needed for WebGL, shouldn't the condition be:
> if (sourceContext && !sourceContext->is2d())
> If the other conditions are necessary, what use cases do they fix?  Do we have
> tests for those use cases?

I copied the conditions that are used for canvas to canvas draws in
CanavsRenderingContext2D::drawImage(). We can introduce that condition in
another patch if it is necessary, I suppose.

[Justin Novosad, 2013-08-20 14:40:32]

On 2013/08/20 14:27:55, arbesfeld wrote:
> On 2013/08/19 20:20:51, junov wrote:
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > File Source/core/page/ImageBitmap.cpp (right):
> > 
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > Source/core/page/ImageBitmap.cpp:76: if (!sourceContext ||
> > !sourceContext->isAccelerated() || !sourceContext->is2d())
> > Do we really need to call makeRenderingResultsAvailable() in all these
cases? 
> > If it is just needed for WebGL, shouldn't the condition be:
> > if (sourceContext && !sourceContext->is2d())
> > If the other conditions are necessary, what use cases do they fix?  Do we
have
> > tests for those use cases?
> 
> I copied the conditions that are used for canvas to canvas draws in
> CanavsRenderingContext2D::drawImage(). We can introduce that condition in
> another patch if it is necessary, I suppose.

Let's at least fully understand what makeRenderingResultsAvailable() does in
order to determine when it is needed.

[Justin Novosad, 2013-08-20 14:44:52]

On 2013/08/20 14:40:32, junov wrote:
> On 2013/08/20 14:27:55, arbesfeld wrote:
> > On 2013/08/19 20:20:51, junov wrote:
> > >
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > > File Source/core/page/ImageBitmap.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > > Source/core/page/ImageBitmap.cpp:76: if (!sourceContext ||
> > > !sourceContext->isAccelerated() || !sourceContext->is2d())
> > > Do we really need to call makeRenderingResultsAvailable() in all these
> cases? 
> > > If it is just needed for WebGL, shouldn't the condition be:
> > > if (sourceContext && !sourceContext->is2d())
> > > If the other conditions are necessary, what use cases do they fix?  Do we
> have
> > > tests for those use cases?
> > 
> > I copied the conditions that are used for canvas to canvas draws in
> > CanavsRenderingContext2D::drawImage(). We can introduce that condition in
> > another patch if it is necessary, I suppose.
> 
> Let's at least fully understand what makeRenderingResultsAvailable() does in
> order to determine when it is needed.

lgtm.

[Stephen White, 2013-08-21 20:56:30]

On 2013/08/20 14:44:52, junov wrote:
> On 2013/08/20 14:40:32, junov wrote:
> > On 2013/08/20 14:27:55, arbesfeld wrote:
> > > On 2013/08/19 20:20:51, junov wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > > > File Source/core/page/ImageBitmap.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/22408008/diff/3001/Source/core/page/ImageBitm...
> > > > Source/core/page/ImageBitmap.cpp:76: if (!sourceContext ||
> > > > !sourceContext->isAccelerated() || !sourceContext->is2d())
> > > > Do we really need to call makeRenderingResultsAvailable() in all these
> > cases? 
> > > > If it is just needed for WebGL, shouldn't the condition be:
> > > > if (sourceContext && !sourceContext->is2d())
> > > > If the other conditions are necessary, what use cases do they fix?  Do
we
> > have
> > > > tests for those use cases?
> > > 
> > > I copied the conditions that are used for canvas to canvas draws in
> > > CanavsRenderingContext2D::drawImage(). We can introduce that condition in
> > > another patch if it is necessary, I suppose.
> > 
> > Let's at least fully understand what makeRenderingResultsAvailable() does in
> > order to determine when it is needed.
> 
> lgtm.

LGTM

[commit-bot: I haz the power, 2013-08-21 20:57:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/arbesfeld@chromium.org/22408008/13001

[commit-bot: I haz the power, 2013-08-21 21:47:46]

Change committed as 156506