Mojo: AsyncWaiter and mojo/public/environment

Mojo: AsyncWaiter and mojo/public/environment

Summary of changes:

 o BindingsSupport is gone:
   - mojo/public/bindings/lib depends on mojo/public/environment/,
     which is also a static library.
   - mojo/public/environment provides a default implementation of
     MojoAsyncWaiter (replacing the AsyncWait functionality of
     BindingsSupport).
   - mojo/public/environment provides TLS support for storing the
     current Buffer* (replacing the Set/GetCurrentBuffer functionality
     of BindingsSupport).
   - mojo/public/environment provides the Environment class, formerly
     part of mojo/public/utility/
   - The standalone implementation of mojo/public/environment/ depends
     on mojo/public/utility/ and assumes clients will be instantiating
     RunLoops on their threads.
   - The chromium-specific implementation of mojo/public/environment/
     depends on mojo/common/ and assumes clients will be instantiating
     MessageLoops on their threads.
   - The chromium-specific implementation of mojo/public/environment/
     is divided into two targets: mojo_environment_chromium and
     mojo_environment_chromium_impl. The former is a static library and
     the latter is a component. (This way all of the state--TLS keys--
     associated with the environment is kept in a DSO when using a
     component build.)

 o RemotePtr and Connector may optionally be parameterized with a
   MojoAsyncWaiter*, allowing users to customize how AsyncWait is
   implemented for a particular usage of bindings. This is needed by
   the GL library so that it can schedule work on an application
   defined run loop.

 o RunLoop gains a RunUntilIdle method to support tests. This allows us
   to delete SimpleBindingsSupport instead of converting it over to an
   implementation of MojoAsyncWaiter.

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

[piman, 2014-01-11 01:26:16]

lgtm

[darin (slow to review), 2014-01-13 21:32:58]

On 2014/01/11 01:26:16, piman wrote:
> lgtm

Note: Antoine's comment only applies to the MojoAsyncWaiter interface.

[darin (slow to review), 2014-01-13 21:33:25]

+trung for the bulk of the code review

[viettrungluu, 2014-01-13 21:47:43]

https://codereview.chromium.org/134253004/diff/770001/mojo/environment/enviro...
File mojo/environment/environment.cc (right):

https://codereview.chromium.org/134253004/diff/770001/mojo/environment/enviro...
mojo/environment/environment.cc:10: // no-op
It might be helpful to have an explanation about why this is a no-op (maybe a
file-level comment).

https://codereview.chromium.org/134253004/diff/770001/mojo/public/environment...
File mojo/public/environment/async_waiter.h (right):

https://codereview.chromium.org/134253004/diff/770001/mojo/public/environment...
mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
MojoGetDefaultAsyncWaiter();
Will this be needed by the GL library? (As in, will it ever need to pick up the
default async waiter from the application?)

If not (and it's only needed for the bindings), maybe it shouldn't be in this
file (and could even be a C++ function).

https://codereview.chromium.org/134253004/diff/770001/mojo/public/environment...
File mojo/public/environment/buffer_tls.h (right):

https://codereview.chromium.org/134253004/diff/770001/mojo/public/environment...
mojo/public/environment/buffer_tls.h:14: extern Buffer* GetCurrentBuffer();
Why "extern"?

[darin (slow to review), 2014-01-13 21:54:01]

On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:

>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/environment/environment.cc
> File mojo/environment/environment.cc (right):
>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/environment/environment.cc#newcode10
> mojo/environment/environment.cc:10: // no-op
> It might be helpful to have an explanation about why this is a no-op
> (maybe a file-level comment).
>

Good idea



>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/public/environment/async_waiter.h
> File mojo/public/environment/async_waiter.h (right):
>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/public/environment/async_waiter.h#newcode37
> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
> MojoGetDefaultAsyncWaiter();
> Will this be needed by the GL library? (As in, will it ever need to pick
> up the default async waiter from the application?)
>

My understanding is that the user of the GL library will need to initialize
the GL
library passing in the value returned by MojoGetDefaultAsyncWaiter(). That
way
the GL library will have a way to schedule work on the user defined run
loop.



>
> If not (and it's only needed for the bindings), maybe it shouldn't be in
> this file (and could even be a C++ function).
>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/public/environment/buffer_tls.h
> File mojo/public/environment/buffer_tls.h (right):
>
> https://codereview.chromium.org/134253004/diff/770001/
> mojo/public/environment/buffer_tls.h#newcode14
> mojo/public/environment/buffer_tls.h:14: extern Buffer*
> GetCurrentBuffer();
> Why "extern"?
>

Oh, I meant to kill that.


>
> https://codereview.chromium.org/134253004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-01-13 21:54:55]

On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org> wrote:

>
>
>
> On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:
>
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/environment/environment.cc
>> File mojo/environment/environment.cc (right):
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/environment/environment.cc#newcode10
>> mojo/environment/environment.cc:10: // no-op
>> It might be helpful to have an explanation about why this is a no-op
>> (maybe a file-level comment).
>>
>
> Good idea
>
>
>
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/public/environment/async_waiter.h
>> File mojo/public/environment/async_waiter.h (right):
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/public/environment/async_waiter.h#newcode37
>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>> MojoGetDefaultAsyncWaiter();
>> Will this be needed by the GL library? (As in, will it ever need to pick
>> up the default async waiter from the application?)
>>
>
> My understanding is that the user of the GL library will need to
> initialize the GL
> library passing in the value returned by MojoGetDefaultAsyncWaiter(). That
> way
> the GL library will have a way to schedule work on the user defined run
> loop.
>

Yes, that's the intention.


>
>
>
>>
>> If not (and it's only needed for the bindings), maybe it shouldn't be in
>> this file (and could even be a C++ function).
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/public/environment/buffer_tls.h
>> File mojo/public/environment/buffer_tls.h (right):
>>
>> https://codereview.chromium.org/134253004/diff/770001/
>> mojo/public/environment/buffer_tls.h#newcode14
>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>> GetCurrentBuffer();
>> Why "extern"?
>>
>
> Oh, I meant to kill that.
>
>
>>
>> https://codereview.chromium.org/134253004/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vtl, 2014-01-13 22:05:38]

On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org> wrote:

>
>
>
> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org> wrote:
>
>>
>>
>>
>> On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:
>>
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/environment/environment.cc
>>> File mojo/environment/environment.cc (right):
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/environment/environment.cc#newcode10
>>> mojo/environment/environment.cc:10: // no-op
>>> It might be helpful to have an explanation about why this is a no-op
>>> (maybe a file-level comment).
>>>
>>
>> Good idea
>>
>>
>>
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/public/environment/async_waiter.h
>>> File mojo/public/environment/async_waiter.h (right):
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/public/environment/async_waiter.h#newcode37
>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>> MojoGetDefaultAsyncWaiter();
>>> Will this be needed by the GL library? (As in, will it ever need to pick
>>> up the default async waiter from the application?)
>>>
>>
>> My understanding is that the user of the GL library will need to
>> initialize the GL
>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
>> That way
>> the GL library will have a way to schedule work on the user defined run
>> loop.
>>
>
> Yes, that's the intention.
>

But the GL library itself doesn't need to know about
MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition of
MojoAsyncWaiter.


>
>
>>
>>
>>
>>>
>>> If not (and it's only needed for the bindings), maybe it shouldn't be in
>>> this file (and could even be a C++ function).
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/public/environment/buffer_tls.h
>>> File mojo/public/environment/buffer_tls.h (right):
>>>
>>> https://codereview.chromium.org/134253004/diff/770001/
>>> mojo/public/environment/buffer_tls.h#newcode14
>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>> GetCurrentBuffer();
>>> Why "extern"?
>>>
>>
>> Oh, I meant to kill that.
>>
>>
>>>
>>> https://codereview.chromium.org/134253004/
>>>
>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-01-13 22:12:09]

On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <vtl@google.com> wrote:

> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org>wrote:
>
>>
>>
>>
>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org> wrote:
>>
>>>
>>>
>>>
>>> On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:
>>>
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/environment/environment.cc
>>>> File mojo/environment/environment.cc (right):
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/environment/environment.cc#newcode10
>>>> mojo/environment/environment.cc:10: // no-op
>>>> It might be helpful to have an explanation about why this is a no-op
>>>> (maybe a file-level comment).
>>>>
>>>
>>> Good idea
>>>
>>>
>>>
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/public/environment/async_waiter.h
>>>> File mojo/public/environment/async_waiter.h (right):
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/public/environment/async_waiter.h#newcode37
>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>> MojoGetDefaultAsyncWaiter();
>>>> Will this be needed by the GL library? (As in, will it ever need to pick
>>>> up the default async waiter from the application?)
>>>>
>>>
>>> My understanding is that the user of the GL library will need to
>>> initialize the GL
>>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
>>> That way
>>> the GL library will have a way to schedule work on the user defined run
>>> loop.
>>>
>>
>> Yes, that's the intention.
>>
>
> But the GL library itself doesn't need to know about
> MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition of
> MojoAsyncWaiter.
>

Correct. It would explicitly pass whatever MojoAsyncWaiter it's given to
RemotePtr/Connector. It technically doesn't care whether
it's MojoGetDefaultAsyncWaiter or not, as long as the semantic/threading
contract is respected.

Antoine


>
>>
>>
>>>
>>>
>>>
>>>>
>>>> If not (and it's only needed for the bindings), maybe it shouldn't be in
>>>> this file (and could even be a C++ function).
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/public/environment/buffer_tls.h
>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>
>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>> GetCurrentBuffer();
>>>> Why "extern"?
>>>>
>>>
>>> Oh, I meant to kill that.
>>>
>>>
>>>>
>>>> https://codereview.chromium.org/134253004/
>>>>
>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-01-13 23:24:10]

If the GL library creates a background thread w/ a MessageLoop, then it
would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
implementation for that thread.

-Darin


On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org> wrote:

>
>
>
> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <vtl@google.com> wrote:
>
>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org>wrote:
>>
>>>
>>>
>>>
>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:
>>>>
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/environment/environment.cc
>>>>> File mojo/environment/environment.cc (right):
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/environment/environment.cc#newcode10
>>>>> mojo/environment/environment.cc:10: // no-op
>>>>> It might be helpful to have an explanation about why this is a no-op
>>>>> (maybe a file-level comment).
>>>>>
>>>>
>>>> Good idea
>>>>
>>>>
>>>>
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/public/environment/async_waiter.h
>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>>> MojoGetDefaultAsyncWaiter();
>>>>> Will this be needed by the GL library? (As in, will it ever need to
>>>>> pick
>>>>> up the default async waiter from the application?)
>>>>>
>>>>
>>>> My understanding is that the user of the GL library will need to
>>>> initialize the GL
>>>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
>>>> That way
>>>> the GL library will have a way to schedule work on the user defined run
>>>> loop.
>>>>
>>>
>>> Yes, that's the intention.
>>>
>>
>> But the GL library itself doesn't need to know about
>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition of
>> MojoAsyncWaiter.
>>
>
> Correct. It would explicitly pass whatever MojoAsyncWaiter it's given to
> RemotePtr/Connector. It technically doesn't care whether
> it's MojoGetDefaultAsyncWaiter or not, as long as the semantic/threading
> contract is respected.
>
> Antoine
>
>
>>
>>>
>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> If not (and it's only needed for the bindings), maybe it shouldn't be
>>>>> in
>>>>> this file (and could even be a C++ function).
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/public/environment/buffer_tls.h
>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>
>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>> GetCurrentBuffer();
>>>>> Why "extern"?
>>>>>
>>>>
>>>> Oh, I meant to kill that.
>>>>
>>>>
>>>>>
>>>>> https://codereview.chromium.org/134253004/
>>>>>
>>>>
>>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-01-13 23:26:56]

On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <darin@chromium.org> wrote:

> If the GL library creates a background thread w/ a MessageLoop, then it
> would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
> implementation for that thread.
>
> -Darin
>

Fair enough. It doesn't do that today, but if it does in the future, I
agree that it needs its own AsyncWait implementation for the type of
message loop it creates.

Antoine


>
>
> On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org>wrote:
>
>>
>>
>>
>> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <vtl@google.com> wrote:
>>
>>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org>wrote:
>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Jan 13, 2014 at 1:47 PM, <viettrungluu@chromium.org> wrote:
>>>>>
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/environment/environment.cc
>>>>>> File mojo/environment/environment.cc (right):
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/environment/environment.cc#newcode10
>>>>>> mojo/environment/environment.cc:10: // no-op
>>>>>> It might be helpful to have an explanation about why this is a no-op
>>>>>> (maybe a file-level comment).
>>>>>>
>>>>>
>>>>> Good idea
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/public/environment/async_waiter.h
>>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>>>> MojoGetDefaultAsyncWaiter();
>>>>>> Will this be needed by the GL library? (As in, will it ever need to
>>>>>> pick
>>>>>> up the default async waiter from the application?)
>>>>>>
>>>>>
>>>>> My understanding is that the user of the GL library will need to
>>>>> initialize the GL
>>>>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
>>>>> That way
>>>>> the GL library will have a way to schedule work on the user defined
>>>>> run loop.
>>>>>
>>>>
>>>> Yes, that's the intention.
>>>>
>>>
>>> But the GL library itself doesn't need to know about
>>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition of
>>> MojoAsyncWaiter.
>>>
>>
>>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's given to
>> RemotePtr/Connector. It technically doesn't care whether
>> it's MojoGetDefaultAsyncWaiter or not, as long as the semantic/threading
>> contract is respected.
>>
>> Antoine
>>
>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> If not (and it's only needed for the bindings), maybe it shouldn't be
>>>>>> in
>>>>>> this file (and could even be a C++ function).
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/public/environment/buffer_tls.h
>>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>>> GetCurrentBuffer();
>>>>>> Why "extern"?
>>>>>>
>>>>>
>>>>> Oh, I meant to kill that.
>>>>>
>>>>>
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-01-13 23:47:22]

On 2014/01/13 23:26:56, piman wrote:
> On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:darin@chromium.org>
wrote:
> 
> > If the GL library creates a background thread w/ a MessageLoop, then it
> > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
> > implementation for that thread.
> >
> > -Darin
> >
> 
> Fair enough. It doesn't do that today, but if it does in the future, I
> agree that it needs its own AsyncWait implementation for the type of
> message loop it creates.

I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter, would it?
It'd need the AsyncWaiter for its own message loop implementation. (It might
then need to call the shell's MojoGetDefaultAsyncWaiter, but not one provided by
the app or anything like that. So this is really an internal implementation
detail, not part of the interface with the app.)

> 
> Antoine
> 
> 
> >
> >
> > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org>wrote:
> >
> >>
> >>
> >>
> >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:vtl@google.com>
wrote:
> >>
> >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org>wrote:
> >>>
> >>>>
> >>>>
> >>>>
> >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org>wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.org>
wrote:
> >>>>>
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/environment/environment.cc
> >>>>>> File mojo/environment/environment.cc (right):
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/environment/environment.cc#newcode10
> >>>>>> mojo/environment/environment.cc:10: // no-op
> >>>>>> It might be helpful to have an explanation about why this is a no-op
> >>>>>> (maybe a file-level comment).
> >>>>>>
> >>>>>
> >>>>> Good idea
> >>>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/public/environment/async_waiter.h
> >>>>>> File mojo/public/environment/async_waiter.h (right):
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/public/environment/async_waiter.h#newcode37
> >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
> >>>>>> MojoGetDefaultAsyncWaiter();
> >>>>>> Will this be needed by the GL library? (As in, will it ever need to
> >>>>>> pick
> >>>>>> up the default async waiter from the application?)
> >>>>>>
> >>>>>
> >>>>> My understanding is that the user of the GL library will need to
> >>>>> initialize the GL
> >>>>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
> >>>>> That way
> >>>>> the GL library will have a way to schedule work on the user defined
> >>>>> run loop.
> >>>>>
> >>>>
> >>>> Yes, that's the intention.
> >>>>
> >>>
> >>> But the GL library itself doesn't need to know about
> >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition
of
> >>> MojoAsyncWaiter.
> >>>
> >>
> >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's given to
> >> RemotePtr/Connector. It technically doesn't care whether
> >> it's MojoGetDefaultAsyncWaiter or not, as long as the semantic/threading
> >> contract is respected.
> >>
> >> Antoine
> >>
> >>
> >>>
> >>>>
> >>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> If not (and it's only needed for the bindings), maybe it shouldn't be
> >>>>>> in
> >>>>>> this file (and could even be a C++ function).
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/public/environment/buffer_tls.h
> >>>>>> File mojo/public/environment/buffer_tls.h (right):
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> >>>>>> mojo/public/environment/buffer_tls.h#newcode14
> >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
> >>>>>> GetCurrentBuffer();
> >>>>>> Why "extern"?
> >>>>>>
> >>>>>
> >>>>> Oh, I meant to kill that.
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> https://codereview.chromium.org/134253004/
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-01-14 00:10:58]

On Mon, Jan 13, 2014 at 3:47 PM, <viettrungluu@chromium.org> wrote:

> On 2014/01/13 23:26:56, piman wrote:
>
>> On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:darin@chromium.org>
>>
> wrote:
>
>  > If the GL library creates a background thread w/ a MessageLoop, then it
>> > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>> > implementation for that thread.
>> >
>> > -Darin
>> >
>>
>
>  Fair enough. It doesn't do that today, but if it does in the future, I
>> agree that it needs its own AsyncWait implementation for the type of
>> message loop it creates.
>>
>
> I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter, would
> it?
>

Nope.


> It'd need the AsyncWaiter for its own message loop implementation. (It
> might
> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
> provided by
> the app or anything like that. So this is really an internal implementation
> detail, not part of the interface with the app.)
>

Correct.

Antoine


>
>
>  Antoine
>>
>
>
>  >
>> >
>> > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org
>> >wrote:
>> >
>> >>
>> >>
>> >>
>> >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:vtl@google.com
>> >
>>
> wrote:
>
>> >>
>> >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour <piman@chromium.org
>> >wrote:
>> >>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher <darin@chromium.org
>> >wrote:
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.org>
>>
> wrote:
>
>> >>>>>
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> >>>>>> mojo/environment/environment.cc
>> >>>>>> File mojo/environment/environment.cc (right):
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>
>> >>>>>> mojo/environment/environment.cc#newcode10
>> >>>>>> mojo/environment/environment.cc:10: // no-op
>> >>>>>> It might be helpful to have an explanation about why this is a
>> no-op
>> >>>>>> (maybe a file-level comment).
>> >>>>>>
>> >>>>>
>> >>>>> Good idea
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> >>>>>> mojo/public/environment/async_waiter.h
>> >>>>>> File mojo/public/environment/async_waiter.h (right):
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> >>>>>> mojo/public/environment/async_waiter.h#newcode37
>> >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>> >>>>>> MojoGetDefaultAsyncWaiter();
>> >>>>>> Will this be needed by the GL library? (As in, will it ever need to
>> >>>>>> pick
>> >>>>>> up the default async waiter from the application?)
>> >>>>>>
>> >>>>>
>> >>>>> My understanding is that the user of the GL library will need to
>> >>>>> initialize the GL
>> >>>>> library passing in the value returned by
>> MojoGetDefaultAsyncWaiter().
>> >>>>> That way
>> >>>>> the GL library will have a way to schedule work on the user defined
>> >>>>> run loop.
>> >>>>>
>> >>>>
>> >>>> Yes, that's the intention.
>> >>>>
>> >>>
>> >>> But the GL library itself doesn't need to know about
>> >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>> definition
>>
> of
>
>> >>> MojoAsyncWaiter.
>> >>>
>> >>
>> >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's given
>> to
>> >> RemotePtr/Connector. It technically doesn't care whether
>> >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>> semantic/threading
>> >> contract is respected.
>> >>
>> >> Antoine
>> >>
>> >>
>> >>>
>> >>>>
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>>
>> >>>>>> If not (and it's only needed for the bindings), maybe it shouldn't
>> be
>> >>>>>> in
>> >>>>>> this file (and could even be a C++ function).
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> >>>>>> mojo/public/environment/buffer_tls.h
>> >>>>>> File mojo/public/environment/buffer_tls.h (right):
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>> >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>> >>>>>> GetCurrentBuffer();
>> >>>>>> Why "extern"?
>> >>>>>>
>> >>>>>
>> >>>>> Oh, I meant to kill that.
>> >>>>>
>> >>>>>
>> >>>>>>
>> >>>>>> https://codereview.chromium.org/134253004/
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/134253004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-01-14 00:12:43]

On 2014/01/13 23:47:22, viettrungluu wrote:
> On 2014/01/13 23:26:56, piman wrote:
> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:darin@chromium.org>
> wrote:
> > 
> > > If the GL library creates a background thread w/ a MessageLoop, then it
> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
> > > implementation for that thread.
> > >
> > > -Darin
> > >
> > 
> > Fair enough. It doesn't do that today, but if it does in the future, I
> > agree that it needs its own AsyncWait implementation for the type of
> > message loop it creates.
> 
> I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter, would it?
> It'd need the AsyncWaiter for its own message loop implementation. (It might
> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one provided
by
> the app or anything like that. So this is really an internal implementation
> detail, not part of the interface with the app.)

(LGTM otherwise FWIW. I remain unconvinced that MojoGetDefaultAsyncWaiter() a)
needs to be extern "C", b) should be defined in async_waiter.h. Quite possibly,
I'm missing something though.)

> 
> > 
> > Antoine
> > 
> > 
> > >
> > >
> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org>wrote:
> > >
> > >>
> > >>
> > >>
> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:vtl@google.com>
> wrote:
> > >>
> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
<piman@chromium.org>wrote:
> > >>>
> > >>>>
> > >>>>
> > >>>>
> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
<darin@chromium.org>wrote:
> > >>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.org>
> wrote:
> > >>>>>
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/environment/environment.cc
> > >>>>>> File mojo/environment/environment.cc (right):
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/environment/environment.cc#newcode10
> > >>>>>> mojo/environment/environment.cc:10: // no-op
> > >>>>>> It might be helpful to have an explanation about why this is a no-op
> > >>>>>> (maybe a file-level comment).
> > >>>>>>
> > >>>>>
> > >>>>> Good idea
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/public/environment/async_waiter.h
> > >>>>>> File mojo/public/environment/async_waiter.h (right):
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
> > >>>>>> MojoGetDefaultAsyncWaiter();
> > >>>>>> Will this be needed by the GL library? (As in, will it ever need to
> > >>>>>> pick
> > >>>>>> up the default async waiter from the application?)
> > >>>>>>
> > >>>>>
> > >>>>> My understanding is that the user of the GL library will need to
> > >>>>> initialize the GL
> > >>>>> library passing in the value returned by MojoGetDefaultAsyncWaiter().
> > >>>>> That way
> > >>>>> the GL library will have a way to schedule work on the user defined
> > >>>>> run loop.
> > >>>>>
> > >>>>
> > >>>> Yes, that's the intention.
> > >>>>
> > >>>
> > >>> But the GL library itself doesn't need to know about
> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the definition
> of
> > >>> MojoAsyncWaiter.
> > >>>
> > >>
> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's given to
> > >> RemotePtr/Connector. It technically doesn't care whether
> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the semantic/threading
> > >> contract is respected.
> > >>
> > >> Antoine
> > >>
> > >>
> > >>>
> > >>>>
> > >>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>>
> > >>>>>> If not (and it's only needed for the bindings), maybe it shouldn't be
> > >>>>>> in
> > >>>>>> this file (and could even be a C++ function).
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/public/environment/buffer_tls.h
> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
> > >>>>>> GetCurrentBuffer();
> > >>>>>> Why "extern"?
> > >>>>>>
> > >>>>>
> > >>>>> Oh, I meant to kill that.
> > >>>>>
> > >>>>>
> > >>>>>>
> > >>>>>> https://codereview.chromium.org/134253004/
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-01-14 02:38:04]

It doesn't need to be extern "C". I'll change that. I just was in the
mindset of defining a C interface :-/

I think it needs to be exposed to developers though as they will need to
call it as part of GL setup:

MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:

> On 2014/01/13 23:47:22, viettrungluu wrote:
>
>> On 2014/01/13 23:26:56, piman wrote:
>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>> darin@chromium.org>
>> wrote:
>> >
>> > > If the GL library creates a background thread w/ a MessageLoop, then
>> it
>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>> > > implementation for that thread.
>> > >
>> > > -Darin
>> > >
>> >
>> > Fair enough. It doesn't do that today, but if it does in the future, I
>> > agree that it needs its own AsyncWait implementation for the type of
>> > message loop it creates.
>>
>
>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter, would
>> it?
>> It'd need the AsyncWaiter for its own message loop implementation. (It
>> might
>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>> provided
>>
> by
>
>> the app or anything like that. So this is really an internal
>> implementation
>> detail, not part of the interface with the app.)
>>
>
> (LGTM otherwise FWIW. I remain unconvinced that
> MojoGetDefaultAsyncWaiter() a)
> needs to be extern "C", b) should be defined in async_waiter.h. Quite
> possibly,
> I'm missing something though.)
>
>
>  >
>> > Antoine
>> >
>> >
>> > >
>> > >
>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org
>> >wrote:
>> > >
>> > >>
>> > >>
>> > >>
>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>> vtl@google.com>
>> wrote:
>> > >>
>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>
> <piman@chromium.org>wrote:
>
>> > >>>
>> > >>>>
>> > >>>>
>> > >>>>
>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>
> <darin@chromium.org>wrote:
>
>> > >>>>
>> > >>>>>
>> > >>>>>
>> > >>>>>
>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.
>> org>
>> wrote:
>> > >>>>>
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/environment/environment.cc
>> > >>>>>> File mojo/environment/environment.cc (right):
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/environment/environment.cc#newcode10
>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>> > >>>>>> It might be helpful to have an explanation about why this is a
>> no-op
>> > >>>>>> (maybe a file-level comment).
>> > >>>>>>
>> > >>>>>
>> > >>>>> Good idea
>> > >>>>>
>> > >>>>>
>> > >>>>>
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/public/environment/async_waiter.h
>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>> > >>>>>> MojoGetDefaultAsyncWaiter();
>> > >>>>>> Will this be needed by the GL library? (As in, will it ever need
>> to
>> > >>>>>> pick
>> > >>>>>> up the default async waiter from the application?)
>> > >>>>>>
>> > >>>>>
>> > >>>>> My understanding is that the user of the GL library will need to
>> > >>>>> initialize the GL
>> > >>>>> library passing in the value returned by
>> MojoGetDefaultAsyncWaiter().
>> > >>>>> That way
>> > >>>>> the GL library will have a way to schedule work on the user
>> defined
>> > >>>>> run loop.
>> > >>>>>
>> > >>>>
>> > >>>> Yes, that's the intention.
>> > >>>>
>> > >>>
>> > >>> But the GL library itself doesn't need to know about
>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>> definition
>> of
>> > >>> MojoAsyncWaiter.
>> > >>>
>> > >>
>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>> given to
>> > >> RemotePtr/Connector. It technically doesn't care whether
>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>> semantic/threading
>> > >> contract is respected.
>> > >>
>> > >> Antoine
>> > >>
>> > >>
>> > >>>
>> > >>>>
>> > >>>>
>> > >>>>>
>> > >>>>>
>> > >>>>>
>> > >>>>>>
>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>> shouldn't be
>> > >>>>>> in
>> > >>>>>> this file (and could even be a C++ function).
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/public/environment/buffer_tls.h
>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>> > >>>>>> GetCurrentBuffer();
>> > >>>>>> Why "extern"?
>> > >>>>>>
>> > >>>>>
>> > >>>>> Oh, I meant to kill that.
>> > >>>>>
>> > >>>>>
>> > >>>>>>
>> > >>>>>> https://codereview.chromium.org/134253004/
>> > >>>>>>
>> > >>>>>
>> > >>>>>
>> > >>>>
>> > >>>
>> > >>
>> > >
>> >
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an
>> email
>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
> https://codereview.chromium.org/134253004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-01-14 02:48:43]

On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org> wrote:

> It doesn't need to be extern "C". I'll change that. I just was in the
> mindset of defining a C interface :-/
>
> I think it needs to be exposed to developers though as they will need to
> call it as part of GL setup:
>
> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>

It's indeed convenient - I rebased
https://codereview.chromium.org/132913004on top of this, and make good
use of it.

Antoine

 On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>
>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>
>>> On 2014/01/13 23:26:56, piman wrote:
>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>> darin@chromium.org>
>>> wrote:
>>> >
>>> > > If the GL library creates a background thread w/ a MessageLoop, then
>>> it
>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>> > > implementation for that thread.
>>> > >
>>> > > -Darin
>>> > >
>>> >
>>> > Fair enough. It doesn't do that today, but if it does in the future, I
>>> > agree that it needs its own AsyncWait implementation for the type of
>>> > message loop it creates.
>>>
>>
>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>> would it?
>>> It'd need the AsyncWaiter for its own message loop implementation. (It
>>> might
>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>> provided
>>>
>> by
>>
>>> the app or anything like that. So this is really an internal
>>> implementation
>>> detail, not part of the interface with the app.)
>>>
>>
>> (LGTM otherwise FWIW. I remain unconvinced that
>> MojoGetDefaultAsyncWaiter() a)
>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>> possibly,
>> I'm missing something though.)
>>
>>
>>  >
>>> > Antoine
>>> >
>>> >
>>> > >
>>> > >
>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org
>>> >wrote:
>>> > >
>>> > >>
>>> > >>
>>> > >>
>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>> vtl@google.com>
>>> wrote:
>>> > >>
>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>
>> <piman@chromium.org>wrote:
>>
>>> > >>>
>>> > >>>>
>>> > >>>>
>>> > >>>>
>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>
>> <darin@chromium.org>wrote:
>>
>>> > >>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.
>>> org>
>>> wrote:
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/environment/environment.cc
>>> > >>>>>> File mojo/environment/environment.cc (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>> > >>>>>> It might be helpful to have an explanation about why this is a
>>> no-op
>>> > >>>>>> (maybe a file-level comment).
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> Good idea
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/async_waiter.h
>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>> need to
>>> > >>>>>> pick
>>> > >>>>>> up the default async waiter from the application?)
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> My understanding is that the user of the GL library will need to
>>> > >>>>> initialize the GL
>>> > >>>>> library passing in the value returned by
>>> MojoGetDefaultAsyncWaiter().
>>> > >>>>> That way
>>> > >>>>> the GL library will have a way to schedule work on the user
>>> defined
>>> > >>>>> run loop.
>>> > >>>>>
>>> > >>>>
>>> > >>>> Yes, that's the intention.
>>> > >>>>
>>> > >>>
>>> > >>> But the GL library itself doesn't need to know about
>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>> definition
>>> of
>>> > >>> MojoAsyncWaiter.
>>> > >>>
>>> > >>
>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>>> given to
>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>> semantic/threading
>>> > >> contract is respected.
>>> > >>
>>> > >> Antoine
>>> > >>
>>> > >>
>>> > >>>
>>> > >>>>
>>> > >>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>> shouldn't be
>>> > >>>>>> in
>>> > >>>>>> this file (and could even be a C++ function).
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>> > >>>>>> GetCurrentBuffer();
>>> > >>>>>> Why "extern"?
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> Oh, I meant to kill that.
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>
>>> > >>>
>>> > >>
>>> > >
>>> >
>>> > To unsubscribe from this group and stop receiving emails from it, send
>>> an
>>> email
>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>
>>
>>
>> https://codereview.chromium.org/134253004/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vtl, 2014-01-14 02:51:06]

Sure, it needs to be exposed as part of an AsyncWaiter implementation that
can be used with our bindings system. But it's not part of the ABI for the
GL library, which is why I think it should be in a separate header.

To use GL, a developer has no choice but to use MojoGLES2Initialize() and
thus no choice but to use AsyncWaiter, but they need not have any
GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
system, they'll want one even if they use their own message loop ... but
using our bindings system isn't required.)

On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org> wrote:

> It doesn't need to be extern "C". I'll change that. I just was in the
> mindset of defining a C interface :-/
>
> I think it needs to be exposed to developers though as they will need to
> call it as part of GL setup:
>
> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>
>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>
>>> On 2014/01/13 23:26:56, piman wrote:
>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>> darin@chromium.org>
>>> wrote:
>>> >
>>> > > If the GL library creates a background thread w/ a MessageLoop, then
>>> it
>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>> > > implementation for that thread.
>>> > >
>>> > > -Darin
>>> > >
>>> >
>>> > Fair enough. It doesn't do that today, but if it does in the future, I
>>> > agree that it needs its own AsyncWait implementation for the type of
>>> > message loop it creates.
>>>
>>
>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>> would it?
>>> It'd need the AsyncWaiter for its own message loop implementation. (It
>>> might
>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>> provided
>>>
>> by
>>
>>> the app or anything like that. So this is really an internal
>>> implementation
>>> detail, not part of the interface with the app.)
>>>
>>
>> (LGTM otherwise FWIW. I remain unconvinced that
>> MojoGetDefaultAsyncWaiter() a)
>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>> possibly,
>> I'm missing something though.)
>>
>>
>>  >
>>> > Antoine
>>> >
>>> >
>>> > >
>>> > >
>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org
>>> >wrote:
>>> > >
>>> > >>
>>> > >>
>>> > >>
>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>> vtl@google.com>
>>> wrote:
>>> > >>
>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>
>> <piman@chromium.org>wrote:
>>
>>> > >>>
>>> > >>>>
>>> > >>>>
>>> > >>>>
>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>
>> <darin@chromium.org>wrote:
>>
>>> > >>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.
>>> org>
>>> wrote:
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/environment/environment.cc
>>> > >>>>>> File mojo/environment/environment.cc (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>> > >>>>>> It might be helpful to have an explanation about why this is a
>>> no-op
>>> > >>>>>> (maybe a file-level comment).
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> Good idea
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/async_waiter.h
>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>> need to
>>> > >>>>>> pick
>>> > >>>>>> up the default async waiter from the application?)
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> My understanding is that the user of the GL library will need to
>>> > >>>>> initialize the GL
>>> > >>>>> library passing in the value returned by
>>> MojoGetDefaultAsyncWaiter().
>>> > >>>>> That way
>>> > >>>>> the GL library will have a way to schedule work on the user
>>> defined
>>> > >>>>> run loop.
>>> > >>>>>
>>> > >>>>
>>> > >>>> Yes, that's the intention.
>>> > >>>>
>>> > >>>
>>> > >>> But the GL library itself doesn't need to know about
>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>> definition
>>> of
>>> > >>> MojoAsyncWaiter.
>>> > >>>
>>> > >>
>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>>> given to
>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>> semantic/threading
>>> > >> contract is respected.
>>> > >>
>>> > >> Antoine
>>> > >>
>>> > >>
>>> > >>>
>>> > >>>>
>>> > >>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>> shouldn't be
>>> > >>>>>> in
>>> > >>>>>> this file (and could even be a C++ function).
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>> > >>>>>> GetCurrentBuffer();
>>> > >>>>>> Why "extern"?
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>> Oh, I meant to kill that.
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>
>>> > >>>>>> https://codereview.chromium.org/134253004/
>>> > >>>>>>
>>> > >>>>>
>>> > >>>>>
>>> > >>>>
>>> > >>>
>>> > >>
>>> > >
>>> >
>>> > To unsubscribe from this group and stop receiving emails from it, send
>>> an
>>> email
>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>
>>
>>
>> https://codereview.chromium.org/134253004/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-01-14 02:59:01]

Makes sense. Is that an argument for moving the definition of
MojoAsyncWaiter back to system?
On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:

> Sure, it needs to be exposed as part of an AsyncWaiter implementation that
> can be used with our bindings system. But it's not part of the ABI for the
> GL library, which is why I think it should be in a separate header.
>
> To use GL, a developer has no choice but to use MojoGLES2Initialize() and
> thus no choice but to use AsyncWaiter, but they need not have any
> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
> system, they'll want one even if they use their own message loop ... but
> using our bindings system isn't required.)
>
> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org> wrote:
>
>> It doesn't need to be extern "C". I'll change that. I just was in the
>> mindset of defining a C interface :-/
>>
>> I think it needs to be exposed to developers though as they will need to
>> call it as part of GL setup:
>>
>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>
>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>
>>>> On 2014/01/13 23:26:56, piman wrote:
>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>> darin@chromium.org>
>>>> wrote:
>>>> >
>>>> > > If the GL library creates a background thread w/ a MessageLoop,
>>>> then it
>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>>> > > implementation for that thread.
>>>> > >
>>>> > > -Darin
>>>> > >
>>>> >
>>>> > Fair enough. It doesn't do that today, but if it does in the future, I
>>>> > agree that it needs its own AsyncWait implementation for the type of
>>>> > message loop it creates.
>>>>
>>>
>>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>>> would it?
>>>> It'd need the AsyncWaiter for its own message loop implementation. (It
>>>> might
>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>>> provided
>>>>
>>> by
>>>
>>>> the app or anything like that. So this is really an internal
>>>> implementation
>>>> detail, not part of the interface with the app.)
>>>>
>>>
>>> (LGTM otherwise FWIW. I remain unconvinced that
>>> MojoGetDefaultAsyncWaiter() a)
>>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>>> possibly,
>>> I'm missing something though.)
>>>
>>>
>>>  >
>>>> > Antoine
>>>> >
>>>> >
>>>> > >
>>>> > >
>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <piman@chromium.org
>>>> >wrote:
>>>> > >
>>>> > >>
>>>> > >>
>>>> > >>
>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>> vtl@google.com>
>>>> wrote:
>>>> > >>
>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>
>>> <piman@chromium.org>wrote:
>>>
>>>> > >>>
>>>> > >>>>
>>>> > >>>>
>>>> > >>>>
>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>
>>> <darin@chromium.org>wrote:
>>>
>>>> > >>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:viettrungluu@chromium.
>>>> org>
>>>> wrote:
>>>> > >>>>>
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/environment/environment.cc
>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>> > >>>>>> It might be helpful to have an explanation about why this is a
>>>> no-op
>>>> > >>>>>> (maybe a file-level comment).
>>>> > >>>>>>
>>>> > >>>>>
>>>> > >>>>> Good idea
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>>> need to
>>>> > >>>>>> pick
>>>> > >>>>>> up the default async waiter from the application?)
>>>> > >>>>>>
>>>> > >>>>>
>>>> > >>>>> My understanding is that the user of the GL library will need to
>>>> > >>>>> initialize the GL
>>>> > >>>>> library passing in the value returned by
>>>> MojoGetDefaultAsyncWaiter().
>>>> > >>>>> That way
>>>> > >>>>> the GL library will have a way to schedule work on the user
>>>> defined
>>>> > >>>>> run loop.
>>>> > >>>>>
>>>> > >>>>
>>>> > >>>> Yes, that's the intention.
>>>> > >>>>
>>>> > >>>
>>>> > >>> But the GL library itself doesn't need to know about
>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>>> definition
>>>> of
>>>> > >>> MojoAsyncWaiter.
>>>> > >>>
>>>> > >>
>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>>>> given to
>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>> semantic/threading
>>>> > >> contract is respected.
>>>> > >>
>>>> > >> Antoine
>>>> > >>
>>>> > >>
>>>> > >>>
>>>> > >>>>
>>>> > >>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>>
>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>> shouldn't be
>>>> > >>>>>> in
>>>> > >>>>>> this file (and could even be a C++ function).
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>> > >>>>>> GetCurrentBuffer();
>>>> > >>>>>> Why "extern"?
>>>> > >>>>>>
>>>> > >>>>>
>>>> > >>>>> Oh, I meant to kill that.
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>>
>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>> > >>>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>
>>>> > >>>
>>>> > >>
>>>> > >
>>>> >
>>>> > To unsubscribe from this group and stop receiving emails from it,
>>>> send an
>>>> email
>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>
>>>
>>>
>>> https://codereview.chromium.org/134253004/
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vtl, 2014-01-14 03:10:42]

Maybe? Where will the GL headers (the ABI part[*]) live?

[*] There may be more non-ABI wrappers that live elsewhere.

On Mon, Jan 13, 2014 at 6:58 PM, Darin Fisher <darin@chromium.org> wrote:

> Makes sense. Is that an argument for moving the definition of
> MojoAsyncWaiter back to system?
> On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>
>> Sure, it needs to be exposed as part of an AsyncWaiter implementation
>> that can be used with our bindings system. But it's not part of the ABI for
>> the GL library, which is why I think it should be in a separate header.
>>
>> To use GL, a developer has no choice but to use MojoGLES2Initialize() and
>> thus no choice but to use AsyncWaiter, but they need not have any
>> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
>> system, they'll want one even if they use their own message loop ... but
>> using our bindings system isn't required.)
>>
>> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org> wrote:
>>
>>> It doesn't need to be extern "C". I'll change that. I just was in the
>>> mindset of defining a C interface :-/
>>>
>>> I think it needs to be exposed to developers though as they will need to
>>> call it as part of GL setup:
>>>
>>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>>
>>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>>
>>>>> On 2014/01/13 23:26:56, piman wrote:
>>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>>> darin@chromium.org>
>>>>> wrote:
>>>>> >
>>>>> > > If the GL library creates a background thread w/ a MessageLoop,
>>>>> then it
>>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>>>> > > implementation for that thread.
>>>>> > >
>>>>> > > -Darin
>>>>> > >
>>>>> >
>>>>> > Fair enough. It doesn't do that today, but if it does in the future,
>>>>> I
>>>>> > agree that it needs its own AsyncWait implementation for the type of
>>>>> > message loop it creates.
>>>>>
>>>>
>>>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>>>> would it?
>>>>> It'd need the AsyncWaiter for its own message loop implementation. (It
>>>>> might
>>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>>>> provided
>>>>>
>>>> by
>>>>
>>>>> the app or anything like that. So this is really an internal
>>>>> implementation
>>>>> detail, not part of the interface with the app.)
>>>>>
>>>>
>>>> (LGTM otherwise FWIW. I remain unconvinced that
>>>> MojoGetDefaultAsyncWaiter() a)
>>>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>>>> possibly,
>>>> I'm missing something though.)
>>>>
>>>>
>>>>  >
>>>>> > Antoine
>>>>> >
>>>>> >
>>>>> > >
>>>>> > >
>>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <
>>>>> piman@chromium.org>wrote:
>>>>> > >
>>>>> > >>
>>>>> > >>
>>>>> > >>
>>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>>> vtl@google.com>
>>>>> wrote:
>>>>> > >>
>>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>>
>>>> <piman@chromium.org>wrote:
>>>>
>>>>> > >>>
>>>>> > >>>>
>>>>> > >>>>
>>>>> > >>>>
>>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>>
>>>> <darin@chromium.org>wrote:
>>>>
>>>>> > >>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:
>>>>> viettrungluu@chromium.org>
>>>>> wrote:
>>>>> > >>>>>
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/environment/environment.cc
>>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>>> > >>>>>> It might be helpful to have an explanation about why this is
>>>>> a no-op
>>>>> > >>>>>> (maybe a file-level comment).
>>>>> > >>>>>>
>>>>> > >>>>>
>>>>> > >>>>> Good idea
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>>>> need to
>>>>> > >>>>>> pick
>>>>> > >>>>>> up the default async waiter from the application?)
>>>>> > >>>>>>
>>>>> > >>>>>
>>>>> > >>>>> My understanding is that the user of the GL library will need
>>>>> to
>>>>> > >>>>> initialize the GL
>>>>> > >>>>> library passing in the value returned by
>>>>> MojoGetDefaultAsyncWaiter().
>>>>> > >>>>> That way
>>>>> > >>>>> the GL library will have a way to schedule work on the user
>>>>> defined
>>>>> > >>>>> run loop.
>>>>> > >>>>>
>>>>> > >>>>
>>>>> > >>>> Yes, that's the intention.
>>>>> > >>>>
>>>>> > >>>
>>>>> > >>> But the GL library itself doesn't need to know about
>>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>>>> definition
>>>>> of
>>>>> > >>> MojoAsyncWaiter.
>>>>> > >>>
>>>>> > >>
>>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>>>>> given to
>>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>>> semantic/threading
>>>>> > >> contract is respected.
>>>>> > >>
>>>>> > >> Antoine
>>>>> > >>
>>>>> > >>
>>>>> > >>>
>>>>> > >>>>
>>>>> > >>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>>
>>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>>> shouldn't be
>>>>> > >>>>>> in
>>>>> > >>>>>> this file (and could even be a C++ function).
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>> > >>>>>> GetCurrentBuffer();
>>>>> > >>>>>> Why "extern"?
>>>>> > >>>>>>
>>>>> > >>>>>
>>>>> > >>>>> Oh, I meant to kill that.
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>>>
>>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>>> > >>>>>>
>>>>> > >>>>>
>>>>> > >>>>>
>>>>> > >>>>
>>>>> > >>>
>>>>> > >>
>>>>> > >
>>>>> >
>>>>> > To unsubscribe from this group and stop receiving emails from it,
>>>>> send an
>>>>> email
>>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>
>>>>
>>>>
>>>> https://codereview.chromium.org/134253004/
>>>>
>>>
>>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-01-14 03:18:06]

public/gles2 should have the GL library's ABI.

Keep in mind that Bindings has a dependency on both MojoAsyncWaiter as well
as the function to get the default impl.

So, we probably need some place to put ABI that both bindings and gles2 can
depend on. If public/environment is for non-ABI, then perhaps that only
leaves public/system for MojoAsyncWaiter. Otherwise, we'd need yet another
subdir under public :-/

We could also put gles2 in system/ to make it clear that everything in
system/ is ABI and the rest is just optional support libs.

-Darin
On Jan 13, 2014 7:10 PM, "Viet-Trung Luu" <vtl@google.com> wrote:

> Maybe? Where will the GL headers (the ABI part[*]) live?
>
> [*] There may be more non-ABI wrappers that live elsewhere.
>
> On Mon, Jan 13, 2014 at 6:58 PM, Darin Fisher <darin@chromium.org> wrote:
>
>> Makes sense. Is that an argument for moving the definition of
>> MojoAsyncWaiter back to system?
>> On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>
>>> Sure, it needs to be exposed as part of an AsyncWaiter implementation
>>> that can be used with our bindings system. But it's not part of the ABI for
>>> the GL library, which is why I think it should be in a separate header.
>>>
>>> To use GL, a developer has no choice but to use MojoGLES2Initialize()
>>> and thus no choice but to use AsyncWaiter, but they need not have any
>>> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
>>> system, they'll want one even if they use their own message loop ... but
>>> using our bindings system isn't required.)
>>>
>>> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org>wrote:
>>>
>>>> It doesn't need to be extern "C". I'll change that. I just was in the
>>>> mindset of defining a C interface :-/
>>>>
>>>> I think it needs to be exposed to developers though as they will need
>>>> to call it as part of GL setup:
>>>>
>>>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>>>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>>>
>>>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>>>
>>>>>> On 2014/01/13 23:26:56, piman wrote:
>>>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>>>> darin@chromium.org>
>>>>>> wrote:
>>>>>> >
>>>>>> > > If the GL library creates a background thread w/ a MessageLoop,
>>>>>> then it
>>>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>>>>> > > implementation for that thread.
>>>>>> > >
>>>>>> > > -Darin
>>>>>> > >
>>>>>> >
>>>>>> > Fair enough. It doesn't do that today, but if it does in the
>>>>>> future, I
>>>>>> > agree that it needs its own AsyncWait implementation for the type of
>>>>>> > message loop it creates.
>>>>>>
>>>>>
>>>>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>>>>> would it?
>>>>>> It'd need the AsyncWaiter for its own message loop implementation.
>>>>>> (It might
>>>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>>>>> provided
>>>>>>
>>>>> by
>>>>>
>>>>>> the app or anything like that. So this is really an internal
>>>>>> implementation
>>>>>> detail, not part of the interface with the app.)
>>>>>>
>>>>>
>>>>> (LGTM otherwise FWIW. I remain unconvinced that
>>>>> MojoGetDefaultAsyncWaiter() a)
>>>>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>>>>> possibly,
>>>>> I'm missing something though.)
>>>>>
>>>>>
>>>>>  >
>>>>>> > Antoine
>>>>>> >
>>>>>> >
>>>>>> > >
>>>>>> > >
>>>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <
>>>>>> piman@chromium.org>wrote:
>>>>>> > >
>>>>>> > >>
>>>>>> > >>
>>>>>> > >>
>>>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>>>> vtl@google.com>
>>>>>> wrote:
>>>>>> > >>
>>>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>>>
>>>>> <piman@chromium.org>wrote:
>>>>>
>>>>>> > >>>
>>>>>> > >>>>
>>>>>> > >>>>
>>>>>> > >>>>
>>>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>>>
>>>>> <darin@chromium.org>wrote:
>>>>>
>>>>>> > >>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:
>>>>>> viettrungluu@chromium.org>
>>>>>> wrote:
>>>>>> > >>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/environment/environment.cc
>>>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>>>> > >>>>>> It might be helpful to have an explanation about why this is
>>>>>> a no-op
>>>>>> > >>>>>> (maybe a file-level comment).
>>>>>> > >>>>>>
>>>>>> > >>>>>
>>>>>> > >>>>> Good idea
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>>>>> need to
>>>>>> > >>>>>> pick
>>>>>> > >>>>>> up the default async waiter from the application?)
>>>>>> > >>>>>>
>>>>>> > >>>>>
>>>>>> > >>>>> My understanding is that the user of the GL library will need
>>>>>> to
>>>>>> > >>>>> initialize the GL
>>>>>> > >>>>> library passing in the value returned by
>>>>>> MojoGetDefaultAsyncWaiter().
>>>>>> > >>>>> That way
>>>>>> > >>>>> the GL library will have a way to schedule work on the user
>>>>>> defined
>>>>>> > >>>>> run loop.
>>>>>> > >>>>>
>>>>>> > >>>>
>>>>>> > >>>> Yes, that's the intention.
>>>>>> > >>>>
>>>>>> > >>>
>>>>>> > >>> But the GL library itself doesn't need to know about
>>>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>>>>> definition
>>>>>> of
>>>>>> > >>> MojoAsyncWaiter.
>>>>>> > >>>
>>>>>> > >>
>>>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter it's
>>>>>> given to
>>>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>>>> semantic/threading
>>>>>> > >> contract is respected.
>>>>>> > >>
>>>>>> > >> Antoine
>>>>>> > >>
>>>>>> > >>
>>>>>> > >>>
>>>>>> > >>>>
>>>>>> > >>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>>>> shouldn't be
>>>>>> > >>>>>> in
>>>>>> > >>>>>> this file (and could even be a C++ function).
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>>> > >>>>>> GetCurrentBuffer();
>>>>>> > >>>>>> Why "extern"?
>>>>>> > >>>>>>
>>>>>> > >>>>>
>>>>>> > >>>>> Oh, I meant to kill that.
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>>>> > >>>>>>
>>>>>> > >>>>>
>>>>>> > >>>>>
>>>>>> > >>>>
>>>>>> > >>>
>>>>>> > >>
>>>>>> > >
>>>>>> >
>>>>>> > To unsubscribe from this group and stop receiving emails from it,
>>>>>> send an
>>>>>> email
>>>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>>
>>>>>
>>>>>
>>>>> https://codereview.chromium.org/134253004/
>>>>>
>>>>
>>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vtl, 2014-01-14 03:20:26]

system seems fine to me (esp. since the definition of MojoAsyncWaiter only
requires a header file). Having gles2 depend on system seems okay. (I also
don't oppose putting gles2 in system, except for the path bloat issue.)

On Mon, Jan 13, 2014 at 7:18 PM, Darin Fisher <darin@chromium.org> wrote:

> public/gles2 should have the GL library's ABI.
>
> Keep in mind that Bindings has a dependency on both MojoAsyncWaiter as
> well as the function to get the default impl.
>
> So, we probably need some place to put ABI that both bindings and gles2
> can depend on. If public/environment is for non-ABI, then perhaps that only
> leaves public/system for MojoAsyncWaiter. Otherwise, we'd need yet another
> subdir under public :-/
>
> We could also put gles2 in system/ to make it clear that everything in
> system/ is ABI and the rest is just optional support libs.
>
> -Darin
> On Jan 13, 2014 7:10 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>
>> Maybe? Where will the GL headers (the ABI part[*]) live?
>>
>> [*] There may be more non-ABI wrappers that live elsewhere.
>>
>> On Mon, Jan 13, 2014 at 6:58 PM, Darin Fisher <darin@chromium.org> wrote:
>>
>>> Makes sense. Is that an argument for moving the definition of
>>> MojoAsyncWaiter back to system?
>>> On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>>
>>>> Sure, it needs to be exposed as part of an AsyncWaiter implementation
>>>> that can be used with our bindings system. But it's not part of the ABI
for
>>>> the GL library, which is why I think it should be in a separate header.
>>>>
>>>> To use GL, a developer has no choice but to use MojoGLES2Initialize()
>>>> and thus no choice but to use AsyncWaiter, but they need not have any
>>>> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
>>>> system, they'll want one even if they use their own message loop ... but
>>>> using our bindings system isn't required.)
>>>>
>>>> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org>wrote:
>>>>
>>>>> It doesn't need to be extern "C". I'll change that. I just was in the
>>>>> mindset of defining a C interface :-/
>>>>>
>>>>> I think it needs to be exposed to developers though as they will need
>>>>> to call it as part of GL setup:
>>>>>
>>>>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>>>>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>>>>
>>>>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>>>>
>>>>>>> On 2014/01/13 23:26:56, piman wrote:
>>>>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>>>>> darin@chromium.org>
>>>>>>> wrote:
>>>>>>> >
>>>>>>> > > If the GL library creates a background thread w/ a MessageLoop,
>>>>>>> then it
>>>>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>>>>>> > > implementation for that thread.
>>>>>>> > >
>>>>>>> > > -Darin
>>>>>>> > >
>>>>>>> >
>>>>>>> > Fair enough. It doesn't do that today, but if it does in the
>>>>>>> future, I
>>>>>>> > agree that it needs its own AsyncWait implementation for the type
>>>>>>> of
>>>>>>> > message loop it creates.
>>>>>>>
>>>>>>
>>>>>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>>>>>> would it?
>>>>>>> It'd need the AsyncWaiter for its own message loop implementation.
>>>>>>> (It might
>>>>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not one
>>>>>>> provided
>>>>>>>
>>>>>> by
>>>>>>
>>>>>>> the app or anything like that. So this is really an internal
>>>>>>> implementation
>>>>>>> detail, not part of the interface with the app.)
>>>>>>>
>>>>>>
>>>>>> (LGTM otherwise FWIW. I remain unconvinced that
>>>>>> MojoGetDefaultAsyncWaiter() a)
>>>>>> needs to be extern "C", b) should be defined in async_waiter.h. Quite
>>>>>> possibly,
>>>>>> I'm missing something though.)
>>>>>>
>>>>>>
>>>>>>  >
>>>>>>> > Antoine
>>>>>>> >
>>>>>>> >
>>>>>>> > >
>>>>>>> > >
>>>>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <
>>>>>>> piman@chromium.org>wrote:
>>>>>>> > >
>>>>>>> > >>
>>>>>>> > >>
>>>>>>> > >>
>>>>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>>>>> vtl@google.com>
>>>>>>> wrote:
>>>>>>> > >>
>>>>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>>>>
>>>>>> <piman@chromium.org>wrote:
>>>>>>
>>>>>>> > >>>
>>>>>>> > >>>>
>>>>>>> > >>>>
>>>>>>> > >>>>
>>>>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>>>>
>>>>>> <darin@chromium.org>wrote:
>>>>>>
>>>>>>> > >>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:
>>>>>>> viettrungluu@chromium.org>
>>>>>>> wrote:
>>>>>>> > >>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/environment/environment.cc
>>>>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>>>>> > >>>>>> It might be helpful to have an explanation about why this
>>>>>>> is a no-op
>>>>>>> > >>>>>> (maybe a file-level comment).
>>>>>>> > >>>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>> Good idea
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h:37: MojoAsyncWaiter*
>>>>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>>>>> > >>>>>> Will this be needed by the GL library? (As in, will it ever
>>>>>>> need to
>>>>>>> > >>>>>> pick
>>>>>>> > >>>>>> up the default async waiter from the application?)
>>>>>>> > >>>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>> My understanding is that the user of the GL library will
>>>>>>> need to
>>>>>>> > >>>>> initialize the GL
>>>>>>> > >>>>> library passing in the value returned by
>>>>>>> MojoGetDefaultAsyncWaiter().
>>>>>>> > >>>>> That way
>>>>>>> > >>>>> the GL library will have a way to schedule work on the user
>>>>>>> defined
>>>>>>> > >>>>> run loop.
>>>>>>> > >>>>>
>>>>>>> > >>>>
>>>>>>> > >>>> Yes, that's the intention.
>>>>>>> > >>>>
>>>>>>> > >>>
>>>>>>> > >>> But the GL library itself doesn't need to know about
>>>>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>>>>>> definition
>>>>>>> of
>>>>>>> > >>> MojoAsyncWaiter.
>>>>>>> > >>>
>>>>>>> > >>
>>>>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter
>>>>>>> it's given to
>>>>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>>>>> semantic/threading
>>>>>>> > >> contract is respected.
>>>>>>> > >>
>>>>>>> > >> Antoine
>>>>>>> > >>
>>>>>>> > >>
>>>>>>> > >>>
>>>>>>> > >>>>
>>>>>>> > >>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>>>>> shouldn't be
>>>>>>> > >>>>>> in
>>>>>>> > >>>>>> this file (and could even be a C++ function).
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>>>> > >>>>>> GetCurrentBuffer();
>>>>>>> > >>>>>> Why "extern"?
>>>>>>> > >>>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>> Oh, I meant to kill that.
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>>>>> > >>>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>
>>>>>>> > >>>
>>>>>>> > >>
>>>>>>> > >
>>>>>>> >
>>>>>>> > To unsubscribe from this group and stop receiving emails from it,
>>>>>>> send an
>>>>>>> email
>>>>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> https://codereview.chromium.org/134253004/
>>>>>>
>>>>>
>>>>
>>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-01-14 03:22:49]

Ok... I'll make it so. The default waiter will live in
environment/default_a sync_waiter.h. Sound good?
On Jan 13, 2014 7:20 PM, "Viet-Trung Luu" <vtl@google.com> wrote:

> system seems fine to me (esp. since the definition of MojoAsyncWaiter only
> requires a header file). Having gles2 depend on system seems okay. (I also
> don't oppose putting gles2 in system, except for the path bloat issue.)
>
> On Mon, Jan 13, 2014 at 7:18 PM, Darin Fisher <darin@chromium.org> wrote:
>
>> public/gles2 should have the GL library's ABI.
>>
>> Keep in mind that Bindings has a dependency on both MojoAsyncWaiter as
>> well as the function to get the default impl.
>>
>> So, we probably need some place to put ABI that both bindings and gles2
>> can depend on. If public/environment is for non-ABI, then perhaps that only
>> leaves public/system for MojoAsyncWaiter. Otherwise, we'd need yet another
>> subdir under public :-/
>>
>> We could also put gles2 in system/ to make it clear that everything in
>> system/ is ABI and the rest is just optional support libs.
>>
>> -Darin
>> On Jan 13, 2014 7:10 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>
>>> Maybe? Where will the GL headers (the ABI part[*]) live?
>>>
>>> [*] There may be more non-ABI wrappers that live elsewhere.
>>>
>>> On Mon, Jan 13, 2014 at 6:58 PM, Darin Fisher <darin@chromium.org>wrote:
>>>
>>>> Makes sense. Is that an argument for moving the definition of
>>>> MojoAsyncWaiter back to system?
>>>> On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>>>
>>>>> Sure, it needs to be exposed as part of an AsyncWaiter implementation
>>>>> that can be used with our bindings system. But it's not part of the ABI
for
>>>>> the GL library, which is why I think it should be in a separate header.
>>>>>
>>>>> To use GL, a developer has no choice but to use MojoGLES2Initialize()
>>>>> and thus no choice but to use AsyncWaiter, but they need not have any
>>>>> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
>>>>> system, they'll want one even if they use their own message loop ... but
>>>>> using our bindings system isn't required.)
>>>>>
>>>>> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org>wrote:
>>>>>
>>>>>> It doesn't need to be extern "C". I'll change that. I just was in the
>>>>>> mindset of defining a C interface :-/
>>>>>>
>>>>>> I think it needs to be exposed to developers though as they will need
>>>>>> to call it as part of GL setup:
>>>>>>
>>>>>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>>>>>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>>>>>
>>>>>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>>>>>
>>>>>>>> On 2014/01/13 23:26:56, piman wrote:
>>>>>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>>>>>> darin@chromium.org>
>>>>>>>> wrote:
>>>>>>>> >
>>>>>>>> > > If the GL library creates a background thread w/ a MessageLoop,
>>>>>>>> then it
>>>>>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an AsyncWait
>>>>>>>> > > implementation for that thread.
>>>>>>>> > >
>>>>>>>> > > -Darin
>>>>>>>> > >
>>>>>>>> >
>>>>>>>> > Fair enough. It doesn't do that today, but if it does in the
>>>>>>>> future, I
>>>>>>>> > agree that it needs its own AsyncWait implementation for the type
>>>>>>>> of
>>>>>>>> > message loop it creates.
>>>>>>>>
>>>>>>>
>>>>>>>  I'm confused. It wouldn't call the app's MojoGetDefaultAsyncWaiter,
>>>>>>>> would it?
>>>>>>>> It'd need the AsyncWaiter for its own message loop implementation.
>>>>>>>> (It might
>>>>>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not
>>>>>>>> one provided
>>>>>>>>
>>>>>>> by
>>>>>>>
>>>>>>>> the app or anything like that. So this is really an internal
>>>>>>>> implementation
>>>>>>>> detail, not part of the interface with the app.)
>>>>>>>>
>>>>>>>
>>>>>>> (LGTM otherwise FWIW. I remain unconvinced that
>>>>>>> MojoGetDefaultAsyncWaiter() a)
>>>>>>> needs to be extern "C", b) should be defined in async_waiter.h.
>>>>>>> Quite possibly,
>>>>>>> I'm missing something though.)
>>>>>>>
>>>>>>>
>>>>>>>  >
>>>>>>>> > Antoine
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > >
>>>>>>>> > >
>>>>>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <
>>>>>>>> piman@chromium.org>wrote:
>>>>>>>> > >
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>>>>>> vtl@google.com>
>>>>>>>> wrote:
>>>>>>>> > >>
>>>>>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>>>>>
>>>>>>> <piman@chromium.org>wrote:
>>>>>>>
>>>>>>>> > >>>
>>>>>>>> > >>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>>>>>
>>>>>>> <darin@chromium.org>wrote:
>>>>>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:
>>>>>>>> viettrungluu@chromium.org>
>>>>>>>> wrote:
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/environment/environment.cc
>>>>>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>>>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>>>>>> > >>>>>> It might be helpful to have an explanation about why this
>>>>>>>> is a no-op
>>>>>>>> > >>>>>> (maybe a file-level comment).
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> Good idea
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>>>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h:37:
>>>>>>>> MojoAsyncWaiter*
>>>>>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>>>>>> > >>>>>> Will this be needed by the GL library? (As in, will it
>>>>>>>> ever need to
>>>>>>>> > >>>>>> pick
>>>>>>>> > >>>>>> up the default async waiter from the application?)
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> My understanding is that the user of the GL library will
>>>>>>>> need to
>>>>>>>> > >>>>> initialize the GL
>>>>>>>> > >>>>> library passing in the value returned by
>>>>>>>> MojoGetDefaultAsyncWaiter().
>>>>>>>> > >>>>> That way
>>>>>>>> > >>>>> the GL library will have a way to schedule work on the user
>>>>>>>> defined
>>>>>>>> > >>>>> run loop.
>>>>>>>> > >>>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>> Yes, that's the intention.
>>>>>>>> > >>>>
>>>>>>>> > >>>
>>>>>>>> > >>> But the GL library itself doesn't need to know about
>>>>>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know the
>>>>>>>> definition
>>>>>>>> of
>>>>>>>> > >>> MojoAsyncWaiter.
>>>>>>>> > >>>
>>>>>>>> > >>
>>>>>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter
>>>>>>>> it's given to
>>>>>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>>>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>>>>>> semantic/threading
>>>>>>>> > >> contract is respected.
>>>>>>>> > >>
>>>>>>>> > >> Antoine
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>> > >>>
>>>>>>>> > >>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>>>>>> shouldn't be
>>>>>>>> > >>>>>> in
>>>>>>>> > >>>>>> this file (and could even be a C++ function).
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>>>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>>>>> > >>>>>> GetCurrentBuffer();
>>>>>>>> > >>>>>> Why "extern"?
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> Oh, I meant to kill that.
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>
>>>>>>>> > >>
>>>>>>>> > >
>>>>>>>> >
>>>>>>>> > To unsubscribe from this group and stop receiving emails from it,
>>>>>>>> send an
>>>>>>>> email
>>>>>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> https://codereview.chromium.org/134253004/
>>>>>>>
>>>>>>
>>>>>
>>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vtl, 2014-01-14 04:23:38]

SGTM.
On Jan 13, 2014 7:22 PM, "Darin Fisher" <darin@chromium.org> wrote:

> Ok... I'll make it so. The default waiter will live in
> environment/default_a sync_waiter.h. Sound good?
> On Jan 13, 2014 7:20 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>
>> system seems fine to me (esp. since the definition of MojoAsyncWaiter
>> only requires a header file). Having gles2 depend on system seems okay. (I
>> also don't oppose putting gles2 in system, except for the path bloat issue.)
>>
>> On Mon, Jan 13, 2014 at 7:18 PM, Darin Fisher <darin@chromium.org> wrote:
>>
>>> public/gles2 should have the GL library's ABI.
>>>
>>> Keep in mind that Bindings has a dependency on both MojoAsyncWaiter as
>>> well as the function to get the default impl.
>>>
>>> So, we probably need some place to put ABI that both bindings and gles2
>>> can depend on. If public/environment is for non-ABI, then perhaps that only
>>> leaves public/system for MojoAsyncWaiter. Otherwise, we'd need yet another
>>> subdir under public :-/
>>>
>>> We could also put gles2 in system/ to make it clear that everything in
>>> system/ is ABI and the rest is just optional support libs.
>>>
>>> -Darin
>>> On Jan 13, 2014 7:10 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>>
>>>> Maybe? Where will the GL headers (the ABI part[*]) live?
>>>>
>>>> [*] There may be more non-ABI wrappers that live elsewhere.
>>>>
>>>> On Mon, Jan 13, 2014 at 6:58 PM, Darin Fisher <darin@chromium.org>wrote:
>>>>
>>>>> Makes sense. Is that an argument for moving the definition of
>>>>> MojoAsyncWaiter back to system?
>>>>> On Jan 13, 2014 6:51 PM, "Viet-Trung Luu" <vtl@google.com> wrote:
>>>>>
>>>>>> Sure, it needs to be exposed as part of an AsyncWaiter implementation
>>>>>> that can be used with our bindings system. But it's not part of the ABI
for
>>>>>> the GL library, which is why I think it should be in a separate header.
>>>>>>
>>>>>> To use GL, a developer has no choice but to use MojoGLES2Initialize()
>>>>>> and thus no choice but to use AsyncWaiter, but they need not have any
>>>>>> GetDefaultAsyncWaiter() defined. (In practice, if they use our bindings
>>>>>> system, they'll want one even if they use their own message loop ... but
>>>>>> using our bindings system isn't required.)
>>>>>>
>>>>>> On Mon, Jan 13, 2014 at 6:38 PM, Darin Fisher <darin@chromium.org>wrote:
>>>>>>
>>>>>>> It doesn't need to be extern "C". I'll change that. I just was in
>>>>>>> the mindset of defining a C interface :-/
>>>>>>>
>>>>>>> I think it needs to be exposed to developers though as they will
>>>>>>> need to call it as part of GL setup:
>>>>>>>
>>>>>>> MojoGLES2Initialize(..., mojo::GetDefaultAsyncWaiter());
>>>>>>> On Jan 13, 2014 4:12 PM, <viettrungluu@chromium.org> wrote:
>>>>>>>
>>>>>>>> On 2014/01/13 23:47:22, viettrungluu wrote:
>>>>>>>>
>>>>>>>>> On 2014/01/13 23:26:56, piman wrote:
>>>>>>>>> > On Mon, Jan 13, 2014 at 3:24 PM, Darin Fisher <mailto:
>>>>>>>>> darin@chromium.org>
>>>>>>>>> wrote:
>>>>>>>>> >
>>>>>>>>> > > If the GL library creates a background thread w/ a
>>>>>>>>> MessageLoop, then it
>>>>>>>>> > > would need to call MojoGetDefaultAsyncWaiter to get an
>>>>>>>>> AsyncWait
>>>>>>>>> > > implementation for that thread.
>>>>>>>>> > >
>>>>>>>>> > > -Darin
>>>>>>>>> > >
>>>>>>>>> >
>>>>>>>>> > Fair enough. It doesn't do that today, but if it does in the
>>>>>>>>> future, I
>>>>>>>>> > agree that it needs its own AsyncWait implementation for the
>>>>>>>>> type of
>>>>>>>>> > message loop it creates.
>>>>>>>>>
>>>>>>>>
>>>>>>>>  I'm confused. It wouldn't call the app's
>>>>>>>>> MojoGetDefaultAsyncWaiter, would it?
>>>>>>>>> It'd need the AsyncWaiter for its own message loop implementation.
>>>>>>>>> (It might
>>>>>>>>> then need to call the shell's MojoGetDefaultAsyncWaiter, but not
>>>>>>>>> one provided
>>>>>>>>>
>>>>>>>> by
>>>>>>>>
>>>>>>>>> the app or anything like that. So this is really an internal
>>>>>>>>> implementation
>>>>>>>>> detail, not part of the interface with the app.)
>>>>>>>>>
>>>>>>>>
>>>>>>>> (LGTM otherwise FWIW. I remain unconvinced that
>>>>>>>> MojoGetDefaultAsyncWaiter() a)
>>>>>>>> needs to be extern "C", b) should be defined in async_waiter.h.
>>>>>>>> Quite possibly,
>>>>>>>> I'm missing something though.)
>>>>>>>>
>>>>>>>>
>>>>>>>>  >
>>>>>>>>> > Antoine
>>>>>>>>> >
>>>>>>>>> >
>>>>>>>>> > >
>>>>>>>>> > >
>>>>>>>>> > > On Mon, Jan 13, 2014 at 2:11 PM, Antoine Labour <
>>>>>>>>> piman@chromium.org>wrote:
>>>>>>>>> > >
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>> > >> On Mon, Jan 13, 2014 at 2:05 PM, Viet-Trung Luu <mailto:
>>>>>>>>> vtl@google.com>
>>>>>>>>> wrote:
>>>>>>>>> > >>
>>>>>>>>> > >>> On Mon, Jan 13, 2014 at 1:54 PM, Antoine Labour
>>>>>>>>>
>>>>>>>> <piman@chromium.org>wrote:
>>>>>>>>
>>>>>>>>> > >>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>> On Mon, Jan 13, 2014 at 1:53 PM, Darin Fisher
>>>>>>>>>
>>>>>>>> <darin@chromium.org>wrote:
>>>>>>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> On Mon, Jan 13, 2014 at 1:47 PM, <mailto:
>>>>>>>>> viettrungluu@chromium.org>
>>>>>>>>> wrote:
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/environment/environment.cc
>>>>>>>>> > >>>>>> File mojo/environment/environment.cc (right):
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/environment/environment.cc#newcode10
>>>>>>>>> > >>>>>> mojo/environment/environment.cc:10: // no-op
>>>>>>>>> > >>>>>> It might be helpful to have an explanation about why this
>>>>>>>>> is a no-op
>>>>>>>>> > >>>>>> (maybe a file-level comment).
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> Good idea
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h
>>>>>>>>> > >>>>>> File mojo/public/environment/async_waiter.h (right):
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h#newcode37
>>>>>>>>> > >>>>>> mojo/public/environment/async_waiter.h:37:
>>>>>>>>> MojoAsyncWaiter*
>>>>>>>>> > >>>>>> MojoGetDefaultAsyncWaiter();
>>>>>>>>> > >>>>>> Will this be needed by the GL library? (As in, will it
>>>>>>>>> ever need to
>>>>>>>>> > >>>>>> pick
>>>>>>>>> > >>>>>> up the default async waiter from the application?)
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> My understanding is that the user of the GL library will
>>>>>>>>> need to
>>>>>>>>> > >>>>> initialize the GL
>>>>>>>>> > >>>>> library passing in the value returned by
>>>>>>>>> MojoGetDefaultAsyncWaiter().
>>>>>>>>> > >>>>> That way
>>>>>>>>> > >>>>> the GL library will have a way to schedule work on the
>>>>>>>>> user defined
>>>>>>>>> > >>>>> run loop.
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>> Yes, that's the intention.
>>>>>>>>> > >>>>
>>>>>>>>> > >>>
>>>>>>>>> > >>> But the GL library itself doesn't need to know about
>>>>>>>>> > >>> MojoGetDefaultAsyncWaiter(), right? It only needs to know
>>>>>>>>> the definition
>>>>>>>>> of
>>>>>>>>> > >>> MojoAsyncWaiter.
>>>>>>>>> > >>>
>>>>>>>>> > >>
>>>>>>>>> > >>  Correct. It would explicitly pass whatever MojoAsyncWaiter
>>>>>>>>> it's given to
>>>>>>>>> > >> RemotePtr/Connector. It technically doesn't care whether
>>>>>>>>> > >> it's MojoGetDefaultAsyncWaiter or not, as long as the
>>>>>>>>> semantic/threading
>>>>>>>>> > >> contract is respected.
>>>>>>>>> > >>
>>>>>>>>> > >> Antoine
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>> > >>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> If not (and it's only needed for the bindings), maybe it
>>>>>>>>> shouldn't be
>>>>>>>>> > >>>>>> in
>>>>>>>>> > >>>>>> this file (and could even be a C++ function).
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h
>>>>>>>>> > >>>>>> File mojo/public/environment/buffer_tls.h (right):
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/diff/770001/
>>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h#newcode14
>>>>>>>>> > >>>>>> mojo/public/environment/buffer_tls.h:14: extern Buffer*
>>>>>>>>> > >>>>>> GetCurrentBuffer();
>>>>>>>>> > >>>>>> Why "extern"?
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> Oh, I meant to kill that.
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> https://codereview.chromium.org/134253004/
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>
>>>>>>>>> > >>
>>>>>>>>> > >
>>>>>>>>> >
>>>>>>>>> > To unsubscribe from this group and stop receiving emails from
>>>>>>>>> it, send an
>>>>>>>>> email
>>>>>>>>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> https://codereview.chromium.org/134253004/
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-01-14 08:13:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/darin@chromium.org/134253004/1080001

[commit-bot: I haz the power, 2014-01-14 16:27:54]

Change committed as 244739

[piman, 2014-01-14 23:44:19]

https://codereview.chromium.org/134253004/diff/1080001/mojo/mojo_examples.gypi
File mojo/mojo_examples.gypi (right):

https://codereview.chromium.org/134253004/diff/1080001/mojo/mojo_examples.gyp...
mojo/mojo_examples.gypi:15: 'mojo_environment_chromium',
This should have been mojo_environment_standalone.

It's rather unfortunate that in the current setup, if you use RunLoop and link
against mojo_environment_chromium, you get no compile error, but a hard-to-debug
run-time failure (you actually have to attach the debugger and see that it's
calling into the wrong Environment).

[darin (slow to review), 2014-01-15 04:36:45]

Fascinating.

Hmm, we could fix that by not providing the Environment symbols in
mojo_environment_chromium as they are no-ops and not needed in that context.

That doesn't address the other direction, in which you might accidentally
link against mojo_environment_standalone instead of
mojo_environment_chromium. I wonder if there are some games we could play
with a #define. Hmm...


On Tue, Jan 14, 2014 at 3:44 PM, <piman@chromium.org> wrote:

>
> https://codereview.chromium.org/134253004/diff/1080001/
> mojo/mojo_examples.gypi
> File mojo/mojo_examples.gypi (right):
>
> https://codereview.chromium.org/134253004/diff/1080001/
> mojo/mojo_examples.gypi#newcode15
> mojo/mojo_examples.gypi:15: 'mojo_environment_chromium',
> This should have been mojo_environment_standalone.
>
> It's rather unfortunate that in the current setup, if you use RunLoop
> and link against mojo_environment_chromium, you get no compile error,
> but a hard-to-debug run-time failure (you actually have to attach the
> debugger and see that it's calling into the wrong Environment).
>
> https://codereview.chromium.org/134253004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.