Changeset 243703 in webkit


Ignore:
Timestamp:
Apr 1, 2019 11:46:44 AM (5 years ago)
Author:
commit-queue@webkit.org
Message:

SVGMatrix.IDL methods do not conform to the specs
​https://bugs.webkit.org/show_bug.cgi?id=196263

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2019-04-01
Reviewed by Simon Fraser.

Source/WebCore:

I think there was a misconception about these functions. The specs link
is: ​https://www.w3.org/TR/SVG11/coords.html#InterfaceSVGMatrix.

Notice that the specs does not state that the SVGMethod methods should
raise the exception NO_MODIFICATION_ALLOWED_ERR if the object is read
only. Notice setting the attribute 'a' for example may raise this
exception. Therefore, I think the specs wanted to make these operations
read-only. None of the methods should raise the exception
NO_MODIFICATION_ALLOWED_ERR.

In fact the SVG code was doing the right thing. For example SVGMatrix::scale()
was calling SVGMatrixValue::scale() which was making a copy of itself,
applying the scale on the copy and then returning the copy. When
SVGMatrix::scale() receives the copy of the SVGMatrixValue it creates and
returns a new SVGMatrix object.

  • WebCore.xcodeproj/project.pbxproj:
  • svg/SVGMatrix.h:

(WebCore::SVGMatrix::create):
(WebCore::SVGMatrix::a const):
(WebCore::SVGMatrix::b const):
(WebCore::SVGMatrix::c const):
(WebCore::SVGMatrix::d const):
(WebCore::SVGMatrix::e const):
(WebCore::SVGMatrix::f const):
(WebCore::SVGMatrix::multiply const):
(WebCore::SVGMatrix::inverse const):
(WebCore::SVGMatrix::translate const):
(WebCore::SVGMatrix::scale const):
(WebCore::SVGMatrix::scaleNonUniform const):
(WebCore::SVGMatrix::rotate const):
(WebCore::SVGMatrix::rotateFromVector const):
(WebCore::SVGMatrix::flipX const):
(WebCore::SVGMatrix::flipY const):
(WebCore::SVGMatrix::skewX const):
(WebCore::SVGMatrix::skewY const):
(WebCore::SVGMatrix::a): Deleted.
(WebCore::SVGMatrix::b): Deleted.
(WebCore::SVGMatrix::c): Deleted.
(WebCore::SVGMatrix::d): Deleted.
(WebCore::SVGMatrix::e): Deleted.
(WebCore::SVGMatrix::f): Deleted.
(WebCore::SVGMatrix::multiply): Deleted.
(WebCore::SVGMatrix::inverse): Deleted.
(WebCore::SVGMatrix::translate): Deleted.
(WebCore::SVGMatrix::scale): Deleted.
(WebCore::SVGMatrix::scaleNonUniform): Deleted.
(WebCore::SVGMatrix::rotate): Deleted.
(WebCore::SVGMatrix::rotateFromVector): Deleted.
(WebCore::SVGMatrix::flipX): Deleted.
(WebCore::SVGMatrix::flipY): Deleted.
(WebCore::SVGMatrix::skewX): Deleted.
(WebCore::SVGMatrix::skewY): Deleted.
(WebCore::SVGMatrix::SVGMatrix): Deleted.

  • svg/SVGMatrix.idl:
  • svg/SVGMatrixValue.h: Removed.
  • svg/SVGTransform.cpp:

(WebCore::SVGTransform::matrix):

  • svg/SVGTransformDistance.cpp:

(WebCore::SVGTransformDistance::addToSVGTransform const):

  • svg/SVGTransformValue.h:

(WebCore::SVGTransformValue::matrix const):
(WebCore::SVGTransformValue::matrix):
(WebCore::SVGTransformValue::svgMatrix): Deleted.
(WebCore::operator==): Deleted.
(WebCore::operator!=): Deleted.

  • svg/properties/SVGMatrixTearOff.h:
  • svg/properties/SVGPropertyTearOff.h:

(WebCore::SVGPropertyTearOff::propertyReference const):

LayoutTests:

  • svg/dom/SVGMatrix-expected.txt:
  • svg/dom/SVGMatrix.html:

Clean this test. Make it test the case when valid arguments are passed to
the methods of the SVGMatrix. Make sure the methods are read-only. All
of them should be making a copy of the matrix, applying the transform and
returning the copy.

Location:
trunk
Files:
1 deleted
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243696 r243703  
     12019-04-01  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        SVGMatrix.IDL methods do not conform to the specs
     4        https://bugs.webkit.org/show_bug.cgi?id=196263
     5
     6        Reviewed by Simon Fraser.
     7
     8        * svg/dom/SVGMatrix-expected.txt:
     9        * svg/dom/SVGMatrix.html:
     10        Clean this test. Make it test the case when valid arguments are passed to
     11        the methods of the SVGMatrix. Make sure the methods are read-only. All
     12        of them should be making a copy of the matrix, applying the transform and
     13        returning the copy.
     14
    1152019-04-01  Shawn Roberts  <sroberts@apple.com>
    216

  • trunk/LayoutTests/svg/dom/SVGMatrix-expected.txt

    r165640 r243703  
    66
    77Check initial matrix values
    8 PASS matrix.a is 1
    9 PASS matrix.b is 0
    10 PASS matrix.c is 0
    11 PASS matrix.d is 1
    12 PASS matrix.e is 0
    13 PASS matrix.f is 0
     8PASS matrixToString(matrix) is "{ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0}"
    149
    1510Check assigning matrices
    … …  
    2621PASS matrix.a = 'aString' is 'aString'
    2722PASS matrix.a is NaN
     23PASS matrix.a = null is null
     24PASS matrix.a is 0
    2825PASS matrix.a = 2 is 2
    2926PASS matrix.b = matrix is matrix
    … …  
    3532PASS matrix.b = 'aString' is 'aString'
    3633PASS matrix.b is NaN
     34PASS matrix.b = null is null
     35PASS matrix.b is 0
    3736PASS matrix.b = 0 is 0
    3837PASS matrix.c = matrix is matrix
    … …  
    4443PASS matrix.c = 'aString' is 'aString'
    4544PASS matrix.c is NaN
     45PASS matrix.c = null is null
     46PASS matrix.c is 0
    4647PASS matrix.c = 0 is 0
    4748PASS matrix.d = matrix is matrix
    … …  
    5354PASS matrix.d = 'aString' is 'aString'
    5455PASS matrix.d is NaN
     56PASS matrix.d = null is null
     57PASS matrix.d is 0
    5558PASS matrix.d = 1 is 1
    5659PASS matrix.e = matrix is matrix
    … …  
    6265PASS matrix.e = 'aString' is 'aString'
    6366PASS matrix.e is NaN
     67PASS matrix.e = null is null
     68PASS matrix.e is 0
    6469PASS matrix.e = 0 is 0
    6570PASS matrix.f = matrix is matrix
    … …  
    7176PASS matrix.f = 'aString' is 'aString'
    7277PASS matrix.f is NaN
     78PASS matrix.f = null is null
     79PASS matrix.f is 0
    7380PASS matrix.f = 200 is 200
    7481
    7582Check that the matrix is still containing the correct values
    76 PASS matrix.a is 2
    77 PASS matrix.b is 0
    78 PASS matrix.c is 0
    79 PASS matrix.d is 1
    80 PASS matrix.e is 0
    81 PASS matrix.f is 200
    82 
    83 Check assigning null works as expected
    84 PASS matrix.f = null is null
    85 PASS matrix.a is 2
    86 PASS matrix.b is 0
    87 PASS matrix.c is 0
    88 PASS matrix.d is 1
    89 PASS matrix.e is 0
    90 PASS matrix.f is 0
     83PASS matrixToString(matrix) is "{ a: 2, b: 0, c: 0, d: 1, e: 0, f: 200}"
    9184
    9285Check calling 'multiply' with invalid arguments
    … …  
    155148PASS matrix.skewY('aString') is non-null.
    156149PASS matrix.skewY(svgElement) is non-null.
     150
     151Check calling SVGMatrix methods with valid arguments
     152PASS matrixToString(matrix.translate(10, 20)) is "{ a: 2, b: 0, c: 0, d: 1, e: 20, f: 220}"
     153PASS matrixToString(matrix.scale(5)) is "{ a: 10, b: 0, c: 0, d: 5, e: 0, f: 200}"
     154PASS matrixToString(matrix.scaleNonUniform(2, 3)) is "{ a: 4, b: 0, c: 0, d: 3, e: 0, f: 200}"
     155PASS matrixToString(matrix.skewX(90)) is "{ a: 2, b: 0, c: 32662478706390740, d: 1, e: 0, f: 200}"
     156PASS matrixToString(matrix.skewY(90)) is "{ a: 2, b: 16331239353195370, c: 0, d: 1, e: 0, f: 200}"
     157
     158Check that the matrix is still containing the correct values
     159PASS matrixToString(matrix) is "{ a: 2, b: 0, c: 0, d: 1, e: 0, f: 200}"
     160
     161Calling methods that throw exceptions
     162PASS matrix.rotateFromVector(0, 4) threw exception TypeError: Type error.
     163PASS matrix.rotateFromVector(4, 0) threw exception TypeError: Type error.
     164PASS matrix.a = 0 is 0
     165PASS matrix.inverse() threw exception InvalidStateError: Matrix is not invertible.
     166
    157167PASS successfullyParsed is true
    158168

  • trunk/LayoutTests/svg/dom/SVGMatrix.html

    r217390 r243703  
    88<div id="console"></div>
    99<script>
    10 description("This test checks the SVGMatrix API");
    1110
    1211var svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg");
    1312var matrix = svgElement.createSVGMatrix();
    1413
     14function matrixToString(matrix)
     15{
     16        return "{ a: " + matrix.a + ", b: " + matrix.b + ", c: " + matrix.c + ", d: " + matrix.d + ", e: " + matrix.e + ", f: " + matrix.f + "}";
     17}
     18
     19function checkAttributeSetting(matrix, name) {
     20        var value = matrix[name];
     21
     22        shouldBe("matrix." + name + " = matrix", "matrix");
     23        shouldBe("matrix." + name, "NaN");
     24        shouldBe("matrix." + name + " = 0", "0");
     25        shouldBe("matrix." + name + " = svgElement", "svgElement");
     26        shouldBe("matrix." + name, "NaN");
     27        shouldBe("matrix." + name + " = 0", "0");
     28        shouldBe("matrix." + name + " = 'aString'", "'aString'");
     29        shouldBe("matrix." + name, "NaN");
     30        //Check assigning null works as expected
     31        shouldBeNull("matrix." + name + " = null");
     32        shouldBe("matrix." + name, "0");
     33        // Reset to previous value.
     34        shouldBe("matrix." + name + " = " + value.toString(), value.toString());
     35}
     36
     37function checkMethodCallingOneMatrixArgument(matrix, name) {
     38        debug("");
     39        debug("Check calling '" + name + "' with invalid arguments");
     40        shouldThrow("matrix." + name + "()");
     41        shouldThrow("matrix." + name + "(true)");
     42        shouldThrow("matrix." + name + "(2)");
     43        shouldThrow("matrix." + name + "('aString')");
     44        shouldThrow("matrix." + name + "(svgElement)");
     45}
     46
     47function checkMethodCallingOneNumericArgument(matrix, name) {
     48        debug("");
     49        debug("Check calling '" + name + "' with invalid arguments");
     50        shouldThrow("matrix." + name + "()");
     51        shouldBeNonNull("matrix." + name + "('aString')");
     52        shouldBeNonNull("matrix." + name + "(svgElement)");
     53}
     54
     55function checkMethodCallingTwoNumericArguments(matrix, name) {
     56        debug("");
     57        debug("Check calling '" + name + "' with invalid arguments");
     58        shouldThrow("matrix." + name + "()");
     59        shouldThrow("matrix." + name + "(true)");
     60        shouldThrow("matrix." + name + "(2)");
     61        shouldThrow("matrix." + name + "('aString')");
     62        shouldThrow("matrix." + name + "(svgElement)");
     63        shouldBeNonNull("matrix." + name + "('aString', 'aString')");
     64        shouldBeNonNull("matrix." + name + "(svgElement, svgElement)");
     65        shouldBeNonNull("matrix." + name + "(2, 'aString')");
     66        shouldBeNonNull("matrix." + name + "(2, svgElement)");
     67        shouldBeNonNull("matrix." + name + "('aString', 2)");
     68        shouldBeNonNull("matrix." + name + "(svgElement, 2)");
     69}
     70
     71description("This test checks the SVGMatrix API");
     72
    1573debug("");
    1674debug("Check initial matrix values");
    17 shouldBe("matrix.a", "1");
    18 shouldBe("matrix.b", "0");
    19 shouldBe("matrix.c", "0");
    20 shouldBe("matrix.d", "1");
    21 shouldBe("matrix.e", "0");
    22 shouldBe("matrix.f", "0");
     75shouldBeEqualToString("matrixToString(matrix)", "{ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0}");
    2376
    2477debug("");
    … …  
    2982debug("");
    3083debug("Check assigning invalid matrices");
    31 shouldBe("matrix.a = matrix", "matrix");
    32 shouldBe("matrix.a", "NaN");
    33 shouldBe("matrix.a = 0", "0");
    34 shouldBe("matrix.a = svgElement", "svgElement");
    35 shouldBe("matrix.a", "NaN");
    36 shouldBe("matrix.a = 0", "0");
    37 shouldBe("matrix.a = 'aString'", "'aString'");
    38 shouldBe("matrix.a", "NaN");
    39 // Reset to previous value.
    40 shouldBe("matrix.a = 2", "2");
    41 
    42 shouldBe("matrix.b = matrix", "matrix");
    43 shouldBe("matrix.b", "NaN");
    44 shouldBe("matrix.b = 0", "0");
    45 shouldBe("matrix.b = svgElement", "svgElement");
    46 shouldBe("matrix.b", "NaN");
    47 shouldBe("matrix.b = 0", "0");
    48 shouldBe("matrix.b = 'aString'", "'aString'");
    49 shouldBe("matrix.b", "NaN");
    50 // Reset to previous value.
    51 shouldBe("matrix.b = 0", "0");
    52 
    53 shouldBe("matrix.c = matrix", "matrix");
    54 shouldBe("matrix.c", "NaN");
    55 shouldBe("matrix.c = 0", "0");
    56 shouldBe("matrix.c = svgElement", "svgElement");
    57 shouldBe("matrix.c", "NaN");
    58 shouldBe("matrix.c = 0", "0");
    59 shouldBe("matrix.c = 'aString'", "'aString'");
    60 shouldBe("matrix.c", "NaN");
    61 // Reset to previous value.
    62 shouldBe("matrix.c = 0", "0");
    63 
    64 shouldBe("matrix.d = matrix", "matrix");
    65 shouldBe("matrix.d", "NaN");
    66 shouldBe("matrix.d = 0", "0");
    67 shouldBe("matrix.d = svgElement", "svgElement");
    68 shouldBe("matrix.d", "NaN");
    69 shouldBe("matrix.d = 0", "0");
    70 shouldBe("matrix.d = 'aString'", "'aString'");
    71 shouldBe("matrix.d", "NaN");
    72 // Reset to previous value.
    73 shouldBe("matrix.d = 1", "1");
    74 
    75 shouldBe("matrix.e = matrix", "matrix");
    76 shouldBe("matrix.e", "NaN");
    77 shouldBe("matrix.e = 0", "0");
    78 shouldBe("matrix.e = svgElement", "svgElement");
    79 shouldBe("matrix.e", "NaN");
    80 shouldBe("matrix.e = 0", "0");
    81 shouldBe("matrix.e = 'aString'", "'aString'");
    82 shouldBe("matrix.e", "NaN");
    83 // Reset to previous value.
    84 shouldBe("matrix.e = 0", "0");
    85 
    86 shouldBe("matrix.f = matrix", "matrix");
    87 shouldBe("matrix.f", "NaN");
    88 shouldBe("matrix.f = 0", "0");
    89 shouldBe("matrix.f = svgElement", "svgElement");
    90 shouldBe("matrix.f", "NaN");
    91 shouldBe("matrix.f = 0", "0");
    92 shouldBe("matrix.f = 'aString'", "'aString'");
    93 shouldBe("matrix.f", "NaN");
    94 // Reset to previous value.
    95 shouldBe("matrix.f = 200", "200");
     84checkAttributeSetting(matrix, "a");
     85checkAttributeSetting(matrix, "b");
     86checkAttributeSetting(matrix, "c");
     87checkAttributeSetting(matrix, "d");
     88checkAttributeSetting(matrix, "e");
     89checkAttributeSetting(matrix, "f");
    9690
    9791debug("");
    9892debug("Check that the matrix is still containing the correct values");
    99 shouldBe("matrix.a", "2");
    100 shouldBe("matrix.b", "0");
    101 shouldBe("matrix.c", "0");
    102 shouldBe("matrix.d", "1");
    103 shouldBe("matrix.e", "0");
    104 shouldBe("matrix.f", "200");
     93shouldBeEqualToString("matrixToString(matrix)", "{ a: 2, b: 0, c: 0, d: 1, e: 0, f: 200}");
     94
     95checkMethodCallingOneMatrixArgument(matrix, "multiply")
     96checkMethodCallingTwoNumericArguments(matrix, "translate");
     97checkMethodCallingOneNumericArgument(matrix, "scale");
     98checkMethodCallingTwoNumericArguments(matrix, "scaleNonUniform");
     99checkMethodCallingOneNumericArgument(matrix, "rotate");
     100checkMethodCallingTwoNumericArguments(matrix, "rotateFromVector");
     101checkMethodCallingOneNumericArgument(matrix, "skewX");
     102checkMethodCallingOneNumericArgument(matrix, "skewY");
    105103
    106104debug("");
    107 debug("Check assigning null works as expected");
    108 shouldBeNull("matrix.f = null");
    109 shouldBe("matrix.a", "2");
    110 shouldBe("matrix.b", "0");
    111 shouldBe("matrix.c", "0");
    112 shouldBe("matrix.d", "1");
    113 shouldBe("matrix.e", "0");
    114 shouldBe("matrix.f", "0");
     105debug("Check calling SVGMatrix methods with valid arguments");
     106shouldBeEqualToString("matrixToString(matrix.translate(10, 20))", "{ a: 2, b: 0, c: 0, d: 1, e: 20, f: 220}");
     107shouldBeEqualToString("matrixToString(matrix.scale(5))", "{ a: 10, b: 0, c: 0, d: 5, e: 0, f: 200}");
     108shouldBeEqualToString("matrixToString(matrix.scaleNonUniform(2, 3))", "{ a: 4, b: 0, c: 0, d: 3, e: 0, f: 200}");
     109shouldBeEqualToString("matrixToString(matrix.skewX(90))", "{ a: 2, b: 0, c: 32662478706390740, d: 1, e: 0, f: 200}");
     110shouldBeEqualToString("matrixToString(matrix.skewY(90))", "{ a: 2, b: 16331239353195370, c: 0, d: 1, e: 0, f: 200}");
    115111
    116112debug("");
    117 debug("Check calling 'multiply' with invalid arguments");
    118 shouldThrow("matrix.multiply()");
    119 shouldThrow("matrix.multiply(true)");
    120 shouldThrow("matrix.multiply(2)");
    121 shouldThrow("matrix.multiply('aString')");
    122 shouldThrow("matrix.multiply(svgElement)");
     113debug("Check that the matrix is still containing the correct values");
     114shouldBeEqualToString("matrixToString(matrix)", "{ a: 2, b: 0, c: 0, d: 1, e: 0, f: 200}");
    123115
    124116debug("");
    125 debug("Check calling 'translate' with invalid arguments");
    126 shouldThrow("matrix.translate()");
    127 shouldThrow("matrix.translate(true)");
    128 shouldThrow("matrix.translate(2)");
    129 shouldThrow("matrix.translate('aString')");
    130 shouldThrow("matrix.translate(svgElement)");
    131 // The following string and object arguments convert to NaN
    132 // per ECMA-262, 9.3, "ToNumber".
    133 shouldBeNonNull("matrix.translate('aString', 'aString')");
    134 shouldBeNonNull("matrix.translate(svgElement, svgElement)");
    135 shouldBeNonNull("matrix.translate(2, 'aString')");
    136 shouldBeNonNull("matrix.translate(2, svgElement)");
    137 shouldBeNonNull("matrix.translate('aString', 2)");
    138 shouldBeNonNull("matrix.translate(svgElement, 2)");
     117debug("Calling methods that throw exceptions");
     118shouldThrow("matrix.rotateFromVector(0, 4)");
     119shouldThrow("matrix.rotateFromVector(4, 0)");
     120shouldBe("matrix.a = 0", "0");
     121shouldThrow("matrix.inverse()");
    139122
    140123debug("");
    141 debug("Check calling 'scale' with invalid arguments");
    142 shouldThrow("matrix.scale()");
    143 shouldBeNonNull("matrix.scale('aString')");
    144 shouldBeNonNull("matrix.scale(svgElement)");
     124successfullyParsed = true;
    145125
    146 
    147 debug("");
    148 debug("Check calling 'scaleNonUniform' with invalid arguments");
    149 shouldThrow("matrix.scaleNonUniform()");
    150 shouldThrow("matrix.scaleNonUniform(true)");
    151 shouldThrow("matrix.scaleNonUniform(2)");
    152 shouldThrow("matrix.scaleNonUniform('aString')");
    153 shouldThrow("matrix.scaleNonUniform(svgElement)");
    154 shouldBeNonNull("matrix.scaleNonUniform('aString', 'aString')");
    155 shouldBeNonNull("matrix.scaleNonUniform(svgElement, svgElement)");
    156 shouldBeNonNull("matrix.scaleNonUniform(2, 'aString')");
    157 shouldBeNonNull("matrix.scaleNonUniform(2, svgElement)");
    158 shouldBeNonNull("matrix.scaleNonUniform('aString', 2)");
    159 shouldBeNonNull("matrix.scaleNonUniform(svgElement, 2)");
    160 
    161 debug("");
    162 debug("Check calling 'rotate' with invalid arguments");
    163 shouldThrow("matrix.rotate()");
    164 shouldBeNonNull("matrix.rotate('aString')");
    165 shouldBeNonNull("matrix.rotate(svgElement)");
    166 
    167 debug("");
    168 debug("Check calling 'rotateFromVector' with invalid arguments");
    169 shouldThrow("matrix.rotateFromVector()");
    170 shouldThrow("matrix.rotateFromVector(true)");
    171 shouldThrow("matrix.rotateFromVector(2)");
    172 shouldThrow("matrix.rotateFromVector('aString')");
    173 shouldThrow("matrix.rotateFromVector(svgElement)");
    174 shouldBeNonNull("matrix.rotateFromVector('aString', 'aString')");
    175 shouldBeNonNull("matrix.rotateFromVector(svgElement, svgElement)");
    176 shouldBeNonNull("matrix.rotateFromVector(2, 'aString')");
    177 shouldBeNonNull("matrix.rotateFromVector(2, svgElement)");
    178 shouldBeNonNull("matrix.rotateFromVector('aString', 2)");
    179 shouldBeNonNull("matrix.rotateFromVector(svgElement, 2)");
    180 
    181 debug("");
    182 debug("Check calling 'skewX' with invalid arguments");
    183 shouldThrow("matrix.skewX()");
    184 shouldBeNonNull("matrix.skewX('aString')");
    185 shouldBeNonNull("matrix.skewX(svgElement)");
    186 
    187 debug("");
    188 debug("Check calling 'skewY' with invalid arguments");
    189 shouldThrow("matrix.skewY()");
    190 shouldBeNonNull("matrix.skewY('aString')");
    191 shouldBeNonNull("matrix.skewY(svgElement)");
    192 
    193 successfullyParsed = true;
    194126</script>
    195127<script src="../../resources/js-test-post.js"></script>

  • trunk/Source/WebCore/ChangeLog

    r243701 r243703  
     12019-04-01  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        SVGMatrix.IDL methods do not conform to the specs
     4        https://bugs.webkit.org/show_bug.cgi?id=196263
     5
     6        Reviewed by Simon Fraser.
     7
     8        I think there was a misconception about these functions. The specs link
     9        is: https://www.w3.org/TR/SVG11/coords.html#InterfaceSVGMatrix.
     10
     11        Notice that the specs does not state that the SVGMethod methods should
     12        raise the exception NO_MODIFICATION_ALLOWED_ERR if the object is read
     13        only. Notice setting the attribute 'a' for example may raise this
     14        exception. Therefore, I think the specs wanted to make these operations
     15        read-only. None of the methods should raise the exception
     16        NO_MODIFICATION_ALLOWED_ERR.
     17
     18        In fact the SVG code was doing the right thing. For example SVGMatrix::scale()
     19        was calling SVGMatrixValue::scale() which was making a copy of itself,
     20        applying the scale on the copy and then returning the copy. When
     21        SVGMatrix::scale() receives the copy of the SVGMatrixValue it creates and
     22        returns a new SVGMatrix object.
     23
     24        * WebCore.xcodeproj/project.pbxproj:
     25        * svg/SVGMatrix.h:
     26        (WebCore::SVGMatrix::create):
     27        (WebCore::SVGMatrix::a const):
     28        (WebCore::SVGMatrix::b const):
     29        (WebCore::SVGMatrix::c const):
     30        (WebCore::SVGMatrix::d const):
     31        (WebCore::SVGMatrix::e const):
     32        (WebCore::SVGMatrix::f const):
     33        (WebCore::SVGMatrix::multiply const):
     34        (WebCore::SVGMatrix::inverse const):
     35        (WebCore::SVGMatrix::translate const):
     36        (WebCore::SVGMatrix::scale const):
     37        (WebCore::SVGMatrix::scaleNonUniform const):
     38        (WebCore::SVGMatrix::rotate const):
     39        (WebCore::SVGMatrix::rotateFromVector const):
     40        (WebCore::SVGMatrix::flipX const):
     41        (WebCore::SVGMatrix::flipY const):
     42        (WebCore::SVGMatrix::skewX const):
     43        (WebCore::SVGMatrix::skewY const):
     44        (WebCore::SVGMatrix::a): Deleted.
     45        (WebCore::SVGMatrix::b): Deleted.
     46        (WebCore::SVGMatrix::c): Deleted.
     47        (WebCore::SVGMatrix::d): Deleted.
     48        (WebCore::SVGMatrix::e): Deleted.
     49        (WebCore::SVGMatrix::f): Deleted.
     50        (WebCore::SVGMatrix::multiply): Deleted.
     51        (WebCore::SVGMatrix::inverse): Deleted.
     52        (WebCore::SVGMatrix::translate): Deleted.
     53        (WebCore::SVGMatrix::scale): Deleted.
     54        (WebCore::SVGMatrix::scaleNonUniform): Deleted.
     55        (WebCore::SVGMatrix::rotate): Deleted.
     56        (WebCore::SVGMatrix::rotateFromVector): Deleted.
     57        (WebCore::SVGMatrix::flipX): Deleted.
     58        (WebCore::SVGMatrix::flipY): Deleted.
     59        (WebCore::SVGMatrix::skewX): Deleted.
     60        (WebCore::SVGMatrix::skewY): Deleted.
     61        (WebCore::SVGMatrix::SVGMatrix): Deleted.
     62        * svg/SVGMatrix.idl:
     63        * svg/SVGMatrixValue.h: Removed.
     64        * svg/SVGTransform.cpp:
     65        (WebCore::SVGTransform::matrix):
     66        * svg/SVGTransformDistance.cpp:
     67        (WebCore::SVGTransformDistance::addToSVGTransform const):
     68        * svg/SVGTransformValue.h:
     69        (WebCore::SVGTransformValue::matrix const):
     70        (WebCore::SVGTransformValue::matrix):
     71        (WebCore::SVGTransformValue::svgMatrix): Deleted.
     72        (WebCore::operator==): Deleted.
     73        (WebCore::operator!=): Deleted.
     74        * svg/properties/SVGMatrixTearOff.h:
     75        * svg/properties/SVGPropertyTearOff.h:
     76        (WebCore::SVGPropertyTearOff::propertyReference const):
     77
    1782019-04-01  Simon Fraser  <simon.fraser@apple.com>
    279

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r243682 r243703  
    23382338                7CE58D501DD69A1E00128552 /* SVGNumber.h in Headers */ = {isa = PBXBuildFile; fileRef = 7CE58D4F1DD69A1E00128552 /* SVGNumber.h */; };
    23392339                7CE58D581DD7D96D00128552 /* SVGTransformValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 7CE58D561DD7D96D00128552 /* SVGTransformValue.h */; };
    2340                 7CE58D5C1DD7EC9C00128552 /* SVGMatrixValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 7CE58D5B1DD7EC9C00128552 /* SVGMatrixValue.h */; };
    23412340                7CE68344192143A800F4D928 /* UserMessageHandlerDescriptor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7CE68342192143A800F4D928 /* UserMessageHandlerDescriptor.cpp */; };
    23422341                7CE68345192143A800F4D928 /* UserMessageHandlerDescriptor.h in Headers */ = {isa = PBXBuildFile; fileRef = 7CE68343192143A800F4D928 /* UserMessageHandlerDescriptor.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    98769875                7CE58D561DD7D96D00128552 /* SVGTransformValue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SVGTransformValue.h; sourceTree = "<group>"; };
    98779876                7CE58D591DD7DE5200128552 /* SVGTransform.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SVGTransform.cpp; sourceTree = "<group>"; };
    9878                 7CE58D5B1DD7EC9C00128552 /* SVGMatrixValue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SVGMatrixValue.h; sourceTree = "<group>"; };
    98799877                7CE68342192143A800F4D928 /* UserMessageHandlerDescriptor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = UserMessageHandlerDescriptor.cpp; sourceTree = "<group>"; };
    98809878                7CE68343192143A800F4D928 /* UserMessageHandlerDescriptor.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UserMessageHandlerDescriptor.h; sourceTree = "<group>"; };
    … …  
    2403124029                                0806E57912893045007CED32 /* SVGMatrix.h */,
    2403224030                                B22278B30D00BF200071B782 /* SVGMatrix.idl */,
    24033                                 7CE58D5B1DD7EC9C00128552 /* SVGMatrixValue.h */,
    2403424031                                B22278B40D00BF200071B782 /* SVGMetadataElement.cpp */,
    2403524032                                B22278B50D00BF200071B782 /* SVGMetadataElement.h */,
    … …  
    3164131638                                0806E57A12893045007CED32 /* SVGMatrix.h in Headers */,
    3164231639                                08CA3D4412894A3800FFF260 /* SVGMatrixTearOff.h in Headers */,
    31643                                 7CE58D5C1DD7EC9C00128552 /* SVGMatrixValue.h in Headers */,
    3164431640                                B2227A4B0D00BF220071B782 /* SVGMetadataElement.h in Headers */,
    3164531641                                B2A1F2B10CEF0ABF00442F6A /* SVGMissingGlyphElement.h in Headers */,

  • trunk/Source/WebCore/svg/SVGMatrix.h

    r242978 r243703  
    11/*
    2  * Copyright (C) 2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2626#pragma once
    2727
    28 #include "SVGMatrixValue.h"
     28#include "AffineTransform.h"
    2929#include "SVGPropertyTearOff.h"
    3030
    … …  
    3232
    3333// FIXME: Remove this class once SVGMatrix becomes an alias to DOMMatrix.
    34 class SVGMatrix : public SVGPropertyTearOff<SVGMatrixValue> {
     34class SVGMatrix : public SVGPropertyTearOff<AffineTransform> {
     35    using Base = SVGPropertyTearOff<AffineTransform>;
     36    using Base::Base;
     37
    3538public:
    36     static Ref<SVGMatrix> create(SVGLegacyAnimatedProperty& animatedProperty, SVGPropertyRole role, SVGMatrixValue& value)
    37     {
    38         return adoptRef(*new SVGMatrix(animatedProperty, role, value));
    39     }
    40 
    41     static Ref<SVGMatrix> create(const SVGMatrixValue& initialValue = { })
     39    static Ref<SVGMatrix> create(SVGLegacyAnimatedProperty& animatedProperty, SVGPropertyRole role, AffineTransform& value)
     40    {
     41        return adoptRef(*new SVGMatrix(&animatedProperty, role, value));
     42    }
     43
     44    static Ref<SVGMatrix> create(const AffineTransform& initialValue = { })
    4245    {
    4346        return adoptRef(*new SVGMatrix(initialValue));
    … …  
    5154    }
    5255
    53     double a()
     56    double a() const
    5457    {
    5558        return propertyReference().a();
    … …  
    6770    }
    6871
    69     double b()
     72    double b() const
    7073    {
    7174        return propertyReference().b();
    … …  
    8386    }
    8487
    85     double c()
     88    double c() const
    8689    {
    8790        return propertyReference().c();
    … …  
    99102    }
    100103
    101     double d()
     104    double d() const
    102105    {
    103106        return propertyReference().d();
    … …  
    115118    }
    116119
    117     double e()
     120    double e() const
    118121    {
    119122        return propertyReference().e();
    … …  
    131134    }
    132135
    133     double f()
     136    double f() const
    134137    {
    135138        return propertyReference().f();
    … …  
    147150    }
    148151
    149     ExceptionOr<Ref<SVGMatrix>> multiply(SVGMatrix& secondMatrix)
    150     {
    151         if (isReadOnly())
    152             return Exception { NoModificationAllowedError };
    153 
    154         auto result = propertyReference().multiply(secondMatrix.propertyReference());
    155         commitChange();
    156 
    157         return SVGMatrix::create(result);
    158     }
    159 
    160     ExceptionOr<Ref<SVGMatrix>> inverse()
    161     {
    162         if (isReadOnly())
    163             return Exception { NoModificationAllowedError };
    164 
    165         auto result = propertyReference().inverse();
    166         if (result.hasException())
    167             return result.releaseException();
    168        
    169         commitChange();
    170         return SVGMatrix::create(result.releaseReturnValue());
    171     }
    172 
    173     ExceptionOr<Ref<SVGMatrix>> translate(float x, float y)
    174     {
    175         if (isReadOnly())
    176             return Exception { NoModificationAllowedError };
    177 
    178         auto result = propertyReference().translate(x, y);       
    179         commitChange();
    180 
    181         return SVGMatrix::create(result);
    182     }
    183 
    184     ExceptionOr<Ref<SVGMatrix>> scale(float scaleFactor)
    185     {
    186         if (isReadOnly())
    187             return Exception { NoModificationAllowedError };
    188 
    189         auto result = propertyReference().scale(scaleFactor);       
    190         commitChange();
    191 
    192         return SVGMatrix::create(result);
    193     }
    194 
    195     ExceptionOr<Ref<SVGMatrix>> scaleNonUniform(float scaleFactorX, float scaleFactorY)
    196     {
    197         if (isReadOnly())
    198             return Exception { NoModificationAllowedError };
    199 
    200         auto result = propertyReference().scaleNonUniform(scaleFactorX, scaleFactorY);       
    201         commitChange();
    202 
    203         return SVGMatrix::create(result);
    204     }
    205 
    206     ExceptionOr<Ref<SVGMatrix>> rotate(float angle)
    207     {
    208         if (isReadOnly())
    209             return Exception { NoModificationAllowedError };
    210 
    211         auto result = propertyReference().rotate(angle);       
    212         commitChange();
    213 
    214         return SVGMatrix::create(result);
    215     }
    216 
    217     ExceptionOr<Ref<SVGMatrix>> rotateFromVector(float x, float y)
    218     {
    219         if (isReadOnly())
    220             return Exception { NoModificationAllowedError };
    221 
    222         auto result = propertyReference().rotateFromVector(x, y);       
    223         if (result.hasException())
    224             return result.releaseException();
    225        
    226         commitChange();
    227         return SVGMatrix::create(result.releaseReturnValue());
    228     }
    229 
    230     ExceptionOr<Ref<SVGMatrix>> flipX()
    231     {
    232         if (isReadOnly())
    233             return Exception { NoModificationAllowedError };
    234 
    235         auto result = propertyReference().flipX();       
    236         commitChange();
    237 
    238         return SVGMatrix::create(result);
    239     }
    240 
    241     ExceptionOr<Ref<SVGMatrix>> flipY()
    242     {
    243         if (isReadOnly())
    244             return Exception { NoModificationAllowedError };
    245 
    246         auto result = propertyReference().flipY();       
    247         commitChange();
    248 
    249         return SVGMatrix::create(result);
    250     }
    251 
    252     ExceptionOr<Ref<SVGMatrix>> skewX(float angle)
    253     {
    254         if (isReadOnly())
    255             return Exception { NoModificationAllowedError };
    256 
    257         auto result = propertyReference().skewX(angle);       
    258         commitChange();
    259 
    260         return SVGMatrix::create(result);
    261     }
    262 
    263     ExceptionOr<Ref<SVGMatrix>> skewY(float angle)
    264     {
    265         if (isReadOnly())
    266             return Exception { NoModificationAllowedError };
    267 
    268         auto result = propertyReference().skewY(angle);       
    269         commitChange();
    270 
    271         return SVGMatrix::create(result);
    272     }
    273 
    274 protected:
    275     SVGMatrix(SVGLegacyAnimatedProperty& animatedProperty, SVGPropertyRole role, SVGMatrixValue& value)
    276         : SVGPropertyTearOff<SVGMatrixValue>(&animatedProperty, role, value)
    277     {
    278     }
    279 
    280     explicit SVGMatrix(const SVGMatrixValue& initialValue)
    281         : SVGPropertyTearOff<SVGMatrixValue>(initialValue)
    282     {
    283     }
    284 
    285     explicit SVGMatrix(const SVGMatrixValue* initialValue)
    286         : SVGPropertyTearOff<SVGMatrixValue>(initialValue)
    287     {
     152    Ref<SVGMatrix> multiply(SVGMatrix& secondMatrix) const
     153    {
     154        auto copy = propertyReference();
     155        copy.multiply(secondMatrix.propertyReference());
     156        return SVGMatrix::create(copy);
     157    }
     158
     159    ExceptionOr<Ref<SVGMatrix>> inverse() const
     160    {
     161        auto inverse = propertyReference().inverse();
     162        if (!inverse)
     163            return Exception { InvalidStateError, "Matrix is not invertible"_s };
     164
     165        return SVGMatrix::create(*inverse);
     166    }
     167
     168    Ref<SVGMatrix> translate(float x, float y) const
     169    {
     170        auto copy = propertyReference();
     171        copy.translate(x, y);
     172        return SVGMatrix::create(copy);
     173    }
     174
     175    Ref<SVGMatrix> scale(float scaleFactor) const
     176    {
     177        auto copy = propertyReference();
     178        copy.scale(scaleFactor);
     179        return SVGMatrix::create(copy);
     180    }
     181
     182    Ref<SVGMatrix> scaleNonUniform(float scaleFactorX, float scaleFactorY) const
     183    {
     184        auto copy = propertyReference();
     185        copy.scaleNonUniform(scaleFactorX, scaleFactorY);
     186        return SVGMatrix::create(copy);
     187    }
     188
     189    Ref<SVGMatrix> rotate(float angle) const
     190    {
     191        auto copy = propertyReference();
     192        copy.rotate(angle);
     193        return SVGMatrix::create(copy);
     194    }
     195
     196    ExceptionOr<Ref<SVGMatrix>> rotateFromVector(float x, float y) const
     197    {
     198        if (!x || !y)
     199            return Exception { TypeError };
     200
     201        auto copy = propertyReference();
     202        copy.rotateFromVector(x, y);
     203        return SVGMatrix::create(copy);
     204    }
     205
     206    Ref<SVGMatrix> flipX() const
     207    {
     208        auto copy = propertyReference();
     209        copy.flipX();
     210        return SVGMatrix::create(copy);
     211    }
     212
     213    Ref<SVGMatrix> flipY() const
     214    {
     215        auto copy = propertyReference();
     216        copy.flipY();
     217        return SVGMatrix::create(copy);
     218    }
     219
     220    Ref<SVGMatrix> skewX(float angle) const
     221    {
     222        auto copy = propertyReference();
     223        copy.skewX(angle);
     224        return SVGMatrix::create(copy);
     225    }
     226
     227    Ref<SVGMatrix> skewY(float angle) const
     228    {
     229        auto copy = propertyReference();
     230        copy.skewY(angle);
     231        return SVGMatrix::create(copy);
    288232    }
    289233};

  • trunk/Source/WebCore/svg/SVGMatrix.idl

    r222429 r243703  
    3434    attribute unrestricted double f;
    3535
    36     [MayThrowException] SVGMatrix multiply(SVGMatrix secondMatrix);
    37     [MayThrowException] SVGMatrix inverse();
    38     [MayThrowException] SVGMatrix translate(unrestricted float x, unrestricted float y);
    39     [MayThrowException] SVGMatrix scale(unrestricted float scaleFactor);
    40     [MayThrowException] SVGMatrix scaleNonUniform(unrestricted float scaleFactorX, unrestricted float scaleFactorY);
    41     [MayThrowException] SVGMatrix rotate(unrestricted float angle);
    42     [MayThrowException] SVGMatrix rotateFromVector(unrestricted float x, unrestricted float y);
    43     [MayThrowException] SVGMatrix flipX();
    44     [MayThrowException] SVGMatrix flipY();
    45     [MayThrowException] SVGMatrix skewX(unrestricted float angle);
    46     [MayThrowException] SVGMatrix skewY(unrestricted float angle);
     36    [NewObject] SVGMatrix multiply(SVGMatrix secondMatrix);
     37    [MayThrowException, NewObject] SVGMatrix inverse();
     38    [NewObject] SVGMatrix translate(unrestricted float x, unrestricted float y);
     39    [NewObject] SVGMatrix scale(unrestricted float scaleFactor);
     40    [NewObject] SVGMatrix scaleNonUniform(unrestricted float scaleFactorX, unrestricted float scaleFactorY);
     41    [NewObject] SVGMatrix rotate(unrestricted float angle);
     42    [MayThrowException, NewObject] SVGMatrix rotateFromVector(unrestricted float x, unrestricted float y);
     43    [NewObject] SVGMatrix flipX();
     44    [NewObject] SVGMatrix flipY();
     45    [NewObject] SVGMatrix skewX(unrestricted float angle);
     46    [NewObject] SVGMatrix skewY(unrestricted float angle);
    4747};

  • trunk/Source/WebCore/svg/SVGTransform.cpp

    r208705 r243703  
    3333Ref<SVGMatrix> SVGTransform::matrix()
    3434{
    35     return SVGMatrixTearOff::create(*this, propertyReference().svgMatrix());
     35    return SVGMatrixTearOff::create(*this, propertyReference().matrix());
    3636}
    3737

  • trunk/Source/WebCore/svg/SVGTransformDistance.cpp

    r208705 r243703  
    161161SVGTransformValue SVGTransformDistance::addToSVGTransform(const SVGTransformValue& transform) const
    162162{
    163     ASSERT(m_type == transform.type() || transform == SVGTransformValue());
     163    ASSERT(m_type == transform.type() || !transform.isValid());
    164164   
    165165    SVGTransformValue newTransform(transform);

  • trunk/Source/WebCore/svg/SVGTransformValue.h

    r208705 r243703  
    22 * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
    33 * Copyright (C) 2004, 2005 Rob Buis <buis@kde.org>
     4 * Copyright (C) 2019 Apple Inc.  All rights reserved.
    45 *
    56 * This library is free software; you can redistribute it and/or
    … …  
    2122#pragma once
    2223
     24#include "AffineTransform.h"
    2325#include "FloatPoint.h"
    24 #include "SVGMatrixValue.h"
    2526
    2627namespace WebCore {
    … …  
    5152    SVGTransformType type() const { return m_type; }
    5253
    53     SVGMatrixValue& svgMatrix() { return static_cast<SVGMatrixValue&>(m_matrix); }
    54     AffineTransform matrix() const { return m_matrix; }
     54    const AffineTransform& matrix() const { return m_matrix; }
     55    AffineTransform& matrix() { return m_matrix; }
    5556    void updateSVGMatrix();
    5657
    … …  
    7475
    7576private:
    76     friend bool operator==(const SVGTransformValue&, const SVGTransformValue&);
    77 
    7877    SVGTransformType m_type { SVG_TRANSFORM_UNKNOWN };
    7978    float m_angle { 0 };
    … …  
    8281};
    8382
    84 inline bool operator==(const SVGTransformValue& a, const SVGTransformValue& b)
    85 {
    86     return a.m_type == b.m_type && a.m_angle == b.m_angle && a.m_matrix == b.m_matrix;
    87 }
    88 
    89 inline bool operator!=(const SVGTransformValue& a, const SVGTransformValue& b)
    90 {
    91     return !(a == b);
    92 }
    93 
    9483} // namespace WebCore

  • trunk/Source/WebCore/svg/properties/SVGMatrixTearOff.h

    r232613 r243703  
    2727class SVGMatrixTearOff final : public SVGMatrix {
    2828public:
    29     static Ref<SVGMatrixTearOff> create(SVGTransform& parent, SVGMatrixValue& value)
     29    static Ref<SVGMatrixTearOff> create(SVGTransform& parent, AffineTransform& value)
    3030    {
    31         ASSERT_UNUSED(value, &parent.propertyReference().svgMatrix() == &value);
     31        ASSERT_UNUSED(value, &parent.propertyReference().matrix() == &value);
    3232        auto result = adoptRef(*new SVGMatrixTearOff(parent));
    3333        parent.addChild(makeWeakPtr(result.get()));
    … …  
    3535    }
    3636
    37     SVGMatrixValue& propertyReference() final { return m_parent->propertyReference().svgMatrix(); }
     37    const AffineTransform& propertyReference() const final { return m_parent->propertyReference().matrix(); }
     38    AffineTransform& propertyReference() final { return m_parent->propertyReference().matrix(); }
    3839
    39     void setValue(SVGMatrixValue& value) final { m_parent->propertyReference().setMatrix(value); }
     40    void setValue(AffineTransform& value) final { m_parent->propertyReference().setMatrix(value); }
    4041
    4142    void commitChange() final

  • trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h

    r242978 r243703  
    6161    }
    6262
     63    virtual const PropertyType& propertyReference() const { return *m_value; }
    6364    virtual PropertyType& propertyReference() { return *m_value; }
    6465    SVGLegacyAnimatedProperty* animatedProperty() const { return m_animatedProperty.get(); }
Note: See TracChangeset for help on using the changeset viewer.