[GeometryInferface] add Constructor(DOMString transformList).

[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819
Committed: https://crrev.com/59c27f138a1ef700d4adb552e8a9c0452d7a8d4e
Cr-Commit-Position: refs/heads/master@{#438107}

[Hwanseung Lee, 2016-11-27 02:07:24]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-27 02:07:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Hwanseung Lee, 2016-11-27 03:42:55]

Description was changed from

==========
[WIP][GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist

BUG=388780, 660819
==========

to

==========
[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819
==========

[commit-bot: I haz the power, 2016-11-27 03:49:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-27 03:49:43]

Dry run: This issue passed the CQ dry run.

[Hwanseung Lee, 2016-11-27 03:57:41]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-27 03:57:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-27 06:49:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-27 06:49:58]

Dry run: This issue passed the CQ dry run.

[Hwanseung Lee, 2016-11-27 09:09:04]

hs1217.lee@samsung.com changed reviewers:
+ dominicc@chromium.org, jinho.bang@samsung.com

[Hwanseung Lee, 2016-11-27 09:09:04]

@dominicc, zino
PTAL, thank you.

[zino, 2016-11-29 01:09:24]

https://codereview.chromium.org/2516973002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMMatrix.cpp (right):

https://codereview.chromium.org/2516973002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMMatrix.cpp:257: return
static_cast<DOMMatrix*>(
This casting isn't a right way.
I think we can make a new internal function. (e.g. setMatrixValueInternal())

[Hwanseung Lee, 2016-11-29 14:12:02]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 14:12:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Hwanseung Lee, 2016-11-29 14:31:25]

i fixed it.

https://codereview.chromium.org/2516973002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMMatrix.cpp (right):

https://codereview.chromium.org/2516973002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMMatrix.cpp:257: return
static_cast<DOMMatrix*>(
On 2016/11/29 01:09:24, zino wrote:
> This casting isn't a right way.
> I think we can make a new internal function. (e.g. setMatrixValueInternal())

Done.

[commit-bot: I haz the power, 2016-11-29 15:27:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-29 15:27:04]

Dry run: This issue passed the CQ dry run.

[Hwanseung Lee, 2016-11-30 12:36:54]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 12:37:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zino, 2016-11-30 13:39:31]

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:101: return new
DOMMatrixReadOnly(transformList, exceptionState);
DOMMatrixReadOnly*
matrix = new DOMMatrixReadOnly(TransformationMatrix::create());
matrix->setMatrixValueInternal(transformList, exceptionState);
return matrix;

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:270:
DOMMatrixReadOnly::DOMMatrixReadOnly(const String& inputString,
This constructor is unncessary I think :)

Please see my previous comment.

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:353: void
DOMMatrixReadOnly::setMatrixValueInternal(const String& inputString,
Could you please rename this to better name? (e.g. setMatrixFromString())

[Hwanseung Lee, 2016-11-30 14:00:31]

@zino
i fixed it.

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:101: return new
DOMMatrixReadOnly(transformList, exceptionState);
On 2016/11/30 13:39:31, zino wrote:
> DOMMatrixReadOnly*
> matrix = new DOMMatrixReadOnly(TransformationMatrix::create());
> matrix->setMatrixValueInternal(transformList, exceptionState);
> return matrix;

Done.

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:270:
DOMMatrixReadOnly::DOMMatrixReadOnly(const String& inputString,
On 2016/11/30 13:39:31, zino wrote:
> This constructor is unncessary I think :)
> 
> Please see my previous comment.

Done.

https://codereview.chromium.org/2516973002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:353: void
DOMMatrixReadOnly::setMatrixValueInternal(const String& inputString,
On 2016/11/30 13:39:31, zino wrote:
> Could you please rename this to better name? (e.g. setMatrixFromString())

Done.

[Hwanseung Lee, 2016-11-30 14:00:36]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 14:00:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zino, 2016-11-30 14:41:28]

lgtm with nit

https://codereview.chromium.org/2516973002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrix.cpp (right):

https://codereview.chromium.org/2516973002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrix.cpp:263:
DOMMatrixReadOnly::setMatrixValueFromString(inputString, exceptionState);
nit: I think you can just call setMatrixValueFromString() without namescope

[Hwanseung Lee, 2016-11-30 14:46:18]

thank you. i fixed it.

https://codereview.chromium.org/2516973002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrix.cpp (right):

https://codereview.chromium.org/2516973002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrix.cpp:263:
DOMMatrixReadOnly::setMatrixValueFromString(inputString, exceptionState);
On 2016/11/30 14:41:28, zino wrote:
> nit: I think you can just call setMatrixValueFromString() without namescope

Done.

[Hwanseung Lee, 2016-11-30 14:54:17]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 14:54:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 16:17:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 16:17:34]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-12-05 08:03:45]

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html
(right):

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html:111:
assert_throws(new SyntaxError(), () => { new DOMMatrixReadOnly(1, 2, 3, 4, 5,
6); },
Could we have tests for the relative lengths and box-size dependent cases?

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:375: "Relative lengths
not supported.");
Maybe something which mentions "must" and "absolute lengths"? That's how the
spec is written. "not supported" leaves some doubt in my mind whether it's just
a Chrome limitation or something, but this is a spec requirement.

Same below for box size.

[Hwanseung Lee, 2016-12-05 15:13:05]

The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-05 15:13:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Hwanseung Lee, 2016-12-05 15:15:19]

i fixed something which you comment.
could you review again?

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html
(right):

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html:111:
assert_throws(new SyntaxError(), () => { new DOMMatrixReadOnly(1, 2, 3, 4, 5,
6); },
On 2016/12/05 08:03:45, dominicc wrote:
> Could we have tests for the relative lengths and box-size dependent cases?

Done.

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:375: "Relative lengths
not supported.");
On 2016/12/05 08:03:45, dominicc wrote:
> Maybe something which mentions "must" and "absolute lengths"? That's how the
> spec is written. "not supported" leaves some doubt in my mind whether it's
just
> a Chrome limitation or something, but this is a spec requirement.
> 
> Same below for box size.

i modified strings. is it fine?

[commit-bot: I haz the power, 2016-12-05 16:46:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-05 16:46:42]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-12-06 08:44:24]

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html
(right):

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html:120:
"Must have an absolute unit.");
Let's make these descriptive about what's being tested. For example something
like "using relative units should throw a SyntaxError".

This description makes it sound like it's OK to have relative units as long as
you have *one* absolute unit, which isn't right.

In general test assertion descriptions which use 'should' tend to be easy to
understand.

Be consistent. Above you started with a lower initial c for can't but here you
you caps Must.

I think if you look at test harness errors you shouldn't end with a period--I
think it adds one where it echoes the test failure message--but double check to
be sure.

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:376: "Must have an
absolute lengths. It contains a relative lenghts");
Spelling--lengths

Grammar--"a relative" is signular but "lengths" is plural.

I'm not sure about not ending with a period when you're writing two sentences
this way. What about:

"Lengths must be absolute"

or

"Lengths must be absolute, not relative"

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:387: "Must have an
absolute lengths. The "
Maybe "Lengths must be absolute, not depend on the box size"

[Hwanseung Lee, 2016-12-07 13:56:59]

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html
(right):

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/geometry-interfaces-dom-matrix-readonly.html:120:
"Must have an absolute unit.");
On 2016/12/06 08:44:24, dominicc wrote:
> Let's make these descriptive about what's being tested. For example something
> like "using relative units should throw a SyntaxError".
> 
> This description makes it sound like it's OK to have relative units as long as
> you have *one* absolute unit, which isn't right.
> 
> In general test assertion descriptions which use 'should' tend to be easy to
> understand.
> 
> Be consistent. Above you started with a lower initial c for can't but here you
> you caps Must.
> 
> I think if you look at test harness errors you shouldn't end with a period--I
> think it adds one where it echoes the test failure message--but double check
to
> be sure.


i checked test harness errors message. your thought is right.
so i removed period at end of test description.

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:376: "Must have an
absolute lengths. It contains a relative lenghts");
On 2016/12/06 08:44:24, dominicc wrote:
> Spelling--lengths
> 
> Grammar--"a relative" is signular but "lengths" is plural.
> 
> I'm not sure about not ending with a period when you're writing two sentences
> this way. What about:
> 
> "Lengths must be absolute"
> 
> or
> 
> "Lengths must be absolute, not relative"

Done.

https://codereview.chromium.org/2516973002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:387: "Must have an
absolute lengths. The "
On 2016/12/06 08:44:24, dominicc wrote:
> Maybe "Lengths must be absolute, not depend on the box size"

Done.

[dominicc (has gone to gerrit), 2016-12-13 07:47:16]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-12-13 07:47:16]

lgtm

[dominicc (has gone to gerrit), 2016-12-13 07:47:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
jinho.bang@samsung.com
Link to the patchset: https://codereview.chromium.org/2516973002/#ps180001
(title: "[GeometryInferface] add Constructor(DOMString transformList).")

[commit-bot: I haz the power, 2016-12-13 07:47:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 09:17:43]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1481615236089750,
"parent_rev": "cd2ae5cd31cc23d3ae2eb4afadf58ddf26992cbf", "commit_rev":
"0dc221437ecf519f673c89c8e472ab0378f7c804"}

[commit-bot: I haz the power, 2016-12-13 09:18:12]

Description was changed from

==========
[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819
==========

to

==========
[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819

Review-Url: https://codereview.chromium.org/2516973002
==========

[commit-bot: I haz the power, 2016-12-13 09:18:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-12-13 09:23:54]

Description was changed from

==========
[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819

Review-Url: https://codereview.chromium.org/2516973002
==========

to

==========
[GeometryInferface] add Constructor(DOMString transformList).

1. if transformList is the empty string,
 set in to the string "matrix(1, 0, 0, 1, 0, 0)".
2. Parse transformList by following the syntax description[1].
 if parsing failed, throw SyntaxError.
3. Set is2D to false if the <transform-list> consists of any 3D
 Transform functions. Otherwise set is2D to true.
4. Transform all <transform-function>s to 4x4 matrices.
5. Post-multiply all matrices from left to right.
6. Return the result of invoking create a matrix of type
 DOMMatrixReadOnly or DOMMatrix as appropriate.

spec list:
https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
[1] = https://drafts.csswg.org/css-transforms-1/#svg-syntax

BUG=388780, 660819

Committed: https://crrev.com/59c27f138a1ef700d4adb552e8a9c0452d7a8d4e
Cr-Commit-Position: refs/heads/master@{#438107}
==========

[commit-bot: I haz the power, 2016-12-13 09:23:56]

Patchset 10 (id:??) landed as
https://crrev.com/59c27f138a1ef700d4adb552e8a9c0452d7a8d4e
Cr-Commit-Position: refs/heads/master@{#438107}