Introduce KeyframeEffectReadOnly interface

Introduce KeyframeEffectReadOnly interface

The Web Animations spec includes a KeyframeEffect interface which inherits from
KeyframeEffectReadOnly which in turn inherits from AnimationEffectReadOnly.
(http://w3c.github.io/web-animations/#the-keyframeeffect-interfaces). Blink
currently does not implement KeyframeEffectReadOnly; KeyframeEffect inherits
directly from AnimationEffectReadOnly.

This patch adds the KeyframeEffectReadOnly interface and corresponding
implementation. Since KeyframeEffectReadOnly objects are never actually used
in the code, and we only need to provide a read-only view on a KeyframeEffect
object, the KeyframeEffectReadOnly::create functions simply pass through to the
KeyframeEffect::create functions.

Note that these interfaces are only exposed behind the
experimental-web-platform-features flag.

Several expectations are correspondingly updated for the web-animations suite of
the imported web platform tests.

BUG=624639, 600248
Committed: https://crrev.com/3120c553f66737e082fe12cace7ca25743f1dfe8
Cr-Commit-Position: refs/heads/master@{#422009}

[suzyh_UTC10 (ex-contributor), 2016-09-26 06:28:45]

suzyh@chromium.org changed reviewers:
+ alancutter@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-26 06:32:06]

Since we don't expose anything of the KeyframeEffect{,ReadOnly} interfaces
except for the constructors, I've moved the private members up to
KeyframeEffectReadOnly but none of the accessors or other functions in the
KeyframeEffect interface. Let me know if you think I should move anything else
from KeyframeEffect to KeyframeEffectReadOnly.

[suzyh_UTC10 (ex-contributor), 2016-09-26 06:32:13]

The CQ bit was checked by suzyh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 06:32:29]

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

[suzyh_UTC10 (ex-contributor), 2016-09-26 07:12:46]

On 2016/09/26 at 06:32:06, suzyh wrote:
> Since we don't expose anything of the KeyframeEffect{,ReadOnly} interfaces
except for the constructors, I've moved the private members up to
KeyframeEffectReadOnly but none of the accessors or other functions in the
KeyframeEffect interface. Let me know if you think I should move anything else
from KeyframeEffect to KeyframeEffectReadOnly.

Oh, I should also update references to KeyframeEffect::Priority and its values.
Whoops. I'll do that tomorrow.

[alancutter (OOO until 2018), 2016-09-26 07:43:33]

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h (right):

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:48: //
http://w3c.github.io/web-animations/#keyframe-effect
Should this be http://w3c.github.io/web-animations/#keyframeeffectreadonly?

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.idl
(right):

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.idl:36:
Constructor(Element? target, (sequence<Dictionary> or Dictionary)? effect,
KeyframeEffectOptions timing),
I'm not sure we can expose these constructors just yet. Won't the NOTREACHED()
methods get reached?
Are there tests that create these and use it in animations?

[commit-bot: I haz the power, 2016-09-26 07:51:41]

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

[commit-bot: I haz the power, 2016-09-26 07:51:42]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[suzyh_UTC10 (ex-contributor), 2016-09-27 03:44:46]

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/imported/wpt/web-animations/interfaces/KeyframeEffect/getComputedTiming-expected.txt
(right):

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/imported/wpt/web-animations/interfaces/KeyframeEffect/getComputedTiming-expected.txt:2:
FAIL values of getComputedTiming() when a KeyframeEffectReadOnly is constructed
without any KeyframeEffectOptions object assert_equals: computed fill expected
"none" but got "both"
Turns out these "expected 'none' but got 'both'" errors were because
constructing a KeyframeEffectReadOnly was resulting in this line passing false
as the second argument:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/animation...

... so wrapping KeyframeEffect::create fixes this too.

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h (right):

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:48: //
http://w3c.github.io/web-animations/#keyframe-effect
On 2016/09/26 at 07:43:33, alancutter wrote:
> Should this be http://w3c.github.io/web-animations/#keyframeeffectreadonly?

Done.

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.idl
(right):

https://codereview.chromium.org/2369833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.idl:36:
Constructor(Element? target, (sequence<Dictionary> or Dictionary)? effect,
KeyframeEffectOptions timing),
On 2016/09/26 at 07:43:33, alancutter wrote:
> I'm not sure we can expose these constructors just yet. Won't the NOTREACHED()
methods get reached?
> Are there tests that create these and use it in animations?

As discussed offline, I've made the KeyframeEffectReadOnly::create functions
simple wrappers around the mutable class's create functions instead.

[suzyh_UTC10 (ex-contributor), 2016-09-27 03:44:54]

The CQ bit was checked by suzyh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 03:45:16]

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

[alancutter (OOO until 2018), 2016-09-27 05:09:05]

lgtm after comment resolved.

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.cpp
(right):

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.cpp:75: }
It's fine to have empty destructors in headers.

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h (right):

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:52: enum
Priority { DefaultPriority, TransitionPriority };
Leave Priority in KeyframeEffect since it's not necessary for the
KeyframeEffectReadOnly interface. This also saves unnecessary churn elsewhere in
the code.

[commit-bot: I haz the power, 2016-09-27 05:17:14]

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

[commit-bot: I haz the power, 2016-09-27 05:17:16]

Dry run: This issue passed the CQ dry run.

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:13:49]

Description was changed from

==========
Introduce KeyframeEffectReadOnly interface

The Web Animations spec includes a KeyframeEffect interface which inherits from
KeyframeEffectReadOnly which in turn inherits from AnimationEffectReadOnly.
(http://w3c.github.io/web-animations/#the-keyframeeffect-interfaces). Blink
currently does not implement KeyframeEffectReadOnly; KeyframeEffect inherits
directly from AnimationEffectReadOnly.

This patch adds the KeyframeEffectReadOnly interface and corresponding
implementation. The minimal amount of code required to implement the
KeyframeEffectReadOnly constructor and create functions is moved or copied from
KeyframeEffect.

Note that these interfaces are only exposed behind the
experimental-web-platform-features flag.

Several expectations are correspondingly updated for the web-animations suite of
the imported web platform tests.

BUG=624639,600248
==========

to

==========
Introduce KeyframeEffectReadOnly interface

The Web Animations spec includes a KeyframeEffect interface which inherits from
KeyframeEffectReadOnly which in turn inherits from AnimationEffectReadOnly.
(http://w3c.github.io/web-animations/#the-keyframeeffect-interfaces). Blink
currently does not implement KeyframeEffectReadOnly; KeyframeEffect inherits
directly from AnimationEffectReadOnly.

This patch adds the KeyframeEffectReadOnly interface and corresponding
implementation. Since KeyframeEffectReadOnly objects are never actually used
in the code, and we only need to provide a read-only view on a KeyframeEffect
object, the KeyframeEffectReadOnly::create functions simply pass through to the
KeyframeEffect::create functions.

Note that these interfaces are only exposed behind the
experimental-web-platform-features flag.

Several expectations are correspondingly updated for the web-animations suite of
the imported web platform tests.

BUG=624639,600248
==========

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:21:39]

suzyh@chromium.org changed reviewers:
+ shans@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:21:59]

+shans for core/ approval and sanity check that this is a reasonable way to
implement this.

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.cpp
(right):

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.cpp:75: }
On 2016/09/27 at 05:09:05, alancutter wrote:
> It's fine to have empty destructors in headers.

Done.

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h (right):

https://codereview.chromium.org/2369833002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:52: enum
Priority { DefaultPriority, TransitionPriority };
On 2016/09/27 at 05:09:05, alancutter wrote:
> Leave Priority in KeyframeEffect since it's not necessary for the
KeyframeEffectReadOnly interface. This also saves unnecessary churn elsewhere in
the code.

Done.

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:22:36]

suzyh@chromium.org changed reviewers:
+ dstockwell@chromium.org
- shans@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:22:48]

suzyh@chromium.org changed reviewers:
+ shans@chromium.org
- dstockwell@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:22:49]

Wait, shans isn't an OWNER. -shans +dstockwell

[suzyh_UTC10 (ex-contributor), 2016-09-27 07:23:16]

suzyh@chromium.org changed reviewers:
+ dstockwell@chromium.org
- shans@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-28 00:57:57]

Hmm, I don't think the last email went to Doug like it was intended to. So,
trying again:

+dstockwell for core/ approval and sanity check that this is a reasonable way to
implement this.

[esprehn, 2016-09-28 04:53:20]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2016-09-28 04:53:21]

https://codereview.chromium.org/2369833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h (right):

https://codereview.chromium.org/2369833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:2: * Copyright
(C) 2016 Google Inc. All rights reserved.
Please use the short copyright on all new files

[dstockwell, 2016-09-29 04:02:08]

lgtm, but see esprehn's comment

[suzyh_UTC10 (ex-contributor), 2016-09-30 00:22:34]

On 2016/09/28 at 04:53:21, esprehn wrote:
>
https://codereview.chromium.org/2369833002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h
(right):
> 
>
https://codereview.chromium.org/2369833002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h:2: *
Copyright (C) 2016 Google Inc. All rights reserved.
> Please use the short copyright on all new files

Done.

[suzyh_UTC10 (ex-contributor), 2016-09-30 00:22:39]

The CQ bit was checked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-09-30 00:22:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org, dstockwell@chromium.org
Link to the patchset: https://codereview.chromium.org/2369833002/#ps80001
(title: "Use short form of copyright message")

[commit-bot: I haz the power, 2016-09-30 00:23:08]

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-09-30 01:52:45]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-30 01:57:49]

Description was changed from

==========
Introduce KeyframeEffectReadOnly interface

The Web Animations spec includes a KeyframeEffect interface which inherits from
KeyframeEffectReadOnly which in turn inherits from AnimationEffectReadOnly.
(http://w3c.github.io/web-animations/#the-keyframeeffect-interfaces). Blink
currently does not implement KeyframeEffectReadOnly; KeyframeEffect inherits
directly from AnimationEffectReadOnly.

This patch adds the KeyframeEffectReadOnly interface and corresponding
implementation. Since KeyframeEffectReadOnly objects are never actually used
in the code, and we only need to provide a read-only view on a KeyframeEffect
object, the KeyframeEffectReadOnly::create functions simply pass through to the
KeyframeEffect::create functions.

Note that these interfaces are only exposed behind the
experimental-web-platform-features flag.

Several expectations are correspondingly updated for the web-animations suite of
the imported web platform tests.

BUG=624639,600248
==========

to

==========
Introduce KeyframeEffectReadOnly interface

The Web Animations spec includes a KeyframeEffect interface which inherits from
KeyframeEffectReadOnly which in turn inherits from AnimationEffectReadOnly.
(http://w3c.github.io/web-animations/#the-keyframeeffect-interfaces). Blink
currently does not implement KeyframeEffectReadOnly; KeyframeEffect inherits
directly from AnimationEffectReadOnly.

This patch adds the KeyframeEffectReadOnly interface and corresponding
implementation. Since KeyframeEffectReadOnly objects are never actually used
in the code, and we only need to provide a read-only view on a KeyframeEffect
object, the KeyframeEffectReadOnly::create functions simply pass through to the
KeyframeEffect::create functions.

Note that these interfaces are only exposed behind the
experimental-web-platform-features flag.

Several expectations are correspondingly updated for the web-animations suite of
the imported web platform tests.

BUG=624639,600248

Committed: https://crrev.com/3120c553f66737e082fe12cace7ca25743f1dfe8
Cr-Commit-Position: refs/heads/master@{#422009}
==========

[commit-bot: I haz the power, 2016-09-30 01:57:51]

Patchset 5 (id:??) landed as
https://crrev.com/3120c553f66737e082fe12cace7ca25743f1dfe8
Cr-Commit-Position: refs/heads/master@{#422009}