Conditionally-defined methods in Mojo interfaces

169 views
Skip to first unread message

Johan Tibell

unread,
Dec 14, 2016, 10:57:58 PM12/14/16
to chromium-mojo
Hi,

I'm converting some priting legacy IPCs, that use conditionally defined messages, to Mojo (CL). Some messages are conditional on the OS (e.g. IS_WIN) and some on a feature (e.g. ENABLE_PRINT_PREVIEW). The question of how to handle conditional messages in Mojo came up. In particular the reviewer asked:

We used to have a fairly strict stance from security folk that if a message
doesn't do anything for a given platform, it shouldn't have a handler in the
first place. (Hence the #ifdef OS_WIN)
 
Have we changed that stance with mojo?

I would like to discuss how we handle such messages, both in the shorter term (e.g. in the above CL) and in the medium/long term.

Mojo doesn't support conditionally defined methods and we don't pre-process mojom files so I see two options in the short term:
  1. Split off the conditionally-defined messages in into a separate interface(s).
  2. Keep them in the same interface but have their implementation call mojo::ReportBadMessage on platforms that don't support them.
The CL above currently does (2).

Below I've tried to list the pros/cons as I see them (I hope I didn't miss any).

Option 1: Split interfaces
We'd have several mojom targets, the OS/feature-specific ones are conditionally compiled by the build system.

Pros:
  • Calling a method that don't exist on the platform is a compile-time error.
Cons:
  • The sender needs to declare, initialize, and use several InterfacePtrs and similarly for the receiver, which needs multiple Bindings.
  • Additional #ifdefs around the declaration, initialization, and use of the additional InterfacePtrs and Bindings.
  • The receiver needs to #ifdef the inheritance of some interfaces.
  • There are several more (error) states to care about e.g. if one connection succeeds and another fails. This shouldn't happen but robust code should probably at least check for it.
  • The split interfaces most likely will have to be associated interfaces, to ensure message ordering.
  • If the interfaces are used in factories (which is the case in my CL above) the factories need to be duplicated in several factories, one per combination of conditions.
  • The interfaces make less conceptual/semantic sense e.g. the PrintingOnWindows interface doesn't really correspond to a service in the new service archicteture. It's just an implementation detail.
Option 2: Runtime rejection of messages
The receiver would implement all methods, but some implementation would look like e.g.

void Impl::Foo() {
#ifdef IS_WIN
  // Windows code
#else
  mojo::ReportBadMessage("Foo only supported on Windows");
#endif
}

Pros:
  • Both sender and receiver code is simpler, with fewer InterfacePtrs, Bindings, and #ifdefs.
  • More code (e.g. all mojom methods) are type checked on all platforms. Less likely to break platform-specific code because there is less of it.
Cons:
  • The receiver can forget to reject messages by calling e.g. mojo::ReportBadMessage, which would lead to confusing errors during development.
  • The receiver could run the OS-specific code on OSes where it shouldn't. (In practice I don't think this is a big risk, as the OS-specific code usually doesn't compile on other OSes.)
  • Calling the wrong method is only caught during testing.

My suggestions

Methods conditional on a feature
For messages that are conditional on a feature (e.g. ENABLE_PRINT_PREVIEW) and would perhaps make sense to split out in any case a variation of option (1) makes sense. Most of the cons don't apply here as the interfaces aren't really dependent, which means no need for associated interfaces, errors on each interface are unrelated and can be treated as such, etc.

We would split the interfaces but always compile both of them. If we don't want to support print preview this would all be handled on the sender side, which will never attempt a connection to the receiver. Since we don't instantiate the receiver side until we get an incomming connection (which we won't if the feature is turned off) there shouldn't be any performance cost (except perhaps some compile time cost).

Methods conditional on an OS
Here I would go with option (2), at least until we have some support in Mojo to do (1) better (see below). The cons in (1) in terms of code complexity isn't worth it.

Making Mojo support conditional messages better
This is just one idea.

We could support per-method attributes in the mojom that would allow us to specify which OSes they should be supported on (default being all OSes). On non-supported OSes we could generate a message receiver in the bindings that automatically rejects messages and closes the connection. This is basically (2) from above but the the dynamic enforcement automatically generated.

Thoughts?

-- Johan

Colin Blundell

unread,
Dec 15, 2016, 5:13:45 AM12/15/16
to Johan Tibell, chromium-mojo
I agree with all of the reasoning that you lay out here, as well as your conclusions. I think that investing in specifying this via mojom would be worth it only if we see evidence that having people do it manually actually leads to problems.

Thanks,

Colin

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAKZ7acO1PyQ0c1jB4Oe6WOxcMPSnbvFGRDFFXw8QxZL%2BWbYDKQ%40mail.gmail.com.

Daniel Cheng

unread,
Dec 15, 2016, 5:44:42 AM12/15/16
to Colin Blundell, Johan Tibell, chromium-mojo
There is option 3, which is to just run the mojom files through the C preprocessor before invoking the IDL compiler.

One disadvantage is that things compiled with different preprocessor defines will not work together, but that's already true today. Another is I'm not aware of a cross-platform way of doing this built into GN.

However, it solves some of the current problems with conditionally-defined fields in mojo structs. For some example issues, see GpuInfo's struct traits for DxDiagNode:
- Types become imprecise: in the IDL file, it's defined as an optional param: but it's actually never optional on Windows (which is where it's used).
- Writing an efficient StructTrait requires style guide violations or being inefficient: https://cs.chromium.org/chromium/src/gpu/ipc/common/gpu_info_struct_traits.h?rcl=0&l=264 has a non-POD variable of static storage duration. The alternative is to construct a temporary for each call, which is a bit wasteful, which leads into
- (Minor) runtime cost: we're serializing and deserializing data that's simply never used.

Daniel

Colin Blundell

unread,
Dec 15, 2016, 7:18:19 AM12/15/16
to Daniel Cheng, Colin Blundell, Johan Tibell, chromium-mojo
How would this impact the generation of mojom files for e.g. Java and JS? Personally I think that the coupling of mojom to C++ that we've had to do is somewhat unfortunate (although clearly necessary given the various use cases), and I'd be sad to see us introduce more such coupling if there's not a large win from doing so. In this case it doesn't seem to me like the win is large enough.

Best,

Colin

Ken Rockot

unread,
Dec 15, 2016, 11:58:36 AM12/15/16
to Colin Blundell, Daniel Cheng, chromium-mojo, Johan Tibell


On Dec 15, 2016 8:57 AM, "Ken Rockot" <roc...@google.com> wrote:


On Dec 15, 2016 4:18 AM, "Colin Blundell" <blun...@chromium.org> wrote:
How would this impact the generation of mojom files for e.g. Java and JS? Personally

This has been and remains my main concern with preprocessing mojom. Running them through the preprocessor is trivial to do, but this can only limit the code we generate, i.e. It can only cull the generated symbol definitions.

We have no ability to output corresponding conditionals in Java or JS bindings. We do care about JS bindings being redistributable, and there doesn't seem to be a great way to inform clients about their host platform. We'd have to introduce a native binding API that provided platform information at runtime.


Best,

Colin


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

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

Ken Rockot

unread,
Dec 15, 2016, 1:54:01 PM12/15/16
to Colin Blundell, Daniel Cheng, chromium-mojo, Johan Tibell
Alternatively we could say that we'll preprocess mojom but only if your mojom target specifies cpp_only. So if you want platform-conditional definitions, you can only generate C++ bindings. Might be a reasonable compromise that doesn't paint us into a corner.

Colin Blundell

unread,
Dec 16, 2016, 3:37:02 AM12/16/16
to Ken Rockot, Colin Blundell, Daniel Cheng, chromium-mojo, Johan Tibell
It's not too hard to imagine a mojom for which there are initially only C++ implementations but later someone wants to implement in Java on Android. It seems unfortunate that that person would then have to deal with this issue, which is completely unrelated to what they're trying to do.


Best,

Colin


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

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

Daniel Cheng

unread,
Dec 16, 2016, 4:15:20 AM12/16/16
to Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
In practice, doesn't Java imply Android anyway? While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

Makes a much stronger statement than:
  // Required on Android; not used by any other platform.
  gfx.mojom.Point? point;

Since it allows the compiler to enforce things, rather than having it fail at run time.

​
​
We have no ability to output corresponding conditionals in Java or JS bindings. We do care about JS bindings being redistributable, and there doesn't seem to be a great way to inform clients about their host platform. We'd have to introduce a native binding API that provided platform information at runtime.
​
​I'm curious how this is expected to interact with .mojom files that are simply never built on a particular platform. It seems like that would trivially break redistribution of JS bindings.​​

Daniel


Best,

Colin


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

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

Colin Blundell

unread,
Dec 16, 2016, 4:20:42 AM12/16/16
to Daniel Cheng, Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

Personally I think that we should try to hold the line as much as possible against the mojom language being tightly coupled to C++.
 

Best,

Colin


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

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

Colin Blundell

unread,
Dec 16, 2016, 4:22:07 AM12/16/16
to Colin Blundell, Daniel Cheng, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

Daniel Cheng

unread,
Dec 16, 2016, 4:27:21 AM12/16/16
to Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 1:22 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

My point is that this doesn't require conditional compilation of Java. Java implementations would be able to use any defined(OS_ANDROID) things just by virtue of being Java, since Java implies OS_ANDROID.

Daniel
 

Colin Blundell

unread,
Dec 16, 2016, 4:29:35 AM12/16/16
to Daniel Cheng, Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 10:27 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:22 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

My point is that this doesn't require conditional compilation of Java. Java implementations would be able to use any defined(OS_ANDROID) things just by virtue of being Java, since Java implies OS_ANDROID.

I'm confused. What is your proposed implementation of proprocessing the mojoms? I had thought we were talking about Ken's proposal, wherein we would use the C preprocessor but only allow it if the mojom target was specified to generate only cpp bindings.

Daniel Cheng

unread,
Dec 16, 2016, 4:34:23 AM12/16/16
to Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 1:29 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:27 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:22 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

My point is that this doesn't require conditional compilation of Java. Java implementations would be able to use any defined(OS_ANDROID) things just by virtue of being Java, since Java implies OS_ANDROID.

I'm confused. What is your proposed implementation of proprocessing the mojoms? I had thought we were talking about Ken's proposal, wherein we would use the C preprocessor but only allow it if the mojom target was specified to generate only cpp bindings.

I think we should just unconditionally preprocess them all.
Even if Mojo doesn't expose a hook for this, an alternative is to just define a mojom per-platform and only add it to the build on that platform. It's brittle, it's fragile, and it involves a bunch of copy and paste, but it works. It also affect Java/JS bindings in exactly the same way that conditional compilation would.

Daniel

Colin Blundell

unread,
Dec 16, 2016, 4:49:20 AM12/16/16
to Daniel Cheng, Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 10:34 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:29 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:27 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:22 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

My point is that this doesn't require conditional compilation of Java. Java implementations would be able to use any defined(OS_ANDROID) things just by virtue of being Java, since Java implies OS_ANDROID.

I'm confused. What is your proposed implementation of proprocessing the mojoms? I had thought we were talking about Ken's proposal, wherein we would use the C preprocessor but only allow it if the mojom target was specified to generate only cpp bindings.

I think we should just unconditionally preprocess them all.

But what would we do in generating Java/JS bindings? Just ignore the markup? Sorry, there's something I'm missing here.
 
Even if Mojo doesn't expose a hook for this, an alternative is to just define a mojom per-platform and only add it to the build on that platform. It's brittle, it's fragile, and it involves a bunch of copy and paste, but it works. It also affect Java/JS bindings in exactly the same way that conditional compilation would.

That's independent of this proposal, right? i.e., on a case-by-case basis we could decide to do that in today's world.

Daniel Cheng

unread,
Dec 16, 2016, 4:55:22 AM12/16/16
to Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 1:49 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:34 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:29 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:27 AM Daniel Cheng <dch...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 1:22 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:20 AM Colin Blundell <blun...@chromium.org> wrote:
On Fri, Dec 16, 2016 at 10:15 AM Daniel Cheng <dch...@chromium.org> wrote:
In practice, doesn't Java imply Android anyway?

There are cross-platform interfaces whose most natural implementation is in Java on Android (e.g., battery status). So if what you're implying is that an interface that would be implemented in Java on Android necessarily is tied to Android *as an interface*, I don't think that's true.
 
While Mojo tries to be language-agnostic in many respects, at the end of the day, C++ is by far the most common language in the Chromium code base. A struct with Android-specific fields requires non-Android platforms to populate the fields with dummy values, adding noise, or for the fields to be marked as optional, complicating Android validation (if the parameters are required). The individual costs are small, but it does add up. 

Having
#if defined(OS_ANDROID)
  gfx.mojom.Point point;
#endif

One further thought: If we did something like this, it would mean that the Android impl of this interface would have to be in C++ and (presumably) go across to Java via JNI. This definitely undercuts part of the vision of Mojo.

My point is that this doesn't require conditional compilation of Java. Java implementations would be able to use any defined(OS_ANDROID) things just by virtue of being Java, since Java implies OS_ANDROID.

I'm confused. What is your proposed implementation of proprocessing the mojoms? I had thought we were talking about Ken's proposal, wherein we would use the C preprocessor but only allow it if the mojom target was specified to generate only cpp bindings.

I think we should just unconditionally preprocess them all.

But what would we do in generating Java/JS bindings? Just ignore the markup? Sorry, there's something I'm missing here.

For example:

struct MyStruct {
#if defined(OS_WIN)
  int32 field1;
#endif
  bool field2;
#if defined(OS_ANDROID)
  string field3;
#endif
};

After preprocessing on OS_ANDROID, this would look like:

struct MyStruct {
  bool field2;
  string field3;
};

And then the Java/JS bindings would be generated based on that, since there's no preprocessor markup left.

Daniel

Colin Blundell

unread,
Dec 16, 2016, 4:59:19 AM12/16/16
to Daniel Cheng, Colin Blundell, Ken Rockot, chromium-mojo, Johan Tibell
Oh, I see. Sorry for missing this, it makes sense.

Hmm. What do others think? I'm still mildly opposed to tying the mojom language to the C preprocessor, but my opposition is definitely more mild after fully understanding Daniel's proposal.

Ken Rockot

unread,
Dec 16, 2016, 10:05:15 AM12/16/16
to Daniel Cheng, Colin Blundell, chromium-mojo, Johan Tibell
Right. Sorry, I thought we had already established this.

The C++-only suggestion was proposed as a compromise, because the above proposal is untenable if we want portable JS bindings. We do want portable JS bindings. It's a good point (and fortunate for us) that Java will effectively always mean Android.

An alternative compromise is that we make preprocessing opt-in via an explicit GN option in the mojom template, and that this option also disables JS bindings generation.

What I am not willing to allow is unconditional preprocessing of all mojom files regardless of target language, because that puts is in an undesirable position. I want the choice to use preprocessing to be explicit, and I want to make sure it's clear that choosing to do preprocessing is also choosing to be non-portable.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

Colin Blundell

unread,
Dec 16, 2016, 10:22:23 AM12/16/16
to Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo, Johan Tibell
I was behind in fully understanding the discussion here, which caused it to go in a bit of a circle :).
 

The C++-only suggestion was proposed as a compromise, because the above proposal is untenable if we want portable JS bindings. We do want portable JS bindings.

Agreed.
 
It's a good point (and fortunate for us) that Java will effectively always mean Android.

An alternative compromise is that we make preprocessing opt-in via an explicit GN option in the mojom template, and that this option also disables JS bindings generation.

What I am not willing to allow is unconditional preprocessing of all mojom files regardless of target language, because that puts is in an undesirable position. I want the choice to use preprocessing to be explicit, and I want to make sure it's clear that choosing to do preprocessing is also choosing to be non-portable.

Thanks for spelling these thoughts out again, Ken. Now that I fully understand the implications here, I think that we should hold the line on portability for all bindings in Chromium as a form of future-proofing rather than leaving it up to individual developers to choose (if the only thing they're sacrificing at the time they make the choice is "won't get JS bindings", I think that usually won't be a meaningful thing to give up from their POV).

Ken Rockot

unread,
Dec 16, 2016, 10:22:31 AM12/16/16
to Daniel Cheng, Colin Blundell, chromium-mojo, Johan Tibell
Though I suppose it's even possible to want non-portable JS in some cases. e.g. WebUI certainly doesn't need to be portable, and I could imagine wanting some platform-specific IPC for WebUI. e.g.

interface FrobinatorSettings {
#if defined(OS_CHROMEOS)
  SetSystemFrobinatorSpeed();
#endif
};

and the JS could do something like

if (settings.setSystemFrobinatorSpeed) {
  // Chrome OS-only stuff.
}

but I would still like to make preprocessing opt-in rather than default behavior.

Daniel Cheng

unread,
Dec 16, 2016, 2:08:44 PM12/16/16
to Ken Rockot, Colin Blundell, chromium-mojo, Johan Tibell
I would be OK with an opt-in, though I don't agree that it should disable JS binding generation. As I mentioned elsewhere, it's trivial to subvert the goal of universal JS bindings by using GN itself to only conditionally include mojoms on certain platforms.

Daniel

Ken Rockot

unread,
Dec 16, 2016, 3:23:12 PM12/16/16
to Daniel Cheng, Colin Blundell, chromium-mojo, Johan Tibell
On Fri, Dec 16, 2016 at 11:08 AM, Daniel Cheng <dch...@chromium.org> wrote:
I would be OK with an opt-in, though I don't agree that it should disable JS binding generation. As I mentioned elsewhere, it's trivial to subvert the goal of universal JS bindings by using GN itself to only conditionally include mojoms on certain platforms.

OK. After some more discussion offline, Ben has also convinced me that even making this opt-in is probably not worthwhile, so I think we can just try turning it on unconditionally.

To summarize the rationale: it's already possible to emit non-portable bindings if you try not-that-hard; it's also possible to avoid non-portable bindings if you know you need to; pre-processing is cheap to implement and maintain since we can just apply cpp to mojom; and finally, this solution results in less code and less error potential than its alternatives.

Johan Tibell

unread,
Dec 20, 2016, 10:44:04 PM12/20/16
to Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
(I was out sick two days so I missed much of the discussion.)

It sounds like agree on preprocessing the mojoms (at least for now).

Do we have a way to do this across platforms? For C++ code clang/gcc/VS does this for us I presume. Can all these (in particular VS) be used in preprocess-only mode on the command line (so we can call the from GN for mojoms)? Any idea where in the build system the preprocessing logic should go? Shall we do this in the generated build rules for mojoms or should the mojom generator (i.e. the Python script) do it itself?

...

[Message clipped]  

Johan Tibell

unread,
Dec 20, 2016, 10:52:04 PM12/20/16
to Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
I filed https://bugs.chromium.org/p/chromium/issues/detail?id=676224 to track implementing preprocessing (and to refer to from my CL that should be eventually changed to use it).

Sam McNally

unread,
Dec 21, 2016, 2:28:26 AM12/21/16
to Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
I'd prefer this as something the bindings generator understands. That would allow the wire format to be consistent across all platforms while the higher-level APIs could omit fields and methods not supported on particular platforms.

Yuzhu Shen

unread,
Dec 21, 2016, 12:01:44 PM12/21/16
to Sam McNally, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
On Tue, Dec 20, 2016 at 11:28 PM, Sam McNally <sa...@chromium.org> wrote:
I'd prefer this as something the bindings generator understands. That would allow the wire format to be consistent across all platforms while the higher-level APIs could omit fields and methods not supported on particular platforms.

IMO, if the user wants platform-specific behavior, we probably don't need to care about cross-platform wire format. For example, when there are required fields that are of different types on different platforms, a message generated on one platform doesn't make sense / useful on another platform anyway.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.

Colin Blundell

unread,
Dec 21, 2016, 1:19:26 PM12/21/16
to Yuzhu Shen, Sam McNally, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Personally I still think that this is pretty unfortunate. Part of the reason we're going to all this effort is to develop a cleaner architecture, and while ifdefs are definitely the right way to go in some cases, I feel that they also provide an easy path to hackiness. Mojo already has a natural separation between interface and implementation and natural ways to signal that a particular functionality isn't supported from implementor to client. I hope that the ability to use ifdefs in mojom doesn't receive significant abuse and lead to engineers taking shortcuts in interface design (I'm of course not worried about anyone in this conversation, but rather down the line as the floodgates get opened).

The consensus on this thread is clearly in favor of making this change, so I just wanted to put my non-blocking dissent out there to satisfy my own conscience ;).

Best,

Colin

Yuzhu Shen

unread,
Dec 21, 2016, 1:56:02 PM12/21/16
to Colin Blundell, Sam McNally, Johan Tibell, Ken Rockot, Daniel Cheng, chromium-mojo
On Wed, Dec 21, 2016 at 10:19 AM, Colin Blundell <blun...@chromium.org> wrote:
Personally I still think that this is pretty unfortunate. Part of the reason we're going to all this effort is to develop a cleaner architecture, and while ifdefs are definitely the right way to go in some cases, I feel that they also provide an easy path to hackiness. Mojo already has a natural separation between interface and implementation and natural ways to signal that a particular functionality isn't supported from implementor to client. I hope that the ability to use ifdefs in mojom doesn't receive significant abuse and lead to engineers taking shortcuts in interface design (I'm of course not worried about anyone in this conversation, but rather down the line as the floodgates get opened).

The consensus on this thread is clearly in favor of making this change, so I just wanted to put my non-blocking dissent out there to satisfy my own conscience ;).

Agreed with everything you said.
For the record, I personally don't like this as well and have pushed back a couple of times in the past. But I can see that there are cases where such feature is useful.

I think we all agree that the usage of this feature should be limited to a minimum. Maybe we could setup a whitelist to register those mojom targets that require pre-processing, and the OWNER of that whitelist should watch out for misuses. We have done similar things for, e.g., sync calls from the browser process.

Johan Tibell

unread,
Dec 21, 2016, 7:32:45 PM12/21/16
to Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Here are the four variations on preprocessing I can see:

1. GN all the way - passing defines on the preprocessor command line

Here we need to figure out the defines needed from the build system configs (i.e. gni files) and pass them as command line arguments to the C preprocessor. Steps:
  1. Create a temp mojom in gen/ by invoking the C preprocessor, passing the needed defines on the command line. Compute these defines by
    • looking in BUILDCONFIG.gn to create OS defines and
    • looking at a new preprocessor_flags argument to the mojom build rule to get feature defines (see below for example).
  2. Pass the temp file to the bindings generator.
The mojom build target will now look like:

mojom("foo") {
  // ...
  preprocessor_flags = [
    "ENABLE_PRINTING=$enable_printing"
  ]
}

2. GN all the way - using build config header files

Similar to (1), but we get OS and features defines from build_config.h and other build config header files (e.g. printing/features/features.h). Steps:
  1. Create a temp mojom in gen/ that #includes build_config.h, and a list of other build config headers listed in the mojom build target, at the top of the file.
  2. Pass the temp file to C preprocessor and output another temp mojom in gen/.
  3. Pass the second temp mojom to the bindings generator.
Example mojom build target:

mojom("foo") {
  // ...
  config_headers = ["//printing/features/features.h"]
  config_deps = ["//printing/features"]
}

We have to be careful to not mess up line numbers, as seen by the bindings generator, as we're passing a modified file to it. A quick test suggests that this might work:

    clang -E gen/my.mojom.pp | grep -v "#"

(Assuming my.mojom.pp is the generated temp file from step (1).)

3. Bindings generator invokes C preprocessor

Steps: similar to (1) or (2), but we pass the defines/build config header file paths to the bindings generator and have it call the preprocessor before processing the mojom.

Differences from above include:
  • The bindings generator might be able to give better error messages as it knows of the original file (and location).
  • Less logic in GN (maybe).
  • We'll have to tell the bindings generator how to invoke the C preprocessor (e.g. by passing the needed path(s), command line arguments).
4. Support OS/feature tags in mojom annotations and "preprocess" using the bindings generator

This doesn't use the C preprocessor but instead teaches the bindings generator about certain OS/feature flags, which it uses to control output. Example mojom:

interface Foo {
  [enable_if=feature(printing) && (os(win) || os(mac))]
  void Print();
};

Example mojom build target:

mojom("foo") {
  // ...
  if (enable_printing) {
    features += ["printing"]
  }
}

The syntax and expressiveness of the annotations languages is up for debate (e.g. we might not have "and", "or", or, "not" and instead assume we always have a list conditions to "or" together e.g. "[os(win), os(mac)]" we'd assume that means Windows or Mac. Feature flags might be "or"-ed together separately.)

This requires a bigger change to the bindings generator (e.g. we need to support the boolean expression language). The benefits include
  • no C preprocessor and
  • we have more options in the code we generate (e.g. we could keep method IDs stable across platforms).
  • maybe more that I haven't thought of.
Summary: I'm currently leaning towards (1).

Other problems

We need to figure out how to invoke the C preprocessor (only) on each platform.

Thoughts?

-- Johan

On Wed, Dec 21, 2016 at 2:43 PM, Johan Tibell <tib...@chromium.org> wrote:

Yuzhu Shen

unread,
Dec 22, 2016, 2:23:51 AM12/22/16
to Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
I wish we could stay away from (4).
IMO, it is nice to keep preprocessor directives external to the core mojom syntax. In other words, we don't have to consider a mojom file with preprocessor directives a valid mojom definition. Instead, it is just a "template" file which we can feed into preprocessor to generate a valid mojom definition. Mojo itself doesn't need to have any knowledge about preprocessing.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.

Colin Blundell

unread,
Dec 22, 2016, 2:40:54 AM12/22/16
to Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
+1. 

Sam McNally

unread,
Dec 22, 2016, 3:36:12 AM12/22/16
to Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
This feels like an artificial distinction. Whether the mojom generator has to deal with something is orthogonal to whether it's part of the mojom syntax; whatever syntax is used for mojom files is the mojom syntax. The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations.

Colin Blundell

unread,
Dec 22, 2016, 3:42:23 AM12/22/16
to Sam McNally, Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
What I'm really saying is that I wouldn't regard the usage of the processor as part of the mojom syntax, e.g. as part of mojom files that we would export beyond Chromium. I would regard it as merely a convenience within Chromium that is outside of the mojom syntax.

Yuzhu Shen

unread,
Dec 22, 2016, 1:02:41 PM12/22/16
to Sam McNally, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Although what the mojom generator does could be orthogonal, introducing new types mojom annotations certainly have something to do with mojom syntax. And I am concerned about (4) because of those new mojom annotations, not whether the bindings generator does the work.
 
whatever syntax is used for mojom files is the mojom syntax.
(Not that I suggest such an extension change, just a thought experiment.) 
Imagine we change the extension of all the mojom files with preprocessor directives to .tmpl. Those template files needs to go into a predecessor along with some other inputs (platform/feature configuration) to generate real .mojom files.

 
The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations. 
 
Even if they achieve the same purpose at the moment, the level of commitment is different.

Colin Blundell

unread,
Dec 22, 2016, 1:06:57 PM12/22/16
to Yuzhu Shen, Sam McNally, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Yes, I thought of such an approach as well. Even if we don't actually do this because it would be too much of a headache, I think that this is how we should conceptually think about it.

Sam McNally

unread,
Dec 27, 2016, 6:40:22 PM12/27/16
to Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Sounds like someone will have a fun time making preprocessing work with imports.

 
The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations. 
 
Even if they achieve the same purpose at the moment, the level of commitment is different.

How so? Once this feature is used, we're committed to it either way; an arbitrary chrome developer won't see a difference. All this distinction achieves is the illusion of mojoms still being pure and untainted by platform-specific concerns, while at the same time adding platform-specific concerns to mojoms.

Colin Blundell

unread,
Dec 28, 2016, 8:25:34 AM12/28/16
to Sam McNally, Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, Colin Blundell, chromium-mojo
Out of curiosity, could you be more explicit here about the challenges you see here? To me it seems that something like just adding the root build dir to the exported import_dirs for a mojom_template target would do the trick, but perhaps there's complexity that I'm missing.


 
The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations. 
 
Even if they achieve the same purpose at the moment, the level of commitment is different.

How so? Once this feature is used, we're committed to it either way; an arbitrary chrome developer won't see a difference. All this distinction achieves is the illusion of mojoms still being pure and untainted by platform-specific concerns, while at the same time adding platform-specific concerns to mojoms.

The distinction in my mind is whether we regard the ability to use C preprocessor syntax as part of the definition of the mojom language that we would export beyond Chromium. I'm arguing that we shouldn't (and thus, mojom files that use the C preprocessor would be by definition internal to Chromium).

Sam McNally

unread,
Dec 29, 2016, 7:45:27 PM12/29/16
to Colin Blundell, Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, chromium-mojo
I was mostly thinking about additional mojom target complexity; it isn't a huge amount more, but mojoms are complicated enough as it is. With a separate preprocess step, mojom targets would need to support "preprocess_deps" and mojoms would need to depend on all the preprocess_deps for their normal mojom deps. Without preprocessing handled internal to the the mojom target, mojom generate actions would just need to depend on preprocess actions for all their mojom deps.


 
The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations. 
 
Even if they achieve the same purpose at the moment, the level of commitment is different.

How so? Once this feature is used, we're committed to it either way; an arbitrary chrome developer won't see a difference. All this distinction achieves is the illusion of mojoms still being pure and untainted by platform-specific concerns, while at the same time adding platform-specific concerns to mojoms.

The distinction in my mind is whether we regard the ability to use C preprocessor syntax as part of the definition of the mojom language that we would export beyond Chromium. I'm arguing that we shouldn't (and thus, mojom files that use the C preprocessor would be by definition internal to Chromium).

Are we planning to export mojom beyond Chromium? If we do, how do you intend to convince external users to not use the C preprocessor in the same way?

Colin Blundell

unread,
Jan 3, 2017, 11:32:55 AM1/3/17
to Sam McNally, Colin Blundell, Yuzhu Shen, Johan Tibell, Ken Rockot, Daniel Cheng, chromium-mojo
Gotcha.
 


 
The question here is whether to add the C preprocessor to mojom syntax or to add new mojom annotations. 
 
Even if they achieve the same purpose at the moment, the level of commitment is different.

How so? Once this feature is used, we're committed to it either way; an arbitrary chrome developer won't see a difference. All this distinction achieves is the illusion of mojoms still being pure and untainted by platform-specific concerns, while at the same time adding platform-specific concerns to mojoms.

The distinction in my mind is whether we regard the ability to use C preprocessor syntax as part of the definition of the mojom language that we would export beyond Chromium. I'm arguing that we shouldn't (and thus, mojom files that use the C preprocessor would be by definition internal to Chromium).

Are we planning to export mojom beyond Chromium?

It seems possible in the fullness of time.
 
If we do, how do you intend to convince external users to not use the C preprocessor in the same way?

If it's not part of the mojom language, then any such mojom files simply wouldn't be valid mojom. People could do whatever they like in their internal codebases, but those files wouldn't be exportable.
Reply all
Reply to author
Forward
0 new messages