Mojo: Add a platform interface for shared memory (embedder::PlatformSharedBuffer).

Mojo: Add a platform interface for shared memory (embedder::PlatformSharedBuffer).

Make the existing RawSharedBuffer an implementation of that. Later,
we'll plumb in a "PlatformSupport" that'll produce these things, but for
now keep the (static) creation methods where they are.

R=darin@chromium.org

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

[darin (slow to review), 2014-08-14 05:21:28]

LGTM

https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
File mojo/embedder/platform_shared_buffer.h (right):

https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
mojo/embedder/platform_shared_buffer.h:37: : public
base::RefCountedThreadSafe<PlatformSharedBuffer> {
Why reference counted? did you consider approaches to avoiding this? where did
they fall flat?

https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
mojo/embedder/platform_shared_buffer.h:58: virtual
embedder::ScopedPlatformHandle DuplicatePlatformHandle() = 0;
nit: can drop the embedder:: prefixes

[viettrungluu, 2014-08-14 15:29:50]

On 2014/08/14 05:21:28, darin wrote:
> LGTM
> 
>
https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
> File mojo/embedder/platform_shared_buffer.h (right):
> 
>
https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
> mojo/embedder/platform_shared_buffer.h:37: : public
> base::RefCountedThreadSafe<PlatformSharedBuffer> {
> Why reference counted? did you consider approaches to avoiding this? where did
> they fall flat?

Someone has to refcount; it depends on if you want to take a round trip to the
kernel (and occupy space in the FD table) to do it. (The alternative is to
actually duplicate FDs when someone duplicates the shared buffer (Mojo) handle.)

> 
>
https://codereview.chromium.org/471773002/diff/1/mojo/embedder/platform_share...
> mojo/embedder/platform_shared_buffer.h:58: virtual
> embedder::ScopedPlatformHandle DuplicatePlatformHandle() = 0;
> nit: can drop the embedder:: prefixes

Will do.

[darin (slow to review), 2014-08-14 16:03:22]

On Thu, Aug 14, 2014 at 8:29 AM, <viettrungluu@chromium.org> wrote:

> On 2014/08/14 05:21:28, darin wrote:
>
>> LGTM
>>
>
>
> https://codereview.chromium.org/471773002/diff/1/mojo/
> embedder/platform_shared_buffer.h
>
>> File mojo/embedder/platform_shared_buffer.h (right):
>>
>
>
> https://codereview.chromium.org/471773002/diff/1/mojo/
> embedder/platform_shared_buffer.h#newcode37
>
>> mojo/embedder/platform_shared_buffer.h:37: : public
>> base::RefCountedThreadSafe<PlatformSharedBuffer> {
>> Why reference counted? did you consider approaches to avoiding this?
>> where did
>> they fall flat?
>>
>
> Someone has to refcount; it depends on if you want to take a round trip to
> the
> kernel (and occupy space in the FD table) to do it. (The alternative is to
> actually duplicate FDs when someone duplicates the shared buffer (Mojo)
> handle.)
>
>
>
Right. The other option I was considering is hiding the reference counting
behind the
implementation. You could enforce that the only way to reference count the
memory
handle is through higher level methods on PlatformSharedBuffer, like a
Duplicate
method.



>
>
> https://codereview.chromium.org/471773002/diff/1/mojo/
> embedder/platform_shared_buffer.h#newcode58
>
>> mojo/embedder/platform_shared_buffer.h:58: virtual
>> embedder::ScopedPlatformHandle DuplicatePlatformHandle() = 0;
>> nit: can drop the embedder:: prefixes
>>
>
> Will do.
>
> https://codereview.chromium.org/471773002/
>

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

[viettrungluu, 2014-08-14 17:16:28]

On 2014/08/14 16:03:22, darin wrote:
> On Thu, Aug 14, 2014 at 8:29 AM, <mailto:viettrungluu@chromium.org> wrote:
> 
> > On 2014/08/14 05:21:28, darin wrote:
> >
> >> LGTM
> >>
> >
> >
> > https://codereview.chromium.org/471773002/diff/1/mojo/
> > embedder/platform_shared_buffer.h
> >
> >> File mojo/embedder/platform_shared_buffer.h (right):
> >>
> >
> >
> > https://codereview.chromium.org/471773002/diff/1/mojo/
> > embedder/platform_shared_buffer.h#newcode37
> >
> >> mojo/embedder/platform_shared_buffer.h:37: : public
> >> base::RefCountedThreadSafe<PlatformSharedBuffer> {
> >> Why reference counted? did you consider approaches to avoiding this?
> >> where did
> >> they fall flat?
> >>
> >
> > Someone has to refcount; it depends on if you want to take a round trip to
> > the
> > kernel (and occupy space in the FD table) to do it. (The alternative is to
> > actually duplicate FDs when someone duplicates the shared buffer (Mojo)
> > handle.)
> >
> >
> >
> Right. The other option I was considering is hiding the reference counting
> behind the
> implementation. You could enforce that the only way to reference count the
> memory
> handle is through higher level methods on PlatformSharedBuffer, like a
> Duplicate
> method.

Once upon a time, I had code that looked something like this, but it all seemed
like needless indirection.

> 
> 
> 
> >
> >
> > https://codereview.chromium.org/471773002/diff/1/mojo/
> > embedder/platform_shared_buffer.h#newcode58
> >
> >> mojo/embedder/platform_shared_buffer.h:58: virtual
> >> embedder::ScopedPlatformHandle DuplicatePlatformHandle() = 0;
> >> nit: can drop the embedder:: prefixes
> >>
> >
> > Will do.
> >
> > https://codereview.chromium.org/471773002/
> >
> 
> 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-08-14 17:50:46]

OK


On Thu, Aug 14, 2014 at 10:16 AM, <viettrungluu@chromium.org> wrote:

> On 2014/08/14 16:03:22, darin wrote:
>
>  On Thu, Aug 14, 2014 at 8:29 AM, <mailto:viettrungluu@chromium.org>
>> wrote:
>>
>
>  > On 2014/08/14 05:21:28, darin wrote:
>> >
>> >> LGTM
>> >>
>> >
>> >
>> > https://codereview.chromium.org/471773002/diff/1/mojo/
>> > embedder/platform_shared_buffer.h
>> >
>> >> File mojo/embedder/platform_shared_buffer.h (right):
>> >>
>> >
>> >
>> > https://codereview.chromium.org/471773002/diff/1/mojo/
>> > embedder/platform_shared_buffer.h#newcode37
>> >
>> >> mojo/embedder/platform_shared_buffer.h:37: : public
>> >> base::RefCountedThreadSafe<PlatformSharedBuffer> {
>> >> Why reference counted? did you consider approaches to avoiding this?
>> >> where did
>> >> they fall flat?
>> >>
>> >
>> > Someone has to refcount; it depends on if you want to take a round trip
>> to
>> > the
>> > kernel (and occupy space in the FD table) to do it. (The alternative is
>> to
>> > actually duplicate FDs when someone duplicates the shared buffer (Mojo)
>> > handle.)
>> >
>> >
>> >
>> Right. The other option I was considering is hiding the reference counting
>> behind the
>> implementation. You could enforce that the only way to reference count the
>> memory
>> handle is through higher level methods on PlatformSharedBuffer, like a
>> Duplicate
>> method.
>>
>
> Once upon a time, I had code that looked something like this, but it all
> seemed
> like needless indirection.
>
>
>
>
>  >
>> >
>> > https://codereview.chromium.org/471773002/diff/1/mojo/
>>
>> > embedder/platform_shared_buffer.h#newcode58
>> >
>> >> mojo/embedder/platform_shared_buffer.h:58: virtual
>> >> embedder::ScopedPlatformHandle DuplicatePlatformHandle() = 0;
>> >> nit: can drop the embedder:: prefixes
>> >>
>> >
>> > Will do.
>> >
>> > https://codereview.chromium.org/471773002/
>> >
>>
>
>  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/471773002/
>

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

[viettrungluu, 2014-08-14 19:35:51]

Committed patchset #3 manually as 289667 (presubmit successful).