Implement Animation constructor

Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag in the Window context.

Test expectations are updated accordingly.

BUG=674513
Review-Url: https://codereview.chromium.org/2806623003
Cr-Commit-Position: refs/heads/master@{#478215}
Committed: https://chromium.googlesource.com/chromium/src/+/8427660616e176502578ec0a82982b3ed3038864

[suzyh_UTC10 (ex-contributor), 2017-04-07 04:16:59]

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

[suzyh_UTC10 (ex-contributor), 2017-04-07 04:17:22]

Seeking early feedback / sanity check

[suzyh_UTC10 (ex-contributor), 2017-04-07 04:17:40]

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

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

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-07 04:20:44]

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

[commit-bot: I haz the power, 2017-04-07 04:20:45]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[alancutter (OOO until 2018), 2017-04-07 06:43:26]

I'm not sure that we're ready to ship the Animation constructor just yet,
especially without an experimental implementation. We should throw an "Illegal
constructor" exception in the constructor if the experimental flag is disabled.

[suzyh_UTC10 (ex-contributor), 2017-04-10 05:34:25]

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

[commit-bot: I haz the power, 2017-04-10 05:34:37]

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), 2017-04-10 06:03:08]

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

[commit-bot: I haz the power, 2017-04-10 06:03:17]

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-10 07:47:51]

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

[commit-bot: I haz the power, 2017-04-10 07:47:52]

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_...)

[suzyh_UTC10 (ex-contributor), 2017-04-10 08:06:38]

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

[commit-bot: I haz the power, 2017-04-10 08:06: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-04-10 09:35:02]

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

[commit-bot: I haz the power, 2017-04-10 09:35:03]

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_...)

[suzyh_UTC10 (ex-contributor), 2017-04-10 23:53:14]

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

[commit-bot: I haz the power, 2017-04-10 23:54:09]

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-11 01:49:04]

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

[commit-bot: I haz the power, 2017-04-11 01:49:04]

Dry run: This issue passed the CQ dry run.

[suzyh_UTC10 (ex-contributor), 2017-04-26 01:40:19]

On 2017/04/07 at 06:43:26, alancutter wrote:
> I'm not sure that we're ready to ship the Animation constructor just yet,
especially without an experimental implementation. We should throw an "Illegal
constructor" exception in the constructor if the experimental flag is disabled.

OK, with this latest patch, without --enable-experimental-web-platform-features,
I get:
> var b = new Animation();
Uncaught TypeError: Failed to construct 'Animation': Illegal constructor
    at <anonymous>:1:9

For comparison, in Chrome Unstable, I get:
> var b = new Animation();
Uncaught ReferenceError: Animation is not defined
    at <anonymous>:1:13

[suzyh_UTC10 (ex-contributor), 2017-04-26 01:40:31]

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

[commit-bot: I haz the power, 2017-04-26 01:41: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-04-26 02:54:35]

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

[commit-bot: I haz the power, 2017-04-26 02:54:37]

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_...)

[suzyh_UTC10 (ex-contributor), 2017-04-26 03:41:31]

Description was changed from

==========
Expose Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. Test expectations are updated accordingly.

BUG=674513
==========

to

==========
Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag; if the flag is not
enabled then the functions throw an "Illegal constructor" TypeError.

Test expectations are updated accordingly.

BUG=674513
==========

[suzyh_UTC10 (ex-contributor), 2017-04-26 03:45:04]

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

[commit-bot: I haz the power, 2017-04-26 03:45:27]

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), 2017-04-26 04:34:48]

lgtm

https://codereview.chromium.org/2806623003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2806623003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/Animation.cpp:97: NOTREACHED();
This is redundant with the ToDocument() call below.

[commit-bot: I haz the power, 2017-04-26 05:32:06]

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

[commit-bot: I haz the power, 2017-04-26 05:32:07]

Dry run: This issue passed the CQ dry run.

[suzyh_UTC10 (ex-contributor), 2017-05-25 06:01:42]

I've sent a PSA to blink-dev about this change:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/WkNcwh6aISA/discus....
I'll give it a few days in case people have any comments, and then land this
patch after branch point.

[suzyh_UTC10 (ex-contributor), 2017-05-25 07:34:20]

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

[commit-bot: I haz the power, 2017-05-25 07:34:33]

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-25 09:37:52]

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

[commit-bot: I haz the power, 2017-05-25 09:37:54]

Dry run: This issue passed the CQ dry run.

[suzyh_UTC10 (ex-contributor), 2017-06-06 06:05:56]

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

[commit-bot: I haz the power, 2017-06-06 06:06:37]

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-06 06:08:58]

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

[commit-bot: I haz the power, 2017-06-06 06:08:59]

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

[suzyh_UTC10 (ex-contributor), 2017-06-06 06:13:24]

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

[commit-bot: I haz the power, 2017-06-06 06:13:51]

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), 2017-06-06 06:17:36]

Description was changed from

==========
Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag; if the flag is not
enabled then the functions throw an "Illegal constructor" TypeError.

Test expectations are updated accordingly.

BUG=674513
==========

to

==========
Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag in the Window context.

Test expectations are updated accordingly.

BUG=674513
==========

[suzyh_UTC10 (ex-contributor), 2017-06-06 06:21:48]

After the discussion on the blink-dev thread, I've changed this to use the
Blink-specific Exposed(Arguments) extended attribute:
https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So...

This now means there should be no web-visible change from this patch unless the
WebAnimationsAPI flag is enabled. I don't understand the
virtual/service-worker-navigation-preload-disabled/webexposed expectation
change; I'm not sure whether that is expected in this case.

Alan, WDYT of this version?

https://codereview.chromium.org/2806623003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2806623003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/Animation.cpp:97: NOTREACHED();
On 2017/04/26 at 04:34:48, alancutter wrote:
> This is redundant with the ToDocument() call below.

Removed.

https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/Animation.cpp:92: if
(!RuntimeEnabledFeatures::webAnimationsAPIEnabled()) {
Given I'm now selectively exposing this, do you think I should instead simply
assert that this flag is enabled?

[commit-bot: I haz the power, 2017-06-06 07:19:48]

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

[commit-bot: I haz the power, 2017-06-06 07:19:49]

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

[suzyh_UTC10 (ex-contributor), 2017-06-07 00:32:42]

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

[commit-bot: I haz the power, 2017-06-07 00:33:12]

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-07 00:46:11]

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

[commit-bot: I haz the power, 2017-06-07 00:46:13]

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...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[suzyh_UTC10 (ex-contributor), 2017-06-07 01:24:06]

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

[commit-bot: I haz the power, 2017-06-07 01:24:23]

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-07 03:22:25]

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

[commit-bot: I haz the power, 2017-06-07 03:22:27]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2017-06-07 05:36:15]

lgtm

https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/Animation.cpp:92: if
(!RuntimeEnabledFeatures::webAnimationsAPIEnabled()) {
On 2017/06/06 at 06:21:48, suzyh_UTC10 wrote:
> Given I'm now selectively exposing this, do you think I should instead simply
assert that this flag is enabled?

Yes, this branch should be unreachable now.

https://codereview.chromium.org/2806623003/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/Animation.h (right):

https://codereview.chromium.org/2806623003/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/Animation.h:79: // Web Animations API
Bindings constructors.
I'd like to see the word IDL used here, even just "IDL constructors". "Bindings"
is a bit too generic and overloaded in Chrome.

[alancutter (OOO until 2018), 2017-06-07 05:36:21]

lgtm

[alancutter (OOO until 2018), 2017-06-07 05:36:21]

lgtm

[alancutter (OOO until 2018), 2017-06-07 05:36:48]

On 2017/06/07 at 05:36:21, alancutter wrote:
> lgtm

This is what happens when you press space to scroll down while the LGTM button
is focused.

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:21:41]

On 2017/06/07 at 05:36:15, alancutter wrote:
> lgtm
> 
>
https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/animation/Animation.cpp (right):
> 
>
https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/animation/Animation.cpp:92: if
(!RuntimeEnabledFeatures::webAnimationsAPIEnabled()) {
> On 2017/06/06 at 06:21:48, suzyh_UTC10 wrote:
> > Given I'm now selectively exposing this, do you think I should instead
simply assert that this flag is enabled?
> 
> Yes, this branch should be unreachable now.

Removed.


>
https://codereview.chromium.org/2806623003/diff/260001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/animation/Animation.h (right):
> 
>
https://codereview.chromium.org/2806623003/diff/260001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/animation/Animation.h:79: // Web Animations API
Bindings constructors.
> I'd like to see the word IDL used here, even just "IDL constructors".
"Bindings" is a bit too generic and overloaded in Chrome.

Done.

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:23:13]

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

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:23:49]

+peria for a final check

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:25:27]

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

[commit-bot: I haz the power, 2017-06-09 04:25:45]

Dry run: CQ is trying da patch.

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

[bashi, 2017-06-09 04:32:38]

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

[bashi, 2017-06-09 04:32:40]

lgtm on Exposed use.

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:37:01]

On 2017/06/09 at 04:21:41, suzyh_UTC10 wrote:
> On 2017/06/07 at 05:36:15, alancutter wrote:
> > lgtm
> > 
> >
https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/animation/Animation.cpp (right):
> > 
> >
https://codereview.chromium.org/2806623003/diff/220001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/animation/Animation.cpp:92: if
(!RuntimeEnabledFeatures::webAnimationsAPIEnabled()) {
> > On 2017/06/06 at 06:21:48, suzyh_UTC10 wrote:
> > > Given I'm now selectively exposing this, do you think I should instead
simply assert that this flag is enabled?
> > 
> > Yes, this branch should be unreachable now.
> 
> Removed.

Whoops, operator error. Now DCHECKed instead of removed.

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:43:55]

The CQ bit was checked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2017-06-09 04:43:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org, bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/2806623003/#ps300001
(title: "Add back in DCHECK about flag being enabled")

[commit-bot: I haz the power, 2017-06-09 04:44:07]

CQ is trying da patch.

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

[peria, 2017-06-09 05:52:19]

lgtm

LGTM on Exposed use.

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

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1496983435511330,
"parent_rev": "01f09fba7a7f4510b216ecb16c55ff52d2a2ba33", "commit_rev":
"8427660616e176502578ec0a82982b3ed3038864"}

[commit-bot: I haz the power, 2017-06-09 06:24:31]

Description was changed from

==========
Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag in the Window context.

Test expectations are updated accordingly.

BUG=674513
==========

to

==========
Implement Animation constructor

This patch changes the Animation object from NoInterfaceObject to having
a constructor, and implements two corresponding Animation::create
functions. The implementations are only exposed behind the
--enable-experimental-web-platform-features flag in the Window context.

Test expectations are updated accordingly.

BUG=674513

Review-Url: https://codereview.chromium.org/2806623003
Cr-Commit-Position: refs/heads/master@{#478215}
Committed:
https://chromium.googlesource.com/chromium/src/+/8427660616e176502578ec0a8298...
==========

[commit-bot: I haz the power, 2017-06-09 06:24:33]

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