Make async waiter explicit MojoGLES2CreateContext param

Make async waiter explicit MojoGLES2CreateContext param

Hiding the async_waiter in a static makes it difficult to deal with
different threads running potentially different loop types running in
the same process. Making this explicit in the create call is much easier.
This assumes that callers will want to permantently bind a context with
a particular async waiter. In the case that we end up with a caller that
wishes to create a context on a thread using one async waiter type and
then bind that context to a thread using a different async waiter type
we can add another version of MakeCurrent, but that seems like a fairly
remote possibility at this point.

R=piman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289074

[jamesr, 2014-07-31 01:34:39]

piman@ - please review
erg@ - FYI, hopefully this doesn't collide too badly with your thunk changes

[jamesr, 2014-07-31 01:36:00]

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
File mojo/gles2/gles2_support_impl.cc (right):

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
mojo/gles2/gles2_support_impl.cc:39:
base::ThreadLocalPointer<GLES2ImplForCommandBuffer> g_gles2_interface;
this change isn't strictly speaking part of the same patch but it's pretty
closely related to using this from multiple threads. I can split it out if you'd
prefer

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
mojo/gles2/gles2_support_impl.cc:65: delete g_gles2_interface.Get();
this is maybe not quite right - I'm assuming that a Destroy() call will always
be made from the thread the context is current on

[piman, 2014-07-31 01:53:46]

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
File mojo/gles2/gles2_support_impl.cc (right):

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
mojo/gles2/gles2_support_impl.cc:39:
base::ThreadLocalPointer<GLES2ImplForCommandBuffer> g_gles2_interface;
On 2014/07/31 01:36:00, jamesr wrote:
> this change isn't strictly speaking part of the same patch but it's pretty
> closely related to using this from multiple threads. I can split it out if
you'd
> prefer

base::LazyInstance<base::ThreadLocalPointer> to avoid adding a static
initializer?
(either way wrt this CL or another).

https://codereview.chromium.org/428413002/diff/1/mojo/gles2/gles2_support_imp...
mojo/gles2/gles2_support_impl.cc:65: delete g_gles2_interface.Get();
On 2014/07/31 01:36:00, jamesr wrote:
> this is maybe not quite right - I'm assuming that a Destroy() call will always
> be made from the thread the context is current on

mmh, I don't think we should impose that restriction, I think it can make
porting code harder (code typically MakeCurrent(NULL, NULL) before destroying
the context, things like GLX have a refcount on the context that's held by the
current thread.)
Either way, we need to reset g_gles2_interface, we don't want to leave a stale
pointer here.

We could maybe have GLES2Context own (be?) the GLES2ImplForCommandBuffer?
I forgot how this interacts with erg's patch.

[jamesr, 2014-08-07 19:36:58]

Elliot's patch will get rid of the extra object that needs to be deleted on
destruction, so I'll wait for that.

[jamesr, 2014-08-07 20:06:43]

PS3 leaks the thunk wrapper object on MojoGLES2Destroy calls.  Could you review
the rest of the patch and then this'll go away with erg@'s patch?

[piman, 2014-08-07 20:59:41]

lgtm

[jamesr, 2014-08-12 18:17:05]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-08-12 18:19:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/428413002/80001

[commit-bot: I haz the power, 2014-08-12 21:52:47]

Change committed as 289074