Implement serialization/deserialization of geometry interfaces

third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp

Index: third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp b/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
index 4792bf41027b54fac07061241ce2125ea2324e29..607d595caa33e0b1ccba99ee4512f4cb37b1c5d8 100644
--- a/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
@@ -14,6 +14,14 @@
 #include "core/fileapi/File.h"
 #include "core/fileapi/FileList.h"
 #include "core/frame/ImageBitmap.h"
+#include "core/geometry/DOMMatrix.h"
+#include "core/geometry/DOMMatrixReadOnly.h"
+#include "core/geometry/DOMPoint.h"
+#include "core/geometry/DOMPointInit.h"
+#include "core/geometry/DOMPointReadOnly.h"
+#include "core/geometry/DOMQuad.h"
+#include "core/geometry/DOMRect.h"
+#include "core/geometry/DOMRectReadOnly.h"
 #include "core/html/ImageData.h"
 #include "core/offscreencanvas/OffscreenCanvas.h"
 #include "platform/RuntimeEnabledFeatures.h"
@@ -297,6 +305,193 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
       memcpy(pixel_array->Data(), pixels, pixel_length);
       return image_data;
     }
+    case kDOMPointTag: {
+      double x = 0, y = 0, z = 0, w = 1;
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPoint* point = DOMPoint::Create(x, y, z, w);
+      if (!point)
+        return nullptr;
+      return point;
+    }
+    case kDOMPointReadOnlyTag: {
+      double x = 0, y = 0, z = 0, w = 1;
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPointReadOnly* point = DOMPointReadOnly::Create(x, y, z, w);
+      if (!point)
+        return nullptr;
+      return point;
+    }
+    case kDOMRectTag: {
+      double x = 0, y = 0, width = 0, height = 0;
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&width) ||
+          !ReadDouble(&height))
+        return nullptr;
+      DOMRect* rect = DOMRect::Create(x, y, width, height);
+      if (!rect)
+        return nullptr;
+      return rect;
+    }
+    case kDOMRectReadOnlyTag: {
+      double x = 0, y = 0, width = 0, height = 0;
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&width) ||
+          !ReadDouble(&height))
+        return nullptr;
+      DOMRectReadOnly* rect = DOMRect::Create(x, y, width, height);
+      if (!rect)
+        return nullptr;
+      return rect;
+    }
+    case kDOMQuadTag: {
+      double x = 0, y = 0, z = 0, w = 1;

[jbroman, 2017/05/15 17:02:55]

nit: this is a little repetitive to confirm that it's correct (though I believe
it is). Could also do something like:

DOMPointInit pointInits[4];
for (DOMPointInit& init : pointInits) {
  double x = 0, y = 0, z = 0, w = 0;
  if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
      !ReadDouble(&w))
    return nullptr;
  init.setX(x);
  init.setY(y);
  init.setZ(z);
  init.setW(w);
}
DOMQuad* quad = DOMQuad::Create(pointInits[0], pointInits[1], pointInits[2],
pointInits[3]);

[fserb, 2017/05/15 17:52:17]

much better. I don't know why I just repeteated it. It was late, I was tired. :)
thanks.

+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPointInit pi1;
+      pi1.setX(x);
+      pi1.setY(y);
+      pi1.setZ(z);
+      pi1.setW(w);
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPointInit pi2;
+      pi2.setX(x);
+      pi2.setY(y);
+      pi2.setZ(z);
+      pi2.setW(w);
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPointInit pi3;
+      pi3.setX(x);
+      pi3.setY(y);
+      pi3.setZ(z);
+      pi3.setW(w);
+      if (!ReadDouble(&x) || !ReadDouble(&y) || !ReadDouble(&z) ||
+          !ReadDouble(&w))
+        return nullptr;
+      DOMPointInit pi4;
+      pi4.setX(x);
+      pi4.setY(y);
+      pi4.setZ(z);
+      pi4.setW(w);
+      DOMQuad* quad = DOMQuad::Create(pi1, pi2, pi3, pi4);
+      if (!quad)
+        return nullptr;
+      return quad;
+    }
+    case kDOMMatrix2DTag: {

[simonp, 2017/05/15 13:34:18]

This is not supported by the spec; per spec all elements will be copied, which
could preserve -0 for m13 with is2D being true.

However, that could well be argued to be a spec bug. I'd be happy to fix the
spec to match this, and test this case in WPT.

[fserb, 2017/05/15 17:52:17]

It was partially unintended. I did it like this because there's no
readBool/writeBool, so I just decided to use a different tag, etc. But yeah, I
think it makes sense. Thanks for pushing this to the spec.

[jbroman, 2017/05/15 18:00:30]

FWIW, you could just write 0 or 1 as a 32-bit integer. It'll only take one byte
anyhow (due to variable-length encoding). If that makes your life easier and
makes things more clearly spec-aligned, feel free to do that.

+      double a = 0, b = 0, c = 0, d = 0, e = 0, f = 0;
+      if (!ReadDouble(&a) || !ReadDouble(&b) || !ReadDouble(&c) ||
+          !ReadDouble(&d) || !ReadDouble(&e) || !ReadDouble(&f))
+        return nullptr;
+      DOMMatrixInit init;
+      init.setIs2D(true);
+      init.setA(a);
+      init.setB(b);
+      init.setC(c);
+      init.setD(d);
+      init.setE(e);
+      init.setF(f);
+      DOMMatrix* matrix = DOMMatrix::fromMatrixForSerialization(init);
+      if (!matrix)
+        return nullptr;
+      return matrix;
+    }
+    case kDOMMatrix2DReadOnlyTag: {
+      double a = 0, b = 0, c = 0, d = 0, e = 0, f = 0;
+      if (!ReadDouble(&a) || !ReadDouble(&b) || !ReadDouble(&c) ||
+          !ReadDouble(&d) || !ReadDouble(&e) || !ReadDouble(&f))
+        return nullptr;
+      DOMMatrixInit init;
+      init.setIs2D(true);
+      init.setA(a);
+      init.setB(b);
+      init.setC(c);
+      init.setD(d);
+      init.setE(e);
+      init.setF(f);
+      DOMMatrixReadOnly* matrix =
+          DOMMatrixReadOnly::fromMatrixForSerialization(init);
+      if (!matrix)
+        return nullptr;
+      return matrix;
+    }
+    case kDOMMatrixTag: {
+      double m11 = 0, m12 = 0, m13 = 0, m14 = 0;
+      double m21 = 0, m22 = 0, m23 = 0, m24 = 0;
+      double m31 = 0, m32 = 0, m33 = 0, m34 = 0;
+      double m41 = 0, m42 = 0, m43 = 0, m44 = 0;
+      if (!ReadDouble(&m11) || !ReadDouble(&m12) || !ReadDouble(&m13) ||
+          !ReadDouble(&m14) || !ReadDouble(&m21) || !ReadDouble(&m22) ||
+          !ReadDouble(&m23) || !ReadDouble(&m24) || !ReadDouble(&m31) ||
+          !ReadDouble(&m32) || !ReadDouble(&m33) || !ReadDouble(&m34) ||
+          !ReadDouble(&m41) || !ReadDouble(&m42) || !ReadDouble(&m43) ||
+          !ReadDouble(&m44))
+        return nullptr;
+      DOMMatrixInit init;
+      init.setIs2D(false);
+      init.setM11(m11);
+      init.setM12(m12);
+      init.setM13(m13);
+      init.setM14(m14);
+      init.setM21(m21);
+      init.setM22(m22);
+      init.setM23(m23);
+      init.setM24(m24);
+      init.setM31(m31);
+      init.setM32(m32);
+      init.setM33(m33);
+      init.setM34(m34);
+      init.setM41(m41);
+      init.setM42(m42);
+      init.setM43(m43);
+      init.setM44(m44);
+      DOMMatrix* matrix = DOMMatrix::fromMatrixForSerialization(init);

[jbroman, 2017/05/15 17:02:55]

Easy to change later, but do you really want to make a dictionary for this, or
would it be more efficient (and less repetitive) to do something like this:

// M11, M12, ..., M44
double matrix[16];
for (double& d : matrix) {
  if (!ReadDouble(&d))
    return nullptr;
}
DOMMatrix* matrix = new DOMMatrix(matrix, WTF_ARRAY_LENGTH(matrix));

[fserb, 2017/05/15 17:52:17]

done. Also updated the interfaces.

+      if (!matrix)
+        return nullptr;
+      return matrix;
+    }
+    case kDOMMatrixReadOnlyTag: {
+      double m11 = 0, m12 = 0, m13 = 0, m14 = 0;
+      double m21 = 0, m22 = 0, m23 = 0, m24 = 0;
+      double m31 = 0, m32 = 0, m33 = 0, m34 = 0;
+      double m41 = 0, m42 = 0, m43 = 0, m44 = 0;
+      if (!ReadDouble(&m11) || !ReadDouble(&m12) || !ReadDouble(&m13) ||
+          !ReadDouble(&m14) || !ReadDouble(&m21) || !ReadDouble(&m22) ||
+          !ReadDouble(&m23) || !ReadDouble(&m24) || !ReadDouble(&m31) ||
+          !ReadDouble(&m32) || !ReadDouble(&m33) || !ReadDouble(&m34) ||
+          !ReadDouble(&m41) || !ReadDouble(&m42) || !ReadDouble(&m43) ||
+          !ReadDouble(&m44))
+        return nullptr;
+      DOMMatrixInit init;
+      init.setIs2D(false);
+      init.setM11(m11);
+      init.setM12(m12);
+      init.setM13(m13);
+      init.setM14(m14);
+      init.setM21(m21);
+      init.setM22(m22);
+      init.setM23(m23);
+      init.setM24(m24);
+      init.setM31(m31);
+      init.setM32(m32);
+      init.setM33(m33);
+      init.setM34(m34);
+      init.setM41(m41);
+      init.setM42(m42);
+      init.setM43(m43);
+      init.setM44(m44);
+      DOMMatrixReadOnly* matrix =
+          DOMMatrixReadOnly::fromMatrixForSerialization(init);
+      if (!matrix)
+        return nullptr;
+      return matrix;
+    }
     case kMessagePortTag: {
       uint32_t index = 0;
       if (!ReadUint32(&index) || !transferred_message_ports_ ||