Readability of returning unions from Mojo RPCs

[Jeffrey Yasskin]

In bluetooth_messages.h, we have several triplets of IPCs of the form:

IPC_MESSAGE_CONTROL3(BluetoothMsg_RequestDeviceSuccess,

                     int /* thread_id */,

                     int /* request_id */,

                     content::BluetoothDevice /* device */)

IPC_MESSAGE_CONTROL4(BluetoothMsg_RequestDeviceError,

                     int /* thread_id */,

                     int /* request_id */,

                     content::BluetoothError /* result */,

                     std::string /* error_message */)

...

IPC_MESSAGE_CONTROL4(BluetoothHostMsg_RequestDevice,

                     int /* thread_id */,

                     int /* request_id */,

                     std::vector<content::BluetoothScanFilter>,

                     std::vector<device::BluetoothUUID> /* optional_services */)

These implement Javascript functions that return Promises. In Mojo, it seems like the replies should be expressed as => responses, and we'd use unions to say that the response is either a success or failure. So it comes out looking like:

union BluetoothRequestDeviceResponse {

  BluetoothRequestDeviceSuccess success;

  BluetoothRequestDeviceError error;

};

struct BluetoothRequestDeviceSuccess {

  BluetoothDevice device;

};

struct BluetoothRequestDeviceError {

  BluetoothError result;

  string error_message;

}; 

...

interface BluetoothService {

  RequestDevice(BluetoothScanFilter[] filters,

                device.BluetoothUUID[] optional_services)

      => (BluetoothRequestDeviceResponse response);

  ...

};

This is not as much better as I'd hope. The response stops being able to accept multiple arguments, and we can't define the alternatives anywhere close to the method declaration. 

Allowing the definition of unions and structs inside an interface definition could help, but we're still going to wind up with extra-verbose calls to callback.Run with this formulation.

Instead, you could allow syntax like:

  RequestDevice(BluetoothScanFilter[] filters,

                device.BluetoothUUID[] optional_services)

      => Success(BluetoothDevice device)

      or Failure(BluetoothError result, string error_message);

and then generate a callback class like:

class RequestDeviceCallback {

  Success(BluetoothDevice device);

  Failure(BluetoothError result, string error_message);

};

that would handle wrapping things up into the union.

This isn't as important as my previous two questions for letting Web Bluetooth switch to Mojo.

Jeffrey

[Darin Fisher]

I would probably just list all three parameters as arguments to the callback. The contract would be: if device is null, then result and error_message tells you why.

It is an interesting idea to expand mojom to express application-level exception callbacks. That has been a topic of discussion from the beginning. As you can tell, we haven't gone there .... maybe we should.

(By the way, can error_message be derived from result, or does it contain more information?)

-Darin

[Xiaohan Wang (王消寒)]

We have the same problem and we did what pretty much what Darin suggested: passing all success and error arguments to the callback (in a struct).

We also have Callback<->Promise conversion immediately before / after the mojo layer to limit the this struct only as a transport layer detail.

[Marijn Kruisselbrink]

One alternative that I was considering 

You can of course already do something like that yourself by defining your own RequestDeviceCallback interface in your mojom file and passing that to the RequestDevice call. I was considering to do something like that for some API calls where a single callback wasn't enough. It would of course change ordering/queuing guarantees and such, although I'm not sure how much, since I'm not entirely sure what behavior mojo/mojom actually guarantees (it's surprisingly hard to find any non-outdated documentation, or I'm just not looking in the right place). One thing I've heard somebody say is that when a method call has a callback, new calls to that method are queued until the callback has been called. I'm not sure why that behavior would be desirable, but that would of course be lost by using a separate channel for the callbacks for each individual method call.

[Yuzhu Shen]

This is not true. Mojo doesn't enforce callback ordering. It does require the callback to be run by the receiver once. If it is dropped silently, Mojo will trigger a DCHECK.

[Darin Fisher]

However, if you are careful to run callbacks prior to returning control to mojo, then you can make some ordering assumptions. Point being that there is one FIFO per interface.

We have a long standing idea to invent a way to share a FIFO across multiple interfaces. This would allow you to have a user defined "callback" interface with multiple methods, and again if you are careful, you can make some ordering assumptions.

-Darin

[Yuzhu Shen]

On Wed, Jul 15, 2015 at 9:10 PM, Darin Fisher <da...@google.com> wrote:

However, if you are careful to run callbacks prior to returning control to mojo, then you can make some ordering assumptions. Point being that there is one FIFO per interface.

We have a long standing idea to invent a way to share a FIFO across multiple interfaces. This would allow you to have a user defined "callback" interface with multiple methods, and again if you are careful, you can make some ordering assumptions.

I just started experimenting the idea that we have discussed, and put it down in a design doc. Hopefully I can send the doc out for review soon.

[Darin Fisher]

Cool, looking forward to it!