Add serialization/deserialization of structs for mojo java bindings.

mojo/not-to-commit/mojo_public_test_interfaces/src/org/chromium/mojo/bindings/test/mojom/imported/Size.java

Index: mojo/not-to-commit/mojo_public_test_interfaces/src/org/chromium/mojo/bindings/test/mojom/imported/Size.java
diff --git a/mojo/not-to-commit/mojo_public_test_interfaces/src/org/chromium/mojo/bindings/test/mojom/imported/Size.java b/mojo/not-to-commit/mojo_public_test_interfaces/src/org/chromium/mojo/bindings/test/mojom/imported/Size.java
new file mode 100644
index 0000000000000000000000000000000000000000..0f2822dafbbae5541011eabf80523daf4fac6413
--- /dev/null
+++ b/mojo/not-to-commit/mojo_public_test_interfaces/src/org/chromium/mojo/bindings/test/mojom/imported/Size.java
@@ -0,0 +1,48 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file is autogenerated by:
+//     mojo/public/tools/bindings/mojom_bindings_generator.py
+// For:
+//     mojo/public/interfaces/bindings/tests/sample_import2.mojom
+//
+
+package org.chromium.mojo.bindings.test.mojom.imported;
+
+public final class Size extends org.chromium.mojo.bindings.Struct {
+
+    private static final int STRUCT_SIZE = 16;
+    private static final DataHeader DEFAULT_STRUCT_INFO = new DataHeader(STRUCT_SIZE, 2);
+
+    public int width;
+    public int height;
+
+    public Size() {
+        super(STRUCT_SIZE);
+    }
+
+    public static Size deserialize(org.chromium.mojo.bindings.Message message) {
+        return decode(new org.chromium.mojo.bindings.Decoder(message));
+    }
+
+    public static Size decode(org.chromium.mojo.bindings.Decoder decoder0) {
+        Size result = new Size();
+        DataHeader mainDataHeader = decoder0.readDataHeader();
+        if (0 < mainDataHeader.numFields) {

[rmcilroy, 2014/06/25 13:14:39]

nit - "if (mainDataHeader.numFields > 0)"

[qsr, 2014/06/25 14:43:05]

Done.

+            result.width = decoder0.readInt(8);

[rmcilroy, 2014/06/25 13:14:39]

Could you pull the offset's out as private static final constants at the to of
the generated struct, and use them both in decode and encode?

[qsr, 2014/06/25 14:43:05]

 I don't think it is a good idea. This is a generated file, there is no risk of
wrong typing a constant. Moreover, this port of the code is mostly write only,
there is nothing interesting here. And having the constants will clutter this
file for nothing.

 To be frank, I'm not even sure having extracted the size is really useful.

[rmcilroy, 2014/06/26 17:10:25]

Frankly I think the exposed use of offsets at all is not ideal. Given that, as
far as I can tell, you only ever encode/decode objects in-order on a given
encoder/decoder object, do you really need to explicitly pass the offsets? 
Could you not simply keep track of the current offset in the encoder / decoder
object rather than explicitly in the generated code?

[qsr, 2014/06/27 12:41:47]

 We do encode/decode in order, but we have to handle holes.
see:
https://chromiumcodereview.appspot.com/317273006/diff/60001/mojo/not-to-commi...
https://chromiumcodereview.appspot.com/317273006/diff/60001/mojo/not-to-commi...

The first one has a hole, the second one if filling it with an int.

 Keeping track of the offset makes it harder in the encoder/decoder, because
now, I need to either keep track of alignement, or skip part in the generated
code. Skipping makes it also hard on the decoding size, because depending on the
version I might need to either skip, or read a data at a certain time.

 And I'm not even speaking of boolean that must be packed.

 All in hole, I started with the state in the Encoder/Decoder, and changed it
that way because it makes both the generation and the Encoder/Decoder simpler.

 I don't see any issue with offset in the generated code. This part of the code
is of no interest for anymore.

 If it hurts your eyes, I can create an package private internal class and push
this code there. But that means more method and more classes while I don't think
having those 2 methods is such a problem.

 

[rmcilroy, 2014/07/01 15:53:45]

I don't appreciate the sarcasm here - yes this is generated code, but I am
reviewing a CL which generates this code, and other non-generated code will have
to interact both with the code and the Encoder/Decoder classes, so I don't think
it is unreasonable to discuss design alternatives for the code.

In any case, I understand the issue with the holes / alignment and see how that
could make things tricky, however, both the C++ and JS bindings generator's do
manage to do this without raw offsets.  C++ seems to use raw structs AFAICT
(which we obviously can't do in Java), however, the JS bindings generator uses
an encoder which keeps track of the current offset, and provides operations to
skip holes.  Personally I think this would be a lot cleaner, and I really think
it makes a lot of sense to share as similar a design to other language binding
generators as possible.

I also really like the way the JS bindings decode multi-dimensional arrays,
e.g.:
  val.multi_array_of_strings = decoder.decodeArrayPointer(new codec.ArrayOf(new
codec.ArrayOf(codec.String)));
I'm sure with a bit of work we could use the same type of approach for the Java
bindings, would this not be possible in Java?

I don't want to block the work your doing here, but I really think there is an
opportunity to maintain a design which is much more consistent with the other
bindings generators.  If you disagree, and if the other Mojo owners are fine
with the approach you are taking here, then feel free to bypass my review - I'm
not an OWNER here so all my comments are just advice in any case.

+        }
+        if (1 < mainDataHeader.numFields) {
+            result.height = decoder0.readInt(12);
+        }
+        return result;
+    }
+
+    @Override
+    protected final void encode(org.chromium.mojo.bindings.Encoder encoder) {
+        org.chromium.mojo.bindings.Encoder encoder0 = encoder.getEncoderAtDataOffset(DEFAULT_STRUCT_INFO);
+        encoder0.encode(width, 8);
+        encoder0.encode(height, 12);
+    }
+}
+