Blink API to allow mailbox usage with texture layers for WebGL and Canvas2D.

Blink API to allow mailbox usage with texture layers for WebGL and Canvas2D.


BUG=179371

[alexst (slow to review), 2013-04-05 14:52:24]

Please take a look at this guys.

This is just the API stubs to enable webgl mailboxes. I want to unblock the
chromium part of this change and move webview compositing to use this new client
interface while I wait on the refcounting work.

[Ken Russell (switch to Gerrit), 2013-04-05 22:51:02]

I'm not familiar with this part of the API so defer my review to jamesr and
piman, but noticed one thing.

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
File Source/Platform/chromium/public/WebCompositorSupport.h (right):

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
Source/Platform/chromium/public/WebCompositorSupport.h:80: virtual
WebExternalTextureLayer*
createExternalTextureLayerForMailbox(WebExternalTextureLayerClient*) { return 0;
}
It seems weird that there are two entry points (createExternalTextureLayer and
createExternalTextureLayerForMailbox) both taking WebExternalTextureLayerClient.
Maybe WebExternalTextureLayerClient should be subclassed into e.g.
WebExternalTextureMailboxClient and the new entry points added to the subclass.

[alexst (slow to review), 2013-04-06 01:25:38]

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
File Source/Platform/chromium/public/WebCompositorSupport.h (right):

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
Source/Platform/chromium/public/WebCompositorSupport.h:80: virtual
WebExternalTextureLayer*
createExternalTextureLayerForMailbox(WebExternalTextureLayerClient*) { return 0;
}
On 2013/04/05 22:51:02, kbr wrote:
> It seems weird that there are two entry points (createExternalTextureLayer and
> createExternalTextureLayerForMailbox) both taking
WebExternalTextureLayerClient.
> Maybe WebExternalTextureLayerClient should be subclassed into e.g.
> WebExternalTextureMailboxClient and the new entry points added to the
subclass.

Once mailboxes work across the board, the texture ID path should just go away.
My thought was after we flip the switch, we could just delete the old entry
points.

[Ken Russell (switch to Gerrit), 2013-04-08 18:22:56]

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
File Source/Platform/chromium/public/WebCompositorSupport.h (right):

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
Source/Platform/chromium/public/WebCompositorSupport.h:80: virtual
WebExternalTextureLayer*
createExternalTextureLayerForMailbox(WebExternalTextureLayerClient*) { return 0;
}
On 2013/04/06 01:25:38, alexst wrote:
> On 2013/04/05 22:51:02, kbr wrote:
> > It seems weird that there are two entry points (createExternalTextureLayer
and
> > createExternalTextureLayerForMailbox) both taking
> WebExternalTextureLayerClient.
> > Maybe WebExternalTextureLayerClient should be subclassed into e.g.
> > WebExternalTextureMailboxClient and the new entry points added to the
> subclass.
> 
> Once mailboxes work across the board, the texture ID path should just go away.
> My thought was after we flip the switch, we could just delete the old entry
> points.

OK, that sounds fine.

[piman, 2013-04-08 19:31:31]

At a high level it looks reasonable - just one question.

I'm not an owner in blink, so you'll need Ken or James to LGTM this.

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
File Source/Platform/chromium/public/WebExternalTextureLayer.h (right):

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
Source/Platform/chromium/public/WebExternalTextureLayer.h:50: virtual void
clearTexture() { }
What does this mean exactly in terms of cc/ ?
Do you have the CL what implements this on the chromium side?

[alexst (slow to review), 2013-04-08 19:38:01]

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
File Source/Platform/chromium/public/WebExternalTextureLayer.h (right):

https://codereview.chromium.org/13638008/diff/1/Source/Platform/chromium/publ...
Source/Platform/chromium/public/WebExternalTextureLayer.h:50: virtual void
clearTexture() { }
On 2013/04/08 19:31:31, piman wrote:
> What does this mean exactly in terms of cc/ ?
> Do you have the CL what implements this on the chromium side?

Yes, it's below. Basically the equivalent of layer->setTexture(0).

https://codereview.chromium.org/12374028/diff/40001/webkit/compositor_binding...

[Ken Russell (switch to Gerrit), 2013-04-08 20:54:40]

This LGTM but I'd like jamesr to approve it as well.

[jamesr, 2013-04-08 22:02:11]

The copyright header on this new file isn't right.  Don't land this until we
know what boilerplate to use for new files in Blink.

[jamesr, 2013-04-08 22:03:31]

The code itself LGTM. Just don't land yet (we're working on getting guidance on
copyright headers so it shouldn't be long).

[alexst (slow to review), 2013-04-09 00:41:13]

Thanks, James.

New patch with rebase plus copyright .

[jamesr, 2013-04-09 03:21:01]

lgtm

[Ken Russell (switch to Gerrit), 2013-04-09 17:07:33]

Landed https://codereview.chromium.org/13947015/ on behalf of alexst.