Retry 3 to split WebGraphicsContext3DCommandBufferImpl::initialize() into two stages.

Retry 3 to split WebGraphicsContext3DCommandBufferImpl::initialize() into two stages.

This CL splits out creation of the context from channel creation.  The
WebGraphicsContext3D will not be fully initialized until MakeCurrent is called
on it, at which point it is tied to the calling thread.

As compared to previous land attempts, this has:
- Protection against isContextLost being called on a dead context
- WebKit-side protection to ensure that GraphicsContext3Ds get made current
- Pepper modified to makeCurrent new contexts

Original review URL: http://codereview.chromium.org/7713015

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=100955

[nduca, 2011-09-09 04:42:04]

Related webkit bug: https://bugs.webkit.org/show_bug.cgi?id=67832

[lain Merrick, 2011-09-09 10:42:47]

LGTM. Third time lucky!

http://codereview.chromium.org/7860028/diff/1/content/renderer/gpu/webgraphic...
File content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h (right):

http://codereview.chromium.org/7860028/diff/1/content/renderer/gpu/webgraphic...
content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h:65: //
TODO(husky): Document threading restrictions in WebGraphicsContext3D.h,
Is this TODO still accurate? Happy to pick it up, I just want to check that's
still a good approach. Also, Gregg's idea of merging the three implementations
should make our lives much easier.

[nduca, 2011-09-09 15:47:19]

http://codereview.chromium.org/7860028/diff/1/content/renderer/gpu/webgraphic...
File content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h (right):

http://codereview.chromium.org/7860028/diff/1/content/renderer/gpu/webgraphic...
content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h:65: //
TODO(husky): Document threading restrictions in WebGraphicsContext3D.h,
On 2011/09/09 10:42:47, Iain Merrick wrote:
> Is this TODO still accurate? Happy to pick it up, I just want to check that's
> still a good approach.
I'll land a comment on the WebGraphicsContext.h side, since this patch pairs
with a webkit-side change. @kbr and I talked this approach through, though we
also considered an option of making delaying makeCurrent a property on the
Attributes structure. I'm down doing either approach, what do you think makes
more sense?

[alokp, 2011-09-09 20:10:37]

+gman

Pepper changes lgtm according to the new convention.
Longer term, do we need MakeCurrent? gman@ wanted to get rid of it IIRC.

[greggman, 2011-09-09 21:24:25]

On 2011/09/09 20:10:37, Alok Priyadarshi wrote:
> +gman
> 
> Pepper changes lgtm according to the new convention.
> Longer term, do we need MakeCurrent? gman@ wanted to get rid of it IIRC.

It's fine. There used to be a call to MakeCurrent in every nearly every function
in WebGraphicsContext3D. The one left is basically there for stuf that's calling
the gl api.

lgtm

[Ken Russell (switch to Gerrit), 2011-09-10 01:34:09]

LGTM, but it seems clear to me that
https://bugs.webkit.org/show_bug.cgi?id=67832 must land and be rolled forward
before this can land -- because WebGraphicsContext3D::initialize() must be
called before the first makeCurrent(), initialize() can only be called once, and
it's already called from the WebKit side, so we can't just modify
RendererWebKitPlatformSupportImpl::createGraphicsContext3D() to hide the change
in behavior while waiting for the WebKit side to land.