Testing typemaps

[Ken Rockot]

A common pattern for unit-testing StructTraits implementations seems to be to introduce some new testing mojom interface with a sync call like

interface FooTraitsTest {

  [Sync] Echo(Foo) => (Foo);

};

As Scott pointed out to me, this is unfortunate. The good news is that it is also unnecessary!

Supposing you have a mojom::Foo type typemapped to a RadFoo C++ type, you can much more simply do:

  RadFoo foo;

  /* ... insert exciting |foo| initialization ... */

  RadFoo out_foo;

  bool success =

      mojom::Foo::Deserialize(mojom::Foo::Serialize(&foo), &out_foo);

  /* ... insert exciting validation logic ... */

No need to introduce new mojom just for these kinds of tests. Please try to use this pattern when unit-testing typemaps!

[Yuzhu Shen]

+1 that there is no need to involve a test interface in this case, not to mention a sync method.

Currently only the generated types of structs have Deserialize()/Serialize() defined. I could trivially add those methods to unions, too. However, the same way won't apply when it comes to enums. Because we generate C++ enums which don't have methods.

So I am thinking to also expose general-purpose serialization functions.

namespace mojo {

// |VectorType| could be either std::vector<> or WTF::Vector<>.

template <typename MojomDataViewType,

          typename UserType,

          template<typename...> class VectorType = std::vector>

VectorType<uint8_t> Serialize(const UserType& input,

                              VectorType<ScopedMojoHandle>* handles = nullptr);

template <typename MojomDataViewType,

          typename UserType,

          template<typename...> class VectorType = std::vector>

bool Deserialize(const VectorType<uint8_t>& input, UserType* output, VectorType<ScopedMojoHandle>* handles = nullptr);

}

// For structs and unions. (They are a little more verbose. But we could also keep the Foo::Serialize()/Deserialize() convenient helpers.)

auto data = mojo::Serialize<mojom::FooDataView>(foo);

bool result = mojo::Deserialize<mojom::FooDataView>(data, &out_foo);

The corresponding MojomDataViewType types for other types:

  - Enums: mojom::Foo

  - Strings: mojo::StringDataView

  - Arrays: mojo::ArrayDataView<ElementDataViewType>

  - Maps: mojo::MapDataView<KeyDataViewType, MapDataViewType>

Why do we want to use this weird type, say, mojo::StringDataView, instead of std::string or WTF::String?

That is because *DataView is the unambiguous type for the wire format of a mojom type; while things like std::string / WTF::String are not. For example, both built-in mojom string and mojo.common.mojom.String16 are mapped to WTF::String.

WDYT? Thanks!

--
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.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CA%2BapAgH75BXy%3DVECQ%3Dju34OO6Vkbq29JbO0BS%3D6AnJtitdch%3Dg%40mail.gmail.com.

[Yuzhu Shen]

FYI, this approach currently won't work if the mojom target is linked into a component, and your serialization/deserialization code depends on that component.

https://bugs.chromium.org/p/chromium/issues/detail?id=702397

I am working on it.

[Marijn Kruisselbrink]

I wonder if this approach will also make it possible to test round-tripping through blink and non-blink struct-traits (maybe something like mojom::Foo::Deserialize(mojom::blink::Foo::Serialize(mojom::blink::Foo::Deserialize(mojom::Foo::Serialize(&foo), &out_foo);?

I'm aware of at least one case where round-tripping through either just the blink struct traits, or just the non-blink struct-traits worked fine, but serializing with one and deserializing with the other would fail, so being able to test that would be very valuable.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CABxY52B9tZqPnryWh2MnvSZiPK26aZc-RHqOFfRxgukTzxH3WQ%40mail.gmail.com.

[Yuzhu Shen]

On Thu, Mar 16, 2017 at 3:42 PM, Marijn Kruisselbrink <m...@chromium.org> wrote:

I wonder if this approach will also make it possible to test round-tripping through blink and non-blink struct-traits (maybe something like mojom::Foo::Deserialize(mojom::blink::Foo::Serialize(mojom::blink::Foo::Deserialize(mojom::Foo::Serialize(&foo), &out_foo);?

Yes. This works.

[John Abd-El-Malek]

Is this in the wiki docs?

On Thu, Mar 16, 2017 at 9:45 AM, Ken Rockot <roc...@chromium.org> wrote:

[Yuzhu Shen]

On Thu, Mar 16, 2017 at 3:37 PM, Yuzhu Shen <yzs...@chromium.org> wrote:

FYI, this approach currently won't work if the mojom target is linked into a component, and your serialization/deserialization code depends on that component.

https://bugs.chromium.org/p/chromium/issues/detail?id=702397

I am working on it.

 

FYI, this issue has been fixed.

[Xiyuan Xia]

Would this Deserialize(...Serialize()) pattern work when the (de)serialized data could not contain handles?

I am trying to use this for my CL(https://codereview.chromium.org/2770693002/) that sends over ImageSkia via shared buffer. After changing to use this pattern, the tests fail with VALIDATION_ERROR_ILLEGAL_HANDLE errors. My guess is that the handles are processed separately from other serialized data and the pattern probably does not exercise that part of code.

Is this supposed to work?

[Ken Rockot]

It won't work with handles, because Serialize has nowhere to store the handles -- they can't exactly be serialized as bytes...

I suppose we should consider making this work if we're going to recommend using the API for testing traits.