Bug 167115 – Web Animations should be off by default and enabled as an experimental feature

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

Parts of the Web Animations spec are implemented in WebKit and only conditioned by a compile-time flag (WEB_ANIMATIONS) rather than a runtime flag (WebAnimations already exist but isn't used). We need to condition those bits of API for the runtime flag and expose that flag as an experimental feature so that it is off by default in most builds.

[Antoine Quint]

<rdar://problem/30048963>

[Antoine Quint]

Created attachment 299023 [details]
Patch

[Antoine Quint]

Comment on attachment 299023 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299023&action=review

> Source/WebKit2/Shared/WebPreferencesDefinitions.h:321
> +    macro(WebAnimationsEnabled, webAnimationsEnabled, Bool, bool, DEFAULT_EXPERIMENTAL_FEATURES_ENABLED, "Web Animations", "Web Animations prototype") \

Even though this is now in the FOR_EACH_WEBKIT_EXPERIMENTAL_FEATURE_PREFERENCE block, I don't see "Web Animations" listed in my builds, both Debug and Release. Also, in a Debug build, using DEFAULT_EXPERIMENTAL_FEATURES_ENABLED makes the feature available by default, whereas setting `false` in its stead correctly turns it off.

[Build Bot]

Comment on attachment 299023 [details]
Patch

Attachment 299023 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2903477

New failing tests:
webanimations/Document.html

[Build Bot]

Created attachment 299025 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 299023 [details]
Patch

Attachment 299023 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2903484

New failing tests:
webanimations/Document.html

[Build Bot]

Created attachment 299026 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 299023 [details]
Patch

Attachment 299023 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2903481

New failing tests:
webanimations/Document.html

[Build Bot]

Created attachment 299027 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 299023 [details]
Patch

Attachment 299023 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2903491

New failing tests:
webanimations/Document.html

[Build Bot]

Created attachment 299028 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Antoine Quint]

Created attachment 299359 [details]
Patch

[Antoine Quint]

Created attachment 299360 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 299360 [details]
Patch

r=me. This looks great!

There is legacy code in a LayoutTest enabling webkit animations that is no longer necessary. You should just be able  to delete it:

LayoutTests/webanimations/script-tests/Document.js
5:    window.testRunner.overridePreference("WebKitWebAnimationsEnabled", "1");

[Antoine Quint]

Created attachment 299366 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 299366 [details]
Patch for landing

Clearing flags on attachment: 299366

Committed r210976: <http://trac.webkit.org/changeset/210976>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

This change appears to have broken the Windows build:

https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/83295

[Joseph Pecoraro]

Comment on attachment 299366 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=299366&action=review

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:860
> +    prefsPrivate3->setWebAnimationsEnabled(TRUE);

This doesn't exist, it needs to be added. I'll look into that right now.

[Joseph Pecoraro]

Reopening just to make sure EWS can try out the patch I'm about to put up for Windows.

[Joseph Pecoraro]

Created attachment 299388 [details]
[PATCH] For Bots

[Joseph Pecoraro]

(In reply to comment #21)
> Created attachment 299388 [details]
> [PATCH] For Bots

Great, the Windows bot fails to update. I can try to land this blind and see if it fixes things.

[Joseph Pecoraro]

Attempted Windows Build Fix:
<https://trac.webkit.org/changeset/210991>

I'll watch the bots.

[Joseph Pecoraro]

Alex landed a better fix:
<https://trac.webkit.org/changeset/210993>

[Antoine Quint]

Thanks Alex!