Make WebKitCSSMatrix an alias of DOMMatrix

Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple alias of it.

Intent-to-ship Link : 
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/
cAiTOdSGyes/Su7dlx32EQAJ

BUG=581955
Review-Url: https://codereview.chromium.org/2709763004
Cr-Commit-Position: refs/heads/master@{#479481}
Committed: https://chromium.googlesource.com/chromium/src/+/0ac2b97eb15689ba196f7f88b184b3a17f13d30d

[Byoungkwon Ko, 2017-02-22 17:16:47]

gogag2@gmail.com changed reviewers:
+ hs1217.lee@samsung.com, jinho.bang@samsung.com

[Byoungkwon Ko, 2017-02-22 17:16:47]

PTAL

[Byoungkwon Ko, 2017-02-22 17:16:54]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 17:17:46]

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, 2017-02-22 18:25:02]

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

[commit-bot: I haz the power, 2017-02-22 18:25:03]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Byoungkwon Ko, 2017-02-22 22:39:46]

Description was changed from

==========
Make WebkitCSSMatrix an alias of DOMMatrix

Make WebkitCSSMatrix an alias of DOMMatrix
DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

BUG=581955
==========

to

==========
Make WebkitCSSMatrix an alias of DOMMatrix

Make WebkitCSSMatrix an alias of DOMMatrix
DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

BUG=581955
==========

[Byoungkwon Ko, 2017-02-22 22:39:46]

gogag2@gmail.com changed reviewers:
- hs1217.lee@samsung.com

[Byoungkwon Ko, 2017-03-09 13:11:14]

gogag2@gmail.com changed reviewers:
+ foolip@chromium.org

[Byoungkwon Ko, 2017-03-09 13:11:15]

@foolip

I am making WebkitCSSMatrix an alias of DOMMatrix.
After inheriting DOMMatrix, WebkitCSSMatrix will call DOMMatrix object.
I don't know whether it is right way or not.
Can you review my code?
Thanks : )

[zino, 2017-03-09 15:59:50]

https://codereview.chromium.org/2709763004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSMatrix.h (right):

https://codereview.chromium.org/2709763004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSMatrix.h:38: class CSSMatrix final :
public DOMMatrix {
IMHO, if the CSSMatrix has only interfaces and the DOMMatrix inherits it, then
We don't need force casting and duplicated implementation.

[foolip, 2017-03-14 14:43:39]

The idea I had in issue 581955 was that WebKitCSSMatrix would be an alias of
DOMMatrix in the sense that window.WebKitCSSMatrix===window.DOMMatrix.

This can be achieved by by adding "readonly attribute DOMMatrixConstructor
WebKitCSSMatrix" in Window.idl, or some partial interface.

It's probably best to consider that change together with shipping DOMMatrix, in
https://groups.google.com/a/chromium.org/d/msg/blink-dev/cAiTOdSGyes/Su7dlx32...

Are you working with xlai@? Can you comment on the Intent to Ship thread if you
think these changes should be made together?

[Byoungkwon Ko, 2017-03-16 01:44:19]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 01:44:50]

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, 2017-03-16 03:38:56]

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

[commit-bot: I haz the power, 2017-03-16 03:38:57]

Dry run: This issue passed the CQ dry run.

[Byoungkwon Ko, 2017-03-16 04:26:38]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 04:26:58]

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, 2017-03-16 05:02:22]

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

[commit-bot: I haz the power, 2017-03-16 05:02:23]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Byoungkwon Ko, 2017-03-16 07:31:37]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 07:31:50]

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, 2017-03-16 09:28:31]

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

[commit-bot: I haz the power, 2017-03-16 09:28:32]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Byoungkwon Ko, 2017-03-16 14:43:07]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 14:43:22]

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

[Byoungkwon Ko, 2017-03-16 16:27:41]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 16:28:25]

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

[Byoungkwon Ko, 2017-03-16 17:56:50]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 17:57:47]

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, 2017-03-16 19:53:33]

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

[commit-bot: I haz the power, 2017-03-16 19:53:34]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[zino, 2017-03-17 15:54:31]

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Window.idl (right):

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Window.idl:202: readonly attribute
DOMMatrixConstructor WebKitCSSMatrix;
I think partial interface is better if possible.

[foolip, 2017-03-21 08:47:30]

OK, this CL matches what I think we should do, and it's encouraging that there
aren't any new test failures. Hopefully that means WebKitCSSMatrix and DOMMatrix
are very close and not that WebKitCSSMatrix test coverage is insufficient...

I think this should be coupled with shipping the DOMMatrix so that those changes
definitely go in the same milestone. Landing that after DOMMatrix is enabled
would work, or just putting everything in a single big CL would be OK too.

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

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:304: return
String::format("matrix(%f, %f, %f, %f, %f, %f)", a(), b(), c(), d(),
Is this a drive-by change, or does it affect the output in  any case?

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Window.idl (right):

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Window.idl:202: readonly attribute
DOMMatrixConstructor WebKitCSSMatrix;
On 2017/03/17 15:54:31, zino wrote:
> I think partial interface is better if possible.

That would be nice, but if moved up to after the WebKitMutationObserver alias
this is fine as well.

[Byoungkwon Ko, 2017-04-26 01:31:35]

@foolip

I have been checking below ship intent.

https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/cAiTOdSGyes/Su...


As I think, it is almost completed.
Could I reflect them?
Thanks.

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

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:304: return
String::format("matrix(%f, %f, %f, %f, %f, %f)", a(), b(), c(), d(),
On 2017/03/21 08:47:30, foolip_UTC7 wrote:
> Is this a drive-by change, or does it affect the output in  any case?

yes, when we were using steam, we should fix decimal fraction part for float
variable.
So I changed to string::format.
Is it better to use precision() ???

[foolip, 2017-04-26 07:29:19]

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

https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:304: return
String::format("matrix(%f, %f, %f, %f, %f, %f)", a(), b(), c(), d(),
On 2017/04/26 01:31:35, Byoungkwon Ko wrote:
> On 2017/03/21 08:47:30, foolip_UTC7 wrote:
> > Is this a drive-by change, or does it affect the output in  any case?
> 
> yes, when we were using steam, we should fix decimal fraction part for float
> variable.
> So I changed to string::format.
> Is it better to use precision() ???

Hmm, this is actually related to https://github.com/w3c/fxtf-drafts/issues/120.
Should double precision be used here?

In any case, I would suggest doing this change separately.

[foolip, 2017-04-26 07:30:51]

On 2017/04/26 01:31:35, Byoungkwon Ko wrote:
> @foolip
> 
> I have been checking below ship intent.
> 
>
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/cAiTOdSGyes/Su...
> 
> 
> As I think, it is almost completed.
> Could I reflect them?
> Thanks.

Yeah, I think this CL is now what we need, but the intent is waiting for
confirmation of a final plan. It's important that this CL isn't landed before
DOMMatrix itself is exposed, so please coordinate with xlai@ and the others.

[foolip, 2017-04-26 07:31:00]

Description was changed from

==========
Make WebkitCSSMatrix an alias of DOMMatrix

Make WebkitCSSMatrix an alias of DOMMatrix
DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

BUG=581955
==========

to

==========
Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

BUG=581955
==========

[Byoungkwon Ko, 2017-04-26 13:03:39]

On 2017/04/26 07:29:19, foolip OOO until May 2 wrote:
>
https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp (right):
> 
>
https://codereview.chromium.org/2709763004/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/dom/DOMMatrixReadOnly.cpp:304: return
> String::format("matrix(%f, %f, %f, %f, %f, %f)", a(), b(), c(), d(),
> On 2017/04/26 01:31:35, Byoungkwon Ko wrote:
> > On 2017/03/21 08:47:30, foolip_UTC7 wrote:
> > > Is this a drive-by change, or does it affect the output in  any case?
> > 
> > yes, when we were using steam, we should fix decimal fraction part for float
> > variable.
> > So I changed to string::format.
> > Is it better to use precision() ???
> 
> Hmm, this is actually related to
https://github.com/w3c/fxtf-drafts/issues/120.
> Should double precision be used here?
> 
> In any case, I would suggest doing this change separately.

Yeah, I will split it, and then I will send you a review again.
thanks.

[Byoungkwon Ko, 2017-04-26 13:05:58]

On 2017/04/26 07:30:51, foolip OOO until May 2 wrote:
> On 2017/04/26 01:31:35, Byoungkwon Ko wrote:
> > @foolip
> > 
> > I have been checking below ship intent.
> > 
> >
>
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/cAiTOdSGyes/Su...
> > 
> > 
> > As I think, it is almost completed.
> > Could I reflect them?
> > Thanks.
> 
> Yeah, I think this CL is now what we need, but the intent is waiting for
> confirmation of a final plan. It's important that this CL isn't landed before
> DOMMatrix itself is exposed, so please coordinate with xlai@ and the others.

Ok, I am waiting it, I will coordinate with xlai@ and the others.
Thanks.

[Byoungkwon Ko, 2017-04-26 13:09:40]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 13:09:54]

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, 2017-04-26 15:00:48]

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

[commit-bot: I haz the power, 2017-04-26 15:00:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[simonp, 2017-05-04 08:35:20]

simonp@opera.com changed reviewers:
+ simonp@opera.com

[simonp, 2017-05-04 08:35:21]

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Window.idl (right):

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Window.idl:208: readonly attribute
DOMMatrixConstructor WebKitCSSMatrix;
This is different from what the spec requires. The spec says:

> For historical reasons, Window objects must also have writable, configurable,
non-enumerable properties named SVGMatrix and WebKitCSSMatrix whose value is the
DOMMatrix interface object.

The difference is that a readonly attribute has a getter instead of being a
straight property. The attribute is also enumerable.

[simonp, 2017-05-04 11:15:28]

On 2017/05/04 08:35:21, simonp wrote:

> The difference is that a readonly attribute has a getter instead of being a
> straight property. The attribute is also enumerable.

I have created a test for this in
https://github.com/w3c/web-platform-tests/pull/5780

[foolip, 2017-05-04 14:43:25]

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Window.idl (right):

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Window.idl:208: readonly attribute
DOMMatrixConstructor WebKitCSSMatrix;
On 2017/05/04 08:35:21, simonp wrote:
> This is different from what the spec requires. The spec says:
> 
> > For historical reasons, Window objects must also have writable,
configurable,
> non-enumerable properties named SVGMatrix and WebKitCSSMatrix whose value is
the
> DOMMatrix interface object.
> 
> The difference is that a readonly attribute has a getter instead of being a
> straight property. The attribute is also enumerable.

This affects all of our aliases, so that's probably something we'd want to fix
in our bindings generator somehow. Let's see how the tests fail first.

[zino, 2017-05-05 11:43:21]

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Window.idl (right):

https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Window.idl:208: readonly attribute
DOMMatrixConstructor WebKitCSSMatrix;
On 2017/05/04 14:43:25, foolip wrote:
> On 2017/05/04 08:35:21, simonp wrote:
> > This is different from what the spec requires. The spec says:
> > 
> > > For historical reasons, Window objects must also have writable,
> configurable,
> > non-enumerable properties named SVGMatrix and WebKitCSSMatrix whose value is
> the
> > DOMMatrix interface object.
> > 
> > The difference is that a readonly attribute has a getter instead of being a
> > straight property. The attribute is also enumerable.
> 
> This affects all of our aliases, so that's probably something we'd want to fix
> in our bindings generator somehow. Let's see how the tests fail first.

FYI, I tried simonp@'s test in latest chromium with this patch.

** Test Result **
This is a testharness.js-based test.
PASS Equivalence test 
FAIL Property descriptor for WebKitCSSMatrix assert_true: writable expected true
got false
PASS Property descriptor for DOMMatrix 
Harness: the test ran to completion.

We probably should remove |readonly|?

[zino, 2017-05-05 11:52:15]

gogag@,

Could you rebase this CL and then fix some layout test erros (e.g.
global-listing-interfaces)?
Because the tests are not related to spec bugs.

Also, once https://codereview.chromium.org/2859293002/ is landed, I think more
tests will be fixed.
(You can do test it locally)

[Byoungkwon Ko, 2017-05-05 14:08:25]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 14:08:44]

Dry run: CQ is trying da patch.

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

[Byoungkwon Ko, 2017-05-05 14:09:00]

On 2017/05/05 11:43:21, zino wrote:
>
https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/frame/Window.idl (right):
> 
>
https://codereview.chromium.org/2709763004/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/frame/Window.idl:208: readonly attribute
> DOMMatrixConstructor WebKitCSSMatrix;
> On 2017/05/04 14:43:25, foolip wrote:
> > On 2017/05/04 08:35:21, simonp wrote:
> > > This is different from what the spec requires. The spec says:
> > > 
> > > > For historical reasons, Window objects must also have writable,
> > configurable,
> > > non-enumerable properties named SVGMatrix and WebKitCSSMatrix whose value
is
> > the
> > > DOMMatrix interface object.
> > > 
> > > The difference is that a readonly attribute has a getter instead of being
a
> > > straight property. The attribute is also enumerable.
> > 
> > This affects all of our aliases, so that's probably something we'd want to
fix
> > in our bindings generator somehow. Let's see how the tests fail first.
> 
> FYI, I tried simonp@'s test in latest chromium with this patch.
> 
> ** Test Result **
> This is a testharness.js-based test.
> PASS Equivalence test 
> FAIL Property descriptor for WebKitCSSMatrix assert_true: writable expected
true
> got false
> PASS Property descriptor for DOMMatrix 
> Harness: the test ran to completion.
> 
> We probably should remove |readonly|?

FYI, I deleted it.
Thanks.

[commit-bot: I haz the power, 2017-05-05 16:00:31]

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

[commit-bot: I haz the power, 2017-05-05 16:00:33]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Byoungkwon Ko, 2017-05-06 06:46:32]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-06 06:46:57]

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, 2017-05-06 08:30:50]

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

[commit-bot: I haz the power, 2017-05-06 08:30:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Byoungkwon Ko, 2017-05-06 13:31:50]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-06 13:32:45]

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, 2017-05-06 15:18:49]

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

[commit-bot: I haz the power, 2017-05-06 15:18:50]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Byoungkwon Ko, 2017-05-09 01:39:19]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 01:40:04]

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, 2017-05-09 03:44:51]

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

[commit-bot: I haz the power, 2017-05-09 03:44:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Byoungkwon Ko, 2017-05-09 15:31:16]

@zino @fooli @simonp

I have question regarding transforms/cssmatrix-crash.html test.
Throwing exception is right as following spec when it is not using absolute
lengths units.

Spec Link: https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix

Could I change it?

[simonp, 2017-05-12 11:36:56]

On 2017/05/09 15:31:16, Byoungkwon Ko wrote:
> @zino @fooli @simonp
> 
> I have question regarding transforms/cssmatrix-crash.html test.
> Throwing exception is right as following spec when it is not using absolute
> lengths units.
> 
> Spec Link: https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix
> 
> Could I change it?

That seems to already be covered by
https://github.com/w3c/web-platform-tests/blob/master/css/geometry-1/DOMMatri...
so I suppose you can remove the other test.

[simonp, 2017-05-12 11:51:27]

On 2017/05/05 14:09:00, Byoungkwon Ko wrote:

> > We probably should remove |readonly|?
> 
> FYI, I deleted it.
> Thanks.

I think this still does not have the right semantics, and will still fail the
test (you'll have a getter and a setter instead of value, it will be
enumerable...).

Maybe WebIDL should have syntax for interface aliases... I filed
https://github.com/heycam/webidl/issues/362

[Byoungkwon Ko, 2017-05-14 15:10:11]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-14 15:10:21]

Dry run: CQ is trying da patch.

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

[Byoungkwon Ko, 2017-05-14 15:10:56]

On 2017/05/12 11:36:56, simonp wrote:
> On 2017/05/09 15:31:16, Byoungkwon Ko wrote:
> > @zino @fooli @simonp
> > 
> > I have question regarding transforms/cssmatrix-crash.html test.
> > Throwing exception is right as following spec when it is not using absolute
> > lengths units.
> > 
> > Spec Link: https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix
> > 
> > Could I change it?
> 
> That seems to already be covered by
>
https://github.com/w3c/web-platform-tests/blob/master/css/geometry-1/DOMMatri...
> so I suppose you can remove the other test.

thanks for your information : )
I fixed it.

[commit-bot: I haz the power, 2017-05-14 16:49:56]

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

[commit-bot: I haz the power, 2017-05-14 16:49:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zino, 2017-05-15 18:51:35]

jinho.bang@samsung.com changed reviewers:
+ fserb@chromium.org

[zino, 2017-05-15 18:52:24]

I added fserb@ for this patch.
He is working on this feature actively now. :)

[zino, 2017-05-15 19:07:35]

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/SmokeTests (left):

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/SmokeTests:978: transforms/cssmatrix-crash.html
Why do you remove this test?

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/transforms/cssmatrix-2d-interface.xhtml
(right):

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/transforms/cssmatrix-2d-interface.xhtml:287:
debug("Test not invertible matrix");
Test for non-invertible matrix.

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/transforms/cssmatrix-crash.html (left):

https://codereview.chromium.org/2709763004/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/transforms/cssmatrix-crash.html:1: <!DOCTYPE
html>
Why do you remove this test?

[fserb, 2017-05-15 19:30:40]

back to this change, at some point this week I might just patch it and move
forward with it, if gogag2 is AWOL. :)

[Byoungkwon Ko, 2017-05-16 06:44:59]

On 2017/05/15 19:30:40, fserb wrote:
> back to this change, at some point this week I might just patch it and move
> forward with it, if gogag2 is AWOL. :)

fserb@

I am not AWOL;;;
I am waiting confirmation of below spec issue.
https://github.com/heycam/webidl/issues/362
Plz check below simonp's comment;;

simonp

I think this still does not have the right semantics, and will still fail the
test (you'll have a getter and a setter instead of value, it will be
enumerable...).

Maybe WebIDL should have syntax for interface aliases... I filed
https://github.com/heycam/webidl/issues/362

As you know, the spec is not resolved yet.
And I have been waiting many spec issue from Feb, and we have fixed some issues
regarding making WebKitCSSMatrix an alias of DOMMatrix.
It's almost over now. Plz give me a little time.
Thanks.

[simonp, 2017-05-16 07:59:12]

On 2017/05/16 06:44:59, Byoungkwon Ko wrote:
> On 2017/05/15 19:30:40, fserb wrote:
> > back to this change, at some point this week I might just patch it and move
> > forward with it, if gogag2 is AWOL. :)
> 
> fserb@
> 
> I am not AWOL;;;
> I am waiting confirmation of below spec issue.
> https://github.com/heycam/webidl/issues/362
> Plz check below simonp's comment;;
> 
> simonp
> 
> I think this still does not have the right semantics, and will still fail the
> test (you'll have a getter and a setter instead of value, it will be
> enumerable...).
> 
> Maybe WebIDL should have syntax for interface aliases... I filed
> https://github.com/heycam/webidl/issues/362
> 
> As you know, the spec is not resolved yet.
> And I have been waiting many spec issue from Feb, and we have fixed some
issues
> regarding making WebKitCSSMatrix an alias of DOMMatrix.
> It's almost over now. Plz give me a little time.
> Thanks.

Please see https://github.com/heycam/webidl/issues/362#issuecomment-301702127
for a possible resolution to the aliasing issue. Y'all's opinion would be
appreciated.

[simonp, 2017-05-16 16:31:08]

On 2017/05/12 11:51:27, simonp wrote:
> On 2017/05/05 14:09:00, Byoungkwon Ko wrote:
> 
> > > We probably should remove |readonly|?
> > 
> > FYI, I deleted it.
> > Thanks.
> 
> I think this still does not have the right semantics, and will still fail the
> test (you'll have a getter and a setter instead of value, it will be
> enumerable...).
> 
> Maybe WebIDL should have syntax for interface aliases... I filed
> https://github.com/heycam/webidl/issues/362

I was mistaken. I assumed normal WebIDL semantics for attribute would be
generated, but when the value is a constructor the property descriptor will in
fact have the right semantics for an alias.

https://github.com/heycam/webidl/issues/362#issuecomment-301836563

So I think we don't need to wait for [LegacyWindowAlias] for this CL; doing the
same thing as is done for webkitURL is OK as it's black-box equivalent.

[fserb, 2017-05-18 15:52:24]

On 2017/05/16 at 16:31:08, simonp wrote:
> On 2017/05/12 11:51:27, simonp wrote:
> > On 2017/05/05 14:09:00, Byoungkwon Ko wrote:
> > 
> > > > We probably should remove |readonly|?
> > > 
> > > FYI, I deleted it.
> > > Thanks.
> > 
> > I think this still does not have the right semantics, and will still fail
the
> > test (you'll have a getter and a setter instead of value, it will be
> > enumerable...).
> > 
> > Maybe WebIDL should have syntax for interface aliases... I filed
> > https://github.com/heycam/webidl/issues/362
> 
> I was mistaken. I assumed normal WebIDL semantics for attribute would be
generated, but when the value is a constructor the property descriptor will in
fact have the right semantics for an alias.
> 
> https://github.com/heycam/webidl/issues/362#issuecomment-301836563
> 
> So I think we don't need to wait for [LegacyWindowAlias] for this CL; doing
the same thing as is done for webkitURL is OK as it's black-box equivalent.

Yep. I think this unblocks this CL, right?

[Byoungkwon Ko, 2017-05-18 15:55:04]

On 2017/05/18 15:52:24, fserb wrote:
> On 2017/05/16 at 16:31:08, simonp wrote:
> > On 2017/05/12 11:51:27, simonp wrote:
> > > On 2017/05/05 14:09:00, Byoungkwon Ko wrote:
> > > 
> > > > > We probably should remove |readonly|?
> > > > 
> > > > FYI, I deleted it.
> > > > Thanks.
> > > 
> > > I think this still does not have the right semantics, and will still fail
> the
> > > test (you'll have a getter and a setter instead of value, it will be
> > > enumerable...).
> > > 
> > > Maybe WebIDL should have syntax for interface aliases... I filed
> > > https://github.com/heycam/webidl/issues/362
> > 
> > I was mistaken. I assumed normal WebIDL semantics for attribute would be
> generated, but when the value is a constructor the property descriptor will in
> fact have the right semantics for an alias.
> > 
> > https://github.com/heycam/webidl/issues/362#issuecomment-301836563
> > 
> > So I think we don't need to wait for [LegacyWindowAlias] for this CL; doing
> the same thing as is done for webkitURL is OK as it's black-box equivalent.
> 
> Yep. I think this unblocks this CL, right?

Yes, right.
if https://codereview.chromium.org/2846523002/ for stringifier is fixed, we can
fix this issue also.
plz review the issue.

[Byoungkwon Ko, 2017-06-08 01:40:52]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 01:41:11]

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, 2017-06-08 03:47:15]

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

[commit-bot: I haz the power, 2017-06-08 03:47:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Byoungkwon Ko, 2017-06-09 03:47:02]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 03:47:10]

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, 2017-06-09 03:55:54]

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

[commit-bot: I haz the power, 2017-06-09 03:55:56]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Byoungkwon Ko, 2017-06-09 22:26:27]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 22:26:57]

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, 2017-06-10 00:02:56]

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

[commit-bot: I haz the power, 2017-06-10 00:02:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Byoungkwon Ko, 2017-06-10 04:31:12]

The CQ bit was checked by gogag2@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-10 04:31:18]

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, 2017-06-10 05:14:09]

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

[commit-bot: I haz the power, 2017-06-10 05:14:10]

Dry run: This issue passed the CQ dry run.

[Byoungkwon Ko, 2017-06-10 12:08:55]

Dear Reviewers

PTAL : )

[fserb, 2017-06-10 15:07:54]

lgtm

[Byoungkwon Ko, 2017-06-12 06:13:51]

@foolip

PTAL :)

[fserb, 2017-06-12 06:43:01]

lgtm

[simonp, 2017-06-12 07:13:38]

On 2017/06/10 12:08:55, Byoungkwon Ko wrote:
> Dear Reviewers
> 
> PTAL : )

lgtm

[zino, 2017-06-12 07:16:12]

lgtm with nits:
  - Please add intent-to-ship link to CL description.
  - Shouldn't we enable DOMMatrix by default in this patch?

[blink-reviews, 2017-06-12 15:39:00]

This should be a different patch. I'll do that after getting back on the
intent-to-ship.

On Mon, Jun 12, 2017 at 3:16 AM <jinho.bang@samsung.com> wrote:

> lgtm with nits:
> - Please add intent-to-ship link to CL description.
> - Shouldn't we enable DOMMatrix by default in this patch?
>
> https://codereview.chromium.org/2709763004/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2017-06-12 15:39:01]

This should be a different patch. I'll do that after getting back on the
intent-to-ship.

On Mon, Jun 12, 2017 at 3:16 AM <jinho.bang@samsung.com> wrote:

> lgtm with nits:
> - Please add intent-to-ship link to CL description.
> - Shouldn't we enable DOMMatrix by default in this patch?
>
> https://codereview.chromium.org/2709763004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Byoungkwon Ko, 2017-06-12 15:46:56]

Description was changed from

==========
Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

BUG=581955
==========

to

==========
Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

Intent-to-ship Link : 
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/
cAiTOdSGyes/Su7dlx32EQAJ

BUG=581955
==========

[foolip, 2017-06-13 13:15:17]

fserb@, I'm unsure what "This should be a different patch. I'll do that after
getting back on the intent-to-ship" means for this CL, but all
global-interface-listing-expected.txt LGTM if landing this is the right thing.

[Byoungkwon Ko, 2017-06-13 14:45:12]

The CQ bit was checked by gogag2@gmail.com

[commit-bot: I haz the power, 2017-06-13 14:45:29]

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, 2017-06-13 14:54:50]

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

[commit-bot: I haz the power, 2017-06-13 14:54:53]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[fserb, 2017-06-14 15:01:28]

fserb@chromium.org changed reviewers:
+ timvolodine@chromium.org

[fserb, 2017-06-14 15:01:32]

timvolodine: android_webview/tools/system_webview_shell

[timvolodine, 2017-06-14 17:37:09]

On 2017/06/14 15:01:32, fserb wrote:
> timvolodine: android_webview/tools/system_webview_shell

lgtm

[fserb, 2017-06-14 17:44:47]

The CQ bit was checked by fserb@chromium.org

[fserb, 2017-06-14 17:44:48]

lgtm

[commit-bot: I haz the power, 2017-06-14 17:45:03]

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, 2017-06-14 17:53:55]

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

[commit-bot: I haz the power, 2017-06-14 17:53:57]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[fserb, 2017-06-14 17:56:10]

fserb@chromium.org changed reviewers:
+ dpranke@chromium.org

[fserb, 2017-06-14 17:56:14]

dpranke: third_party/WebKit/LayoutTests/SmokeTests

[Dirk Pranke, 2017-06-14 19:03:18]

lgtm

[fserb, 2017-06-14 19:53:34]

The CQ bit was checked by fserb@chromium.org

[commit-bot: I haz the power, 2017-06-14 19:54:02]

CQ is trying da patch.

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

[fserb, 2017-06-14 19:54:32]

The CQ bit was unchecked by fserb@chromium.org

[fserb, 2017-06-14 19:54:36]

The CQ bit was checked by fserb@chromium.org

[commit-bot: I haz the power, 2017-06-14 19:55:09]

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, 2017-06-14 20:06:14]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1497470076484590,
"parent_rev": "690256f7a5cfb0fe8e61cf19c8e026f3ac11af88", "commit_rev":
"0ac2b97eb15689ba196f7f88b184b3a17f13d30d"}

[commit-bot: I haz the power, 2017-06-14 20:06:25]

Description was changed from

==========
Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

Intent-to-ship Link : 
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/
cAiTOdSGyes/Su7dlx32EQAJ

BUG=581955
==========

to

==========
Make WebkitCSSMatrix an alias of DOMMatrix

DOMMatrix was changed sufficiently that WebKitCSSMatrix can be made a simple
alias of it.

Intent-to-ship Link : 
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/
cAiTOdSGyes/Su7dlx32EQAJ

BUG=581955

Review-Url: https://codereview.chromium.org/2709763004
Cr-Commit-Position: refs/heads/master@{#479481}
Committed:
https://chromium.googlesource.com/chromium/src/+/0ac2b97eb15689ba196f7f88b184...
==========

[commit-bot: I haz the power, 2017-06-14 20:06:28]

Committed patchset #16 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/0ac2b97eb15689ba196f7f88b184...

[zino, 2017-06-17 00:17:49]

A revert of this CL (patchset #16 id:300001) has been created in
https://codereview.chromium.org/2943213002/ by jinho.bang@samsung.com.

The reason for reverting is: The WebKitCSSMatrix is not working after this
change. Becuase the GeometryInterface is not shipped yet..