Use union instead of overload fo DOMMatrix/DOMMatrixReadOnly

third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html

Index: third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html b/third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html
index c054e895eaafcbd57b9277b5bd14a854746cd8a0..47090aba009909527e9de38a1191603641b0d43e 100644
--- a/third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/DOMMatrix-001.html
@@ -27,7 +27,7 @@
             m12: 0, m22: 2, m32: 0, m42: 10,
             m13: 0, m23: 0, m33: 1, m43: 0,
             m14: 0, m24: 0, m34: 0, m44: 1,
-            is2D: false,
+            is2D: true,
             isIdentity: false
         };
 
@@ -38,15 +38,15 @@
             checkDOMMatrix(new DOMMatrix(new DOMMatrix()), initial);
         },'testConstructor1');
         test(function() {
-            var float32Array = new Float32Array(
+            var float32Array = new Float32Array([
                 2.0, 0.0, 0.0, 0.0,
                 0.0, 2.0, 0.0, 0.0,
                 0.0, 0.0, 1.0, 0.0,
-                10.0, 10.0, 0.0, 1.0);
+                10.0, 10.0, 0.0, 1.0]);
             checkDOMMatrix(new DOMMatrix(float32Array), scaleTranslate2D, false);
         },'testConstructor2');
         test(function() {
-            var float32Array = new Float32Array(2.0, 0.0, 0.0, 2.0, 10.0, 10.0);
+            var float32Array = new Float32Array([2.0, 0.0, 0.0, 2.0, 10.0, 10.0]);
             checkDOMMatrix(new DOMMatrix(float32Array), scaleTranslate2D);
         },'testConstructor3');
         test(function() {
@@ -58,7 +58,7 @@
             checkDOMMatrix(new DOMMatrix(float64Array), scaleTranslate2D, false);
         },'testConstructor4');
         test(function() {
-            var float64Array = new Float64Array(2.0, 0.0, 0.0, 2.0, 10.0, 10.0);
+            var float64Array = new Float64Array([2.0, 0.0, 0.0, 2.0, 10.0, 10.0]);
             checkDOMMatrix(new DOMMatrix(float64Array), scaleTranslate2D);
         },'testConstructor5');
         test(function() {
@@ -71,23 +71,18 @@
         },'testConstructor6');
         test(function() {
             var sequence = [ 2.0, 0.0, 0.0, 2.0, 10.0, 10.0];
-            checkDOMMatrix(new DOMMatrix(float64Array), scaleTranslate2D);
+            checkDOMMatrix(new DOMMatrix(sequence), scaleTranslate2D);
         },'testConstructor7');
         test(function() {
             var string = 'scale(2) translateX(5px) translateY(5px)';
             checkDOMMatrix(new DOMMatrix(string), scaleTranslate2D);
         },'testConstructor8');
         test(function() {
-            var string = 'scale(2 2) translateX(5) translateY(5)';

[Justin Novosad, 2017/05/08 14:50:39]

It seems the old version of the test was quite deliberate about testing scale()
with and without the comma.  I think it is possible that the comma-less notation
is correct (though not supported by blink).  Members of the CSS WG have been
working on updating the notation, but I am not sure what is the current status
of that work.  See: https://wiki.csswg.org/ideas/functional-notation#transforms

[fserb, 2017/05/08 15:31:19]

It seems even the draft CSS transform doesn't accept the comma-less style. Also,
the unitless "5" is clearly not ok.

+            var string = 'scale(2, 2) translateX(5px) translateY(5px)';
             checkDOMMatrix(new DOMMatrix(string), scaleTranslate2D);
         },'testConstructor9');
         test(function() {
-            var string = 'scale(2, 2), translateX(5)  ,translateY(5)';
-            checkDOMMatrix(new DOMMatrix(string), scaleTranslate2D);
-        },'testConstructor10');
-        test(function() {
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX    (5px)'); });
-            assert_throws('SyntaxError', function() { new DOMMatrix('scale(2)translateX(5px)'); });

[Justin Novosad, 2017/05/08 14:50:39]

Why is this removed? Are you sure this case was not revealing a bug in Chrome?

[fserb, 2017/05/08 15:31:18]

No spaces between function and (.
But the space between ) and the next function is optional.

             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5em)'); });
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5ex)'); });
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5ch)'); });
@@ -97,7 +92,7 @@
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5vmin)'); });
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5vmax)'); });
             assert_throws('SyntaxError', function() { new DOMMatrix('translateX(5%)'); });
-        },'testConstructor11');
+        },'testConstructor10');
         test(function() {
             var sequence = [
                 2.0, 1.0, 0.0, 0.0,
@@ -112,7 +107,7 @@
                 is2D: false,
                 isIdentity: false
             });
-        },'testConstructor12');
+        },'testConstructor11');
         test(function() {
             var matrix = new DOMMatrix([
                 2.0, 1.0, 0.0, 0.0,
@@ -127,19 +122,16 @@
                 is2D: false,
                 isIdentity: false
             });
-        },'testConstructor13');
-        test(function() {
-            assert_throws(new TypeError(), function() { new DOMMatrixReadOnly(); });

[Justin Novosad, 2017/05/08 14:50:39]

Based on my reading of the spec, this call to the constructor should just make
an identity matrix.  I am guessing that is youre interpretation too.  The
original author of this test must have had a reason though.  I think you should
reach out to them.

[fserb, 2017/05/08 15:31:19]

This is clearly off. The previous implementation had a empty constructor, now it
has a full optional one.

-        },'testConstructorIllegal0');
+        },'testConstructor12');
         test(function() {
             var string = 'scale(2, 2), translateX(5px) translateY(5px)';
-            assert_throws(new TypeError(), function() { new DOMMatrixReadOnly(string); });
-        },'testConstructorIllegal1');
+            assert_throws(new SyntaxError(), function() { new DOMMatrixReadOnly(string); });

[Justin Novosad, 2017/05/08 14:50:39]

I agree with this correction under the assumption that this test was meant to
test an error in parsing the transform string. Unfortunately this test is not
well documented (no comments, no use of the description argument of
assert_throwa), so it is hard to guess what the intent it.  In the spec, there
are conditions under which the constructor throws a TypeError().  The spec says
a type error should be thrown if the current global object is not Window.  And I
think we might be lacking test coverage for that case. Basically: passing a
string to the DOMMatrix constructor on a worker should throw.  

This aspect of the spec is a bit weak IMHO, I bet it has to do with avoiding to
call the CSS parser off the main thread of the browser.  Though style cannot be
safely computed off the main thread, CSS parsing can be safe, and that is all we
need here.  It is basically like the work we did for canvas filters and color
values for OffscreenCanvas. Anyways, this getting off topic, but I think it
deserves a follow-up. Please create an issue either against the spec or in crbug
to get to the bottom of this.

+        },'testConstructorIllegal0');
         test(function() {
             var sequence = [ 2.0, 0.0, 0.0, 2.0, 10.0];
             assert_throws(new TypeError(), function() { new DOMMatrixReadOnly(sequence); });
-        },'testConstructorIllegal2');
-        
+        },'testConstructorIllegal1');
+
     </script>
 </body>
 </html>