Got questions about Mojo/servicification? Ask 'em now, help the whole community!

103 views
Skip to first unread message

Colin Blundell

unread,
May 22, 2017, 7:41:15 AM5/22/17
to chromium-dev, blink-dev
TL;DR Ask any questions you have about Mojo/services on this thread and help us determine how to improve the docs, benefiting everyone!

Hi all,

As the long-term projects of Mojo-ification and servicification ripple out through the codebase and community, the team has heard feedback that documentation is not always as complete as it could be. We've tried hard to fill in gaps (e.g., Mojo, services, servicification strategies), but of course from our perspective it's not always apparent just where the gaps are :). This is your chance to help open our eyes! Ask any questions that you have about either of these, and we'll both make sure that the questions get answered and that the answers make their way into the documentation!

Thanks,

Colin (on behalf of the services team)

PS If you don't have any questions now but think of some later, services-dev@ is always open!

Eric Seckler

unread,
May 23, 2017, 4:41:04 AM5/23/17
to Colin Blundell, chromium-dev, blink-dev
Here's one thing I was wondering about. I didn't see an answer in the docs you mentioned, but if there is, feel free to point me to it :)

AFAIU, two mojo services can either live in the same process or in different processes. If they live in different processes (and don't run on chrome's IO threads), are their IPCs to each other still proxied through the IO threads? If so, would they not be proxied through the IO threads if they lived in the same process?

Thanks!
Eric

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMGE5NFGKp5f_RDXH8Atz0WXHLUQUTJmd4yJF%3Dota%2B2wVQ4o-g%40mail.gmail.com.

Ken Rockot

unread,
May 23, 2017, 8:44:20 AM5/23/17
to Eric Seckler, Chromium-dev, blink-dev, Colin Blundell


On May 23, 2017 1:41 AM, "Eric Seckler" <esec...@chromium.org> wrote:
Here's one thing I was wondering about. I didn't see an answer in the docs you mentioned, but if there is, feel free to point me to it :)

AFAIU, two mojo services can either live in the same process or in different processes. If they live in different processes (and don't run on chrome's IO threads), are their IPCs to each other still proxied through the IO threads? If so, would they not be proxied through the IO threads if they lived in the same process?

Mojo IPCs are not proxied through any other thread on send. On receive, they only necessarily originate from the IO/IPC thread if coming from out-of-process.

For intraprocess pipes using C++ bindings, a message send is effectively a (serialized) PostTask directly to the receiving end's task runner. If everything goes as planned we will also soon remove the serialization/deserialization steps from the intraprocess case.


Thanks!
Eric

On Mon, May 22, 2017 at 12:41 PM Colin Blundell <blun...@chromium.org> wrote:
TL;DR Ask any questions you have about Mojo/services on this thread and help us determine how to improve the docs, benefiting everyone!

Hi all,

As the long-term projects of Mojo-ification and servicification ripple out through the codebase and community, the team has heard feedback that documentation is not always as complete as it could be. We've tried hard to fill in gaps (e.g., Mojo, services, servicification strategies), but of course from our perspective it's not always apparent just where the gaps are :). This is your chance to help open our eyes! Ask any questions that you have about either of these, and we'll both make sure that the questions get answered and that the answers make their way into the documentation!

Thanks,

Colin (on behalf of the services team)

PS If you don't have any questions now but think of some later, services-dev@ is always open!

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMGE5NFGKp5f_RDXH8Atz0WXHLUQUTJmd4yJF%3Dota%2B2wVQ4o-g%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Fady Samuel

unread,
May 23, 2017, 8:57:02 AM5/23/17
to Eric Seckler, Ken Rockot, Chromium-dev, Colin Blundell, blink-dev
That's cool! Once intraprocess mojo is "free" does it make sense to implement a mojom when you need cross thread communication as well? In other words, is mojo the thing to use for all async communication in chromium and not just when we are thinking about potential process boundaries? Thanks!

Fady

Colin Blundell

unread,
May 23, 2017, 9:01:22 AM5/23/17
to Fady Samuel, Eric Seckler, Ken Rockot, Chromium-dev, Colin Blundell, blink-dev
On Tue, May 23, 2017 at 2:56 PM Fady Samuel <fsa...@chromium.org> wrote:
That's cool! Once intraprocess mojo is "free" does it make sense to implement a mojom when you need cross thread communication as well? In other words, is mojo the thing to use for all async communication in chromium and not just when we are thinking about potential process boundaries? Thanks!

I don't think so. Defining things in mojom and then implementing them in C++ comes with conceptual overhead over just defining them in C++; I think there needs to be concrete benefit to justify that overhead. Additionally, binary size would presumably balloon.

Best,

Colin

Ken Rockot

unread,
May 23, 2017, 9:03:39 AM5/23/17
to Fady Samuel, Chromium-dev, Eric Seckler, blink-dev, Colin Blundell


On May 23, 2017 5:56 AM, "Fady Samuel" <fsa...@chromium.org> wrote:
That's cool! Once intraprocess mojo is "free" does it make sense to implement a mojom when you need cross thread communication as well? In other words, is mojo the thing to use for all async communication in chromium and not just when we are thinking about potential process boundaries? Thanks!

Interesting idea. For now I would still probably avoid taking it that far. :)

I suspect we'll​ want to remain fairly conservative about where we introduce mojom boundaries. Almost free is still not free, and this would be unnecessary complexity. Plus, imagine all the extra typemaps!

Colin Blundell

unread,
May 23, 2017, 9:04:13 AM5/23/17
to Colin Blundell, Fady Samuel, Eric Seckler, Ken Rockot, Chromium-dev, blink-dev
On Tue, May 23, 2017 at 3:01 PM Colin Blundell <blun...@chromium.org> wrote:
On Tue, May 23, 2017 at 2:56 PM Fady Samuel <fsa...@chromium.org> wrote:
That's cool! Once intraprocess mojo is "free" does it make sense to implement a mojom when you need cross thread communication as well? In other words, is mojo the thing to use for all async communication in chromium and not just when we are thinking about potential process boundaries? Thanks!

I don't think so. Defining things in mojom and then implementing them in C++ comes with conceptual overhead over just defining them in C++; I think there needs to be concrete benefit to justify that overhead. Additionally, binary size would presumably balloon.

(That said, totally agreed about how cool the introduction of serialization only when necessary would/will be :).

Colin Blundell

unread,
May 24, 2017, 8:55:23 AM5/24/17
to Colin Blundell, Fady Samuel, Eric Seckler, Ken Rockot, Chromium-dev, blink-dev
As a reminder, if anyone has any questions about Mojo/services/servicification/etc at any point in the future, the folks on services-dev@ will always be happy to help answer them!

Best,

Colin

Gabriel Charette

unread,
May 25, 2017, 11:32:50 AM5/25/17
to Colin Blundell, Fady Samuel, Eric Seckler, Ken Rockot, Chromium-dev, blink-dev

One thing Mojo is breaking for me is my CTAGS (i.e. my editor can no longer GoTo Definition for things defined in mojoms). Maybe this is just my scripts that only parse .h/.cc IIRC. Do you know if CTAGS understand .mojom? Similarly syntax highlighting in my editor doesn't work, again I could probably just tell it which syntax to assume for .mojom files. Is there documentation on how to set up one's dev environment in a Mojo friendly way?


--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGE5NG0cyxgMUuuiyGjoZ_jYaB9V9yt38xUCvwrw2W2eiLhfA%40mail.gmail.com.

Ken Rockot

unread,
May 25, 2017, 11:37:06 AM5/25/17
to Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Thu, May 25, 2017 at 8:32 AM, Gabriel Charette <g...@chromium.org> wrote:

One thing Mojo is breaking for me is my CTAGS (i.e. my editor can no longer GoTo Definition for things defined in mojoms). Maybe this is just my scripts that only parse .h/.cc IIRC. Do you know if CTAGS understand .mojom? Similarly syntax highlighting in my editor doesn't work, again I could probably just tell it which syntax to assume for .mojom files. Is there documentation on how to set up one's dev environment in a Mojo friendly way?

There is definitely no CTAGS support for Mojom, and I'm not aware of anyone creating syntax configs for mojom for any popular editors. It's reasonably close to C++ syntax.

Ken Rockot

unread,
May 25, 2017, 11:37:24 AM5/25/17
to Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
There is definitely no CTAGS support for Mojom, and I'm not aware of anyone creating syntax configs for mojom for any popular editors. It's reasonably close to C++ syntax.
On Thu, May 25, 2017 at 8:32 AM, Gabriel Charette <g...@chromium.org> wrote:

Scott Graham

unread,
May 25, 2017, 1:23:05 PM5/25/17
to Ken Rockot, Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
I had been been using http://ctags.sourceforge.net/ for years, but after Gab's email I saw that some people are trying to rejuvenate it at ctags.io. I just tried adding the option --map-C++=+.mojom and with that, vim is able to jump to mojom interfaces, enums, and structs, but unfortunately not to interface methods, which would be the most useful. It looks like it has some extension points so it'd probably be pokeable, but I didn't feel that motivated yet. (The x64 head build also crashed when I tried to run it on the whole chrome tree recursively, but that was probably unrelated to adding mojom to the spec.)

Re: syntax, assuming you're using the right editor, apparently I made this a while ago https://cs.chromium.org/chromium/src/tools/vim/mojom/syntax/mojom.vim. It might need a bit of updating.


Reilly Grant

unread,
May 25, 2017, 1:29:48 PM5/25/17
to Scott Graham, Ken Rockot, Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
Something I recently realized is broken by Mojo's generated headers is that x-refs can't cross the Blink/Chromium boundary because Blink code calls the Blink variant of an interface and Chromium code calls the Chromium variant of an interface.

Primiano Tucci

unread,
May 25, 2017, 1:55:27 PM5/25/17
to Reilly Grant, Scott Graham, Ken Rockot, Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
One thing that caused a bit of friction for the mojoification of tracing and memory-infra is that we have some core structs in base (because lot of places in the codebase needs to use tracing and memory-infra), hence we cannot depend on the auto-generated mojo stubs from base, hence we had to use typemaps.
It's all good and I understand the dependency rationale, but this ended up with me having to review and maintain some extremely mechanical (and error-prone) code like memory_instrumentation_struct_traits.cc. Here's the question: could we have a tools/generate_struct_traits which takes the base class, takes the mojo file and generates the struct traits file which we then check-in. It would be perfectly find to have to run the tool manually, this at least would remove the burden of reviewing and being careful when editing the code. It seems that there are already 62 struct_traits.cc files in the codebase that people have to maintain, I guess for similar dependency reasons.


Scott Graham

unread,
May 25, 2017, 2:22:19 PM5/25/17
to Ken Rockot, Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev

Colin Blundell

unread,
May 29, 2017, 9:23:56 AM5/29/17
to Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, yzs...@chromium.org, Gabriel Charette, Colin Blundell, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Thu, May 25, 2017 at 7:55 PM Primiano Tucci <prim...@chromium.org> wrote:
One thing that caused a bit of friction for the mojoification of tracing and memory-infra is that we have some core structs in base (because lot of places in the codebase needs to use tracing and memory-infra), hence we cannot depend on the auto-generated mojo stubs from base, hence we had to use typemaps.
It's all good and I understand the dependency rationale, but this ended up with me having to review and maintain some extremely mechanical (and error-prone) code like memory_instrumentation_struct_traits.cc. Here's the question: could we have a tools/generate_struct_traits which takes the base class, takes the mojo file and generates the struct traits file which we then check-in. It would be perfectly find to have to run the tool manually, this at least would remove the burden of reviewing and being careful when editing the code.

+Yuzhu Shen, is this something you've thought about?

Colin Blundell

unread,
May 30, 2017, 3:51:01 AM5/30/17
to Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, yzs...@chromium.org, Gabriel Charette, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
I agree that it's important to continue improving the tooling for Mojo as it gets used more and more widely throughout the codebase. I've filed a master bug and individual bugs for all of the tooling requests that were made in this thread. I don't know offhand how (in)feasible the various desires are in the short/medium-term, but we'll at least be tracking them. 

Best,

Colin

Daniel Murphy

unread,
May 30, 2017, 5:13:07 PM5/30/17
to Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, yzs...@chromium.org, Gabriel Charette, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
I have a question about memory fragmentation & providing buffers -

It seems like in a perfect world mojo will be the spot where we are allocating all memory that has to do with messaging. If I want to send a message, I'll be allocating mojo objects, populating them and sending them. Then on the receiving end I'll be using / storing that data.

What kind of support is available in mojo for:
  • Allocating messages & message structs in a memory-local way with respect to the macro message as a whole - this might require the client providing information like string or vector size.
  • Claiming memory (or asking if we CAN claim memory - it might be shared memory) received using mojo so we don't have to copy.
I'm very interested in eliminating all unnecessary memory copies.

Daniel

Daniel Murphy

unread,
May 30, 2017, 5:13:44 PM5/30/17
to Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, yzs...@chromium.org, Gabriel Charette, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
(resending using chromium address)

I have a question about memory fragmentation & providing buffers -

It seems like in a perfect world mojo will be the spot where we are allocating all memory that has to do with messaging. If I want to send a message, I'll be allocating mojo objects, populating them and sending them. Then on the receiving end I'll be using / storing that data.

What kind of support is available in mojo for:
  • Allocating messages & message structs in a memory-local way with respect to the macro message as a whole - this might require the client providing information like string or vector size.
  • Claiming memory (or asking if we CAN claim memory - it might be shared memory) received using mojo so we don't have to copy.
I'm very interested in eliminating all unnecessary memory copies.

On Tue, May 30, 2017 at 12:50 AM Colin Blundell <blun...@chromium.org> wrote:

Yuzhu Shen

unread,
May 30, 2017, 5:46:17 PM5/30/17
to Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Gabriel Charette, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, May 29, 2017 at 6:23 AM, Colin Blundell <blun...@chromium.org> wrote:


On Thu, May 25, 2017 at 7:55 PM Primiano Tucci <prim...@chromium.org> wrote:
One thing that caused a bit of friction for the mojoification of tracing and memory-infra is that we have some core structs in base (because lot of places in the codebase needs to use tracing and memory-infra), hence we cannot depend on the auto-generated mojo stubs from base, hence we had to use typemaps.
It's all good and I understand the dependency rationale, but this ended up with me having to review and maintain some extremely mechanical (and error-prone) code like memory_instrumentation_struct_traits.cc. Here's the question: could we have a tools/generate_struct_traits which takes the base class, takes the mojo file and generates the struct traits file which we then check-in. It would be perfectly find to have to run the tool manually, this at least would remove the burden of reviewing and being careful when editing the code.

+Yuzhu Shen, is this something you've thought about?

(For people that are interested in this topic, I have replied in this issue: Issue 727557.)

Gabriel Charette

unread,
Jun 5, 2017, 11:48:17 AM6/5/17
to Yuzhu Shen, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Gabriel Charette, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
My main issue with Mojo is that it effectively defines a new language... I keep running into code where I'm like "oh, what's this type?", then I dig in the codebase and can't find it only to eventually figure out it's a type implicitly defined by Mojo at compile-time [1]... IDEs and code search do not understand these links and nor do I... I'm sure it's all pretty and nice once you've grokked the whole language but I have not.

Also, every time I review a CL that touches Mojo code I'm like "how was one supposed to figure out that this was required?" and then "how am I supposed to know as a reviewer that it is correct?", e.g. https://chromium-review.googlesource.com/c/517988/9/services/preferences/public/interfaces/preferences.mojom

It's normal to be exposed to new APIs as a reviewer (same happens with //base all the time), what's annoying is not being able to code search for said unknown things and find an API with documentation. I did find the Mojo documentation online but perhaps it's missing a section about pure syntax/tokens (e.g. what does "=>" mean in the above change, I couldn't find that in the documentation...)

[1] Example of confusion about an implicit Mojo type from a recent CR:

File services/preferences/persistent_pref_store_impl.cc:

  • Patch Set #9, Line 60: CommitPendingWriteCallback

    The interface uses base::OnceClosure and I don't see where CommitPendingWri

    This is an override of mojom::PersistentPrefStore::CommitPendingWrite(). This method takes a CommitPendingWriteCallback as argument. CommitPendingWriteCallback is defined as "using CommitPendingWriteCallback = base::OnceClosure;" in the header generated from preferences.mojom.

(and lastly, in that specific case, defining CommitPendingWriteCallback is more confusing than re-using base::OnceClosure IMO but that's off-topic)

Joe Mason

unread,
Jun 5, 2017, 3:13:24 PM6/5/17
to Gabriel Charette, Yuzhu Shen, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 11:48 AM, Gabriel Charette <g...@chromium.org> wrote:
My main issue with Mojo is that it effectively defines a new language... I keep running into code where I'm like "oh, what's this type?", then I dig in the codebase and can't find it only to eventually figure out it's a type implicitly defined by Mojo at compile-time [1]... IDEs and code search do not understand these links and nor do I... I'm sure it's all pretty and nice once you've grokked the whole language but I have not.

Also, every time I review a CL that touches Mojo code I'm like "how was one supposed to figure out that this was required?" and then "how am I supposed to know as a reviewer that it is correct?", e.g. https://chromium-review.googlesource.com/c/517988/9/services/preferences/public/interfaces/preferences.mojom

It's normal to be exposed to new APIs as a reviewer (same happens with //base all the time), what's annoying is not being able to code search for said unknown things and find an API with documentation. I did find the Mojo documentation online but perhaps it's missing a section about pure syntax/tokens (e.g. what does "=>" mean in the above change, I couldn't find that in the documentation...)


(Looking at the CL you listed - woah, "CommitPendingWrite() => ()" seems weird at first. Without checking, I think the difference is that "CommitPendingWrite()" would pass an asynchronous message, and the caller would never know when the message handler is finished. While "CommitPendingWrite() => ()" gets a callback called with the "return value" of the message handler. In this case the return value has arguments so the callback has no parameters, so the only use is that the caller knows when CommitPendingWrite is finished.)

I agree that Mojo uses a LOT of typedefs which I find hurt discoverability more than they help readability. Are PersistentPrefStorePtr, PersistentPrefStoreRequest and PersistentPrefStorePtrInfo really clearer than mojo::InterfacePtr<PersistentPrefStore>, mojo::InterfaceRequest<PersistentPrefStore>, and mojo::InterfacePtrInfo<PersistentPrefStore>? It's easy to find interface_ptr_info.h and all its documentation in code search by looking for InterfacePtrInfo, which is the natural thing to do if you're not already familiar with Mojo. To know what PersistentPrefStorePtrInfo means you need to already know about that type mapping.

(Also, out of curiosity, why is mojo::Binding<PersistentPrefStore> not typedefined to PersistentPrefStoreBinding like all the others?)

Note that deciding not to use the mojom-generated typedefs in Chromium code shouldn't need any change in Mojo - it would just be a style guide update. So I think it's a worthwhile discussion. Maybe it's worth a separate thread?

 

Yuzhu Shen

unread,
Jun 5, 2017, 3:20:17 PM6/5/17
to Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 12:13 PM, Joe Mason <joenot...@google.com> wrote:
On Mon, Jun 5, 2017 at 11:48 AM, Gabriel Charette <g...@chromium.org> wrote:
My main issue with Mojo is that it effectively defines a new language... I keep running into code where I'm like "oh, what's this type?", then I dig in the codebase and can't find it only to eventually figure out it's a type implicitly defined by Mojo at compile-time [1]... IDEs and code search do not understand these links and nor do I... I'm sure it's all pretty and nice once you've grokked the whole language but I have not.

Also, every time I review a CL that touches Mojo code I'm like "how was one supposed to figure out that this was required?" and then "how am I supposed to know as a reviewer that it is correct?", e.g. https://chromium-review.googlesource.com/c/517988/9/services/preferences/public/interfaces/preferences.mojom

It's normal to be exposed to new APIs as a reviewer (same happens with //base all the time), what's annoying is not being able to code search for said unknown things and find an API with documentation. I did find the Mojo documentation online but perhaps it's missing a section about pure syntax/tokens (e.g. what does "=>" mean in the above change, I couldn't find that in the documentation...)


(Looking at the CL you listed - woah, "CommitPendingWrite() => ()" seems weird at first. Without checking, I think the difference is that "CommitPendingWrite()" would pass an asynchronous message, and the caller would never know when the message handler is finished. While "CommitPendingWrite() => ()" gets a callback called with the "return value" of the message handler. In this case the return value has arguments so the callback has no parameters, so the only use is that the caller knows when CommitPendingWrite is finished.)

I agree that Mojo uses a LOT of typedefs which I find hurt discoverability more than they help readability. Are PersistentPrefStorePtr, PersistentPrefStoreRequest and PersistentPrefStorePtrInfo really clearer than mojo::InterfacePtr<PersistentPrefStore>, mojo::InterfaceRequest<PersistentPrefStore>, and mojo::InterfacePtrInfo<PersistentPrefStore>? It's easy to find interface_ptr_info.h and all its documentation in code search by looking for InterfacePtrInfo, which is the natural thing to do if you're not already familiar with Mojo. To know what PersistentPrefStorePtrInfo means you need to already know about that type mapping.

(Also, out of curiosity, why is mojo::Binding<PersistentPrefStore> not typedefined to PersistentPrefStoreBinding like all the others?)

That is because usually you don't pass mojo::Binding<> around like the other types, and therefore type it less often than the other types.
Some people do like shorter names. (Imagine you need to type mojo::AssociatedInterfacePtrInfo<PersistentPrefStore> over and over. :))
But I agree that it may have some extra learning cost.

Gabriel Charette

unread,
Jun 5, 2017, 3:26:56 PM6/5/17
to Joe Mason, Gabriel Charette, Yuzhu Shen, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 3:13 PM Joe Mason <joenot...@google.com> wrote:
On Mon, Jun 5, 2017 at 11:48 AM, Gabriel Charette <g...@chromium.org> wrote:
My main issue with Mojo is that it effectively defines a new language... I keep running into code where I'm like "oh, what's this type?", then I dig in the codebase and can't find it only to eventually figure out it's a type implicitly defined by Mojo at compile-time [1]... IDEs and code search do not understand these links and nor do I... I'm sure it's all pretty and nice once you've grokked the whole language but I have not.

Also, every time I review a CL that touches Mojo code I'm like "how was one supposed to figure out that this was required?" and then "how am I supposed to know as a reviewer that it is correct?", e.g. https://chromium-review.googlesource.com/c/517988/9/services/preferences/public/interfaces/preferences.mojom

It's normal to be exposed to new APIs as a reviewer (same happens with //base all the time), what's annoying is not being able to code search for said unknown things and find an API with documentation. I did find the Mojo documentation online but perhaps it's missing a section about pure syntax/tokens (e.g. what does "=>" mean in the above change, I couldn't find that in the documentation...)


(Looking at the CL you listed - woah, "CommitPendingWrite() => ()" seems weird at first. Without checking, I think the difference is that "CommitPendingWrite()" would pass an asynchronous message, and the caller would never know when the message handler is finished. While "CommitPendingWrite() => ()" gets a callback called with the "return value" of the message handler. In this case the return value has arguments so the callback has no parameters, so the only use is that the caller knows when CommitPendingWrite is finished.)

Oh wow, yeah, that was totally non-obvious to me... also IIUC this awaited response isn't synchronous, so how does it know which method to callback? Another magical binding?


I agree that Mojo uses a LOT of typedefs which I find hurt discoverability more than they help readability. Are PersistentPrefStorePtr, PersistentPrefStoreRequest and PersistentPrefStorePtrInfo really clearer than mojo::InterfacePtr<PersistentPrefStore>, mojo::InterfaceRequest<PersistentPrefStore>, and mojo::InterfacePtrInfo<PersistentPrefStore>? It's easy to find interface_ptr_info.h and all its documentation in code search by looking for InterfacePtrInfo, which is the natural thing to do if you're not already familiar with Mojo. To know what PersistentPrefStorePtrInfo means you need to already know about that type mapping.

Ah interesting, and indeed I didn't know about that mapping (would have had a much easier time figuring it out had it used the explicit type rather than an implicit typedef).

Joe Mason

unread,
Jun 5, 2017, 4:10:29 PM6/5/17
to Yuzhu Shen, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 3:19 PM, Yuzhu Shen <yzs...@chromium.org> wrote:

That is because usually you don't pass mojo::Binding<> around like the other types, and therefore type it less often than the other types.
Some people do like shorter names. (Imagine you need to type mojo::AssociatedInterfacePtrInfo<PersistentPrefStore> over and over. :))
But I agree that it may have some extra learning cost.

From the point of view of an occasional Mojo user (which is the state many reviewers would be in) it would make sense for individual components to add their own typedefs if a Mojo type is too long to be usable. That way the typedef shows up near the code that uses it (and in the CL that introduces it!) so it can be found through code search. Right now, the best way that I've found to remind myself what the generated types expand into is to look at the generated header (in this case out/Debug/gen/services/preferences/public/interfaces/preferences.mojom.h). Which isn't easily accessibly to reviewers, I don't think.

I can see the benefit to non-casual Mojo users of having standardized typedefs across all of Chrome, though, so that you don't have different components using different names for identical concepts. Maybe rather than auto-generated typedefs, we could just have a styleguide rule saying IF you choose to use shorter names for Mojo types, they MUST follow the naming scheme used by the existing generated files. (Except with a different namespace so as not to conflict with the generated names, assuming this is only enforced as a style rule and not with changes to the code generator.)

I do think it's important to have typedefs that are used in Chromium code actually checked in to the Chromium tree so that they're discoverable through code search.

Alternately maybe some tooling so that mojom-generated types are annotated in both code search and Gerrit diffs would help.

 Joe

Joe Mason

unread,
Jun 5, 2017, 4:20:35 PM6/5/17
to Gabriel Charette, Yuzhu Shen, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 3:26 PM, Gabriel Charette <g...@chromium.org> wrote:
Oh wow, yeah, that was totally non-obvious to me... also IIUC this awaited response isn't synchronous, so how does it know which method to callback? Another magical binding?

That is pretty well explained in the page I linked to, I think: the next section below the .idl definition is "Generated Code For Target Languages", and the C++ page (https://chromium.googlesource.com/chromium/src/+/master/mojo/public/cpp/bindings/README.md) explains how the return values and callbacks work.

Actually it explains the typedef mappings, too, I just noticed. But I think it's normal when a reviewer sees an unfamiliar C++ type to search for it through code search, and if I'm wondering what "PersistentPrefStorePtrInfo" means I'm not sure how I'd stumble on a README in the bindings directly. While on finding an entire unfamiliar file type, .mojom, which starts with "interface Foo" my first instinct is to google "mojom interface" which brings me... hmm, to https://www.chromium.org/developers/design-documents/mojo/associated-interfaces which is not very helpful (it's design docs for an advanced feature of interfaces). The useful doc is currently the 4th result down. Not ideal.


Joe

Yuzhu Shen

unread,
Jun 5, 2017, 4:31:53 PM6/5/17
to Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
I don't think discoverability is a huge issue at the moment:
cs.chromium.org has those generated files and can show you the typedef/using line when you click on Foo{PtrInfo, Ptr, Request}.
Locally, IDEs could easily do that too. (I am using Eclipse.)

Marijn Kruisselbrink

unread,
Jun 5, 2017, 4:35:52 PM6/5/17
to Yuzhu Shen, Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
code search only has those generated files for code that is already checked in. And as such is useless for people reviewing new code (the scenario being described here). For what it's worth I also don't understand why we even have those typedefs in the first place. They might make code slightly easier to write in some cases (although even then I keep having to look at the generated file to remind myself of what typedefs actually exist), but they definitely hurt in readability. And one of the prime goals of the google c++ style guide is to optimize for the reader, not the writer. So any argument favors the writer over the reader doesn't seem particularly relevant as reason to keep something.

Michael Giuffrida

unread,
Jun 5, 2017, 4:40:29 PM6/5/17
to blun...@chromium.org, chromium-dev, blink-dev
I don't see the word "executable" anywhere, and see very little "library" words. Maybe it's just not obvious to me, but what does a Mojo-ified product look like in terms of:
  • executables - is there still just one (chrome), or one per service?
  • libraries - is there one per service? do we link everything dynamically, or are we duplicating a lot of object code?
Or from another perspective, if I want to run services which don't require Chrome, what executable do I run, and what shared libraries does that pull in?

On Mon, May 22, 2017 at 4:42 AM Colin Blundell <blun...@chromium.org> wrote:
TL;DR Ask any questions you have about Mojo/services on this thread and help us determine how to improve the docs, benefiting everyone!

Hi all,

As the long-term projects of Mojo-ification and servicification ripple out through the codebase and community, the team has heard feedback that documentation is not always as complete as it could be. We've tried hard to fill in gaps (e.g., Mojo, services, servicification strategies), but of course from our perspective it's not always apparent just where the gaps are :). This is your chance to help open our eyes! Ask any questions that you have about either of these, and we'll both make sure that the questions get answered and that the answers make their way into the documentation!

Thanks,

Colin (on behalf of the services team)

PS If you don't have any questions now but think of some later, services-dev@ is always open!

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

Joe Mason

unread,
Jun 5, 2017, 5:12:12 PM6/5/17
to Yuzhu Shen, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
They first result I found for "PtrInfo" in code search is https://cs.chromium.org/chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h?l=66&ct=xref_jump_to_def&gsn=MediaClientAssociatedPtrInfo which gives an error:

Can't load 'chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h': Could not resolve path src/out/Debug/gen/ash/public/interfaces/media.mojom.h.

Yuzhu Shen

unread,
Jun 5, 2017, 6:06:06 PM6/5/17
to Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 2:12 PM, Joe Mason <joenot...@chromium.org> wrote:
They first result I found for "PtrInfo" in code search is https://cs.chromium.org/chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h?l=66&ct=xref_jump_to_def&gsn=MediaClientAssociatedPtrInfo which gives an error:

Can't load 'chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h': Could not resolve path src/out/Debug/gen/ash/public/interfaces/media.mojom.h.

This mojom interface is built only for chrome os. Maybe that is why it is not included in the code search?
It sounds like a bug/feature request that we should file for code search.

Yuzhu Shen

unread,
Jun 5, 2017, 6:17:27 PM6/5/17
to Marijn Kruisselbrink, Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
First of all, I agree that it makes sense to optimize for readers.
What I have a different opinion is that this is not necessarily bad for readability. Having to scan through lengthy names is also an eyesore for readers:
mojo::InterfacePtrInfo<mojom::PersistentPrefStore> v.s. mojom::PresistentPrefStorePtrInfo.

Learning that "for each Foo interface we have Foo{Ptr, PtrInfo, Request} typedefs" is an one-time cost. As a comparison, having to write/read lengthy names is a recurring cost.

I would like to point out that adding those typedefs were feature requests from Mojo users in the past. So (at least some) people do feel that they are useful.

Joe Mason

unread,
Jun 5, 2017, 6:30:52 PM6/5/17
to Yuzhu Shen, Marijn Kruisselbrink, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
My personal preference as far as naming goes:

InterfacePtrInfo<Foo> and InterfaceRequest<Foo> don't implement Foo, so I would use them directly. The most important methods of those classes are found in InterfacePtrInfo and InterfaceRequest, and I'm well used to the usage where SomeClass<Foo> is a SomeClass specialized to work with Foo. My eyes don't see that as an eyesore; I'm easily able to pick out the two important pieces of information in that concrete type name.

InterfacePtr<Foo> is actually an implementation of the Foo interface, so I WOULD use the typedef FooPtr for it, because it's more important to remember that it is-a Foo. So here the typedef reduces confusion because it stops me from thinking that InterfacePtr is a pointer-to-Foo or something like that.

(I also would have named it something like InterfaceProxy instead of InterfacePtr, since "pointer" is already overloaded in C++, but I'm pretty sure that ship has sailed.)

Ken Rockot

unread,
Jun 5, 2017, 6:41:24 PM6/5/17
to Joe Mason, Yuzhu Shen, Marijn Kruisselbrink, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 3:30 PM, Joe Mason <joenot...@chromium.org> wrote:
My personal preference as far as naming goes:

InterfacePtrInfo<Foo> and InterfaceRequest<Foo> don't implement Foo, so I would use them directly. The most important methods of those classes are found in InterfacePtrInfo and InterfaceRequest, and I'm well used to the usage where SomeClass<Foo> is a SomeClass specialized to work with Foo. My eyes don't see that as an eyesore; I'm easily able to pick out the two important pieces of information in that concrete type name.

InterfacePtr<Foo> is actually an implementation of the Foo interface, so I WOULD use the typedef FooPtr for it, because it's more important to remember that it is-a Foo. So here the typedef reduces confusion because it stops me from thinking that InterfacePtr is a pointer-to-Foo or something like that.

None of these things are inheriting from Foo. InterfacePtr<Foo>::get and ::operator-> return a Foo*, but it does not implement Foo directly.

I think we'd need to hear some stronger arguments than personal preference to change the convention here, as I don't find the claims about readability to be very convincing. Once the convention is well-established (which admittedly takes some time), the meaning of the mojom::FooPtr is pretty obvious and clearly less verbose than mojo::InterfacePtr<mojom::Foo>.


(I also would have named it something like InterfaceProxy instead of InterfacePtr, since "pointer" is already overloaded in C++, but I'm pretty sure that ship has sailed.)

I would agree on both counts, except "Proxy" has its own semantic shortcomings and as you say the ship has sailed. :)

Ken Rockot

unread,
Jun 5, 2017, 6:55:45 PM6/5/17
to Michael Giuffrida, Colin Blundell, chromium-dev, blink-dev
On Mon, Jun 5, 2017 at 1:40 PM, Michael Giuffrida <mich...@chromium.org> wrote:
I don't see the word "executable" anywhere, and see very little "library" words. Maybe it's just not obvious to me, but what does a Mojo-ified product look like in terms of:
  • executables - is there still just one (chrome), or one per service?
Either one. This decision is a decision for the embedding application / ecosystem to make, not one which is prescribed by Mojo or the service model.

In theory we support both, in practice there are complications (being ironed out) to interconnecting separately shipped binaries, and today we only ship a single binary (chrome) which embeds the all services' code.

  • libraries - is there one per service? do we link everything dynamically, or are we duplicating a lot of object code?
Or from another perspective, if I want to run services which don't require Chrome, what executable do I run, and what shared libraries does that pull in?

Again, entirely your decision as someone who wants to run services which don't require chrome. Think of Mojo as library code you can link against which helps you get yourself connected to other applications which also link against Mojo. You could use this capability without the service manager or anything resembling "services" - in fact the ARC++ bridge does this.

The service manager is another layer on top of that, which coordinates application interconnection and lifecycle management using (relatively) stable mojom interfaces that every application can understand, as that's all part of the service manager's public client library.

The main hangups with using external service binaries/repos here are that:
  • the chrome browser process is the only thing shipping in production which runs the service manager today. In theory we can ship a standalone service manager process (in fact you can already run chrome --type=service-manager to get one, though it's not configurable in any useful capacity) and teach it about other binaries on the system that it can launch as service instances.
  • we aren't ready to freeze a stable set of public service manager client APIs, which means external dependencies would get out of sync. we don't want SM API changes in the chromium repo to require updating external dependents.
 

On Mon, May 22, 2017 at 4:42 AM Colin Blundell <blun...@chromium.org> wrote:
TL;DR Ask any questions you have about Mojo/services on this thread and help us determine how to improve the docs, benefiting everyone!

Hi all,

As the long-term projects of Mojo-ification and servicification ripple out through the codebase and community, the team has heard feedback that documentation is not always as complete as it could be. We've tried hard to fill in gaps (e.g., Mojo, services, servicification strategies), but of course from our perspective it's not always apparent just where the gaps are :). This is your chance to help open our eyes! Ask any questions that you have about either of these, and we'll both make sure that the questions get answered and that the answers make their way into the documentation!

Thanks,

Colin (on behalf of the services team)

PS If you don't have any questions now but think of some later, services-dev@ is always open!

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGE5NFGKp5f_RDXH8Atz0WXHLUQUTJmd4yJF%3Dota%2B2wVQ4o-g%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACi5S_1dr4-G-r%2Ba08WRO1zY_g9j-KvYEAiDnTHqeD6O4Ac5EQ%40mail.gmail.com.

Colin Blundell

unread,
Jun 6, 2017, 12:07:08 PM6/6/17
to Yuzhu Shen, Joe Mason, Gabriel Charette, Colin Blundell, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Tue, Jun 6, 2017 at 12:06 AM Yuzhu Shen <yzs...@chromium.org> wrote:
On Mon, Jun 5, 2017 at 2:12 PM, Joe Mason <joenot...@chromium.org> wrote:
They first result I found for "PtrInfo" in code search is https://cs.chromium.org/chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h?l=66&ct=xref_jump_to_def&gsn=MediaClientAssociatedPtrInfo which gives an error:

Can't load 'chromium/src/out/Debug/gen/ash/public/interfaces/media.mojom.h': Could not resolve path src/out/Debug/gen/ash/public/interfaces/media.mojom.h.

This mojom interface is built only for chrome os. Maybe that is why it is not included in the code search?

Indeed, from offline discussion with Ken, my understanding is that it's a known limitation of codesearch that its indexing is restricted to a limited set of platforms / build parameters.

James Cook

unread,
Jun 7, 2017, 1:05:09 PM6/7/17
to Colin Blundell, Yuzhu Shen, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
Most Chrome OS code is indexed. Try clicking on methods here:

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/upgrade_detector_chromeos.h?l=25

I suspect this is a problem specific to mojo, or generated code in general.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

Colin Blundell

unread,
Jun 8, 2017, 11:11:51 AM6/8/17
to James Cook, Colin Blundell, Yuzhu Shen, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Wed, Jun 7, 2017 at 7:04 PM James Cook <jame...@chromium.org> wrote:
Most Chrome OS code is indexed. Try clicking on methods here:

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/upgrade_detector_chromeos.h?l=25

I suspect this is a problem specific to mojo, or generated code in general.

It looks like it's generated code, for example the same problem shows up with ash_strings.h.

Daniel Murphy

unread,
Jun 20, 2017, 5:56:48 PM6/20/17
to Colin Blundell, James Cook, m...@google.com, Yuzhu Shen, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Ken Rockot, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
Regarding same-process mojo services:

How do we prevent data copies? +Marijn Kruisselbrink is creating the blob mojo service, and ideally when we have data from JS, we can std::move that straight into the blob storage service if it is in the same process.

However, the interface generated for methods like this:

where we pass a byte array accepts it as a const ref, copying the data. How do we provide our own buffer here to prevent copying when we're in the same process?

Daniel

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMGE5NEsammjYYaXvwKDJ2DToQZZ3ytcyGTRU1U5BsuqzXi16g%40mail.gmail.com.

Ken Rockot

unread,
Jun 20, 2017, 6:03:00 PM6/20/17
to Daniel Murphy, Colin Blundell, James Cook, m...@google.com, Yuzhu Shen, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Tue, Jun 20, 2017 at 2:56 PM, Daniel Murphy <dmu...@chromium.org> wrote:
Regarding same-process mojo services:

How do we prevent data copies? +Marijn Kruisselbrink is creating the blob mojo service, and ideally when we have data from JS, we can std::move that straight into the blob storage service if it is in the same process.

However, the interface generated for methods like this:

where we pass a byte array accepts it as a const ref, copying the data. How do we provide our own buffer here to prevent copying when we're in the same process?

I don't believe there is any practical way to avoid copying in every scenario simultaneously.

We really can't have it both ways here. Considering a vector arg: we could pass by value and move it in the in-process case, but we'd incur an extra copy in the serialized case when the input taken from a const ref; conversely as we pass the arg by const ref today, we must (like base::Bind) incur a copy for the in-process case in order to retain ownership of the data while it's in transit for an arbitrarily long period, potentially across a thread boundary.

Ken Rockot

unread,
Jun 20, 2017, 6:07:29 PM6/20/17
to Daniel Murphy, Colin Blundell, James Cook, m...@google.com, Yuzhu Shen, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
However, I do suspect that most of the time, argument data is generated for the express purpose of sending it over IPC, so it's likely that by-value message arguments would be a more efficient default, and we could live with the extra copy in cases where the caller really does need to copy.

Of course the big trade-off there is that users are probably far more likely to accidentally copy where it's not necessary.

Yuzhu Shen

unread,
Jun 20, 2017, 6:15:37 PM6/20/17
to Ken Rockot, Daniel Murphy, Colin Blundell, James Cook, m...@google.com, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
On Tue, Jun 20, 2017 at 3:07 PM, Ken Rockot <roc...@chromium.org> wrote:
However, I do suspect that most of the time, argument data is generated for the express purpose of sending it over IPC, so it's likely that by-value message arguments would be a more efficient default, and we could live with the extra copy in cases where the caller really does need to copy.

Of course the big trade-off there is that users are probably far more likely to accidentally copy where it's not necessary.

We are already doing pass-by-value for generated structs/unions. And because they are move-only, accidental copy is not an issue.

For arrays/maps, if their elements are not move-only, such as array<uint8>, we pass by const ref. An easy way (I know, not perfect :)) to change the behavior is to use a wrapper struct:

struct SomeData {
  array<uint8> data;
};

Daniel Murphy

unread,
Jun 21, 2017, 3:48:12 PM6/21/17
to Yuzhu Shen, Ken Rockot, Colin Blundell, James Cook, m...@google.com, Joe Mason, Gabriel Charette, Primiano Tucci, Reilly Grant, Scott Graham, Fady Samuel, Eric Seckler, Chromium-dev, blink-dev
That's not a bad workaround.
Reply all
Reply to author
Forward
0 new messages