Bug 167115 - Web Animations should be off by default and enabled as an experimental feature
Summary: Web Animations should be off by default and enabled as an experimental feature
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-17 02:45 PST by Antoine Quint
Modified: 2017-01-21 12:21 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.93 KB, patch)
2017-01-17 02:47 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (864.08 KB, application/zip)
2017-01-17 03:54 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (823.44 KB, application/zip)
2017-01-17 03:58 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (1.60 MB, application/zip)
2017-01-17 04:02 PST, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (9.30 MB, application/zip)
2017-01-17 04:10 PST, Build Bot
no flags Details
Patch (14.38 KB, patch)
2017-01-20 11:50 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (12.53 KB, patch)
2017-01-20 11:52 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (13.94 KB, patch)
2017-01-20 12:03 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
[PATCH] For Bots (4.19 KB, patch)
2017-01-20 14:38 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-01-17 02:45:09 PST
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.
Comment 1 Antoine Quint 2017-01-17 02:45:26 PST
<rdar://problem/30048963>
Comment 2 Antoine Quint 2017-01-17 02:47:52 PST
Created attachment 299023 [details]
Patch
Comment 3 Antoine Quint 2017-01-17 02:50:46 PST
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.
Comment 4 Build Bot 2017-01-17 03:54:45 PST
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
Comment 5 Build Bot 2017-01-17 03:54:50 PST
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
Comment 6 Build Bot 2017-01-17 03:58:30 PST
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
Comment 7 Build Bot 2017-01-17 03:58:35 PST
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
Comment 8 Build Bot 2017-01-17 04:02:13 PST
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
Comment 9 Build Bot 2017-01-17 04:02:18 PST
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
Comment 10 Build Bot 2017-01-17 04:10:40 PST
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
Comment 11 Build Bot 2017-01-17 04:10:46 PST
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
Comment 12 Antoine Quint 2017-01-20 11:50:11 PST
Created attachment 299359 [details]
Patch
Comment 13 Antoine Quint 2017-01-20 11:52:22 PST
Created attachment 299360 [details]
Patch
Comment 14 Joseph Pecoraro 2017-01-20 11:58:14 PST
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");
Comment 15 Antoine Quint 2017-01-20 12:03:06 PST
Created attachment 299366 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2017-01-20 13:05:37 PST
Comment on attachment 299366 [details]
Patch for landing

Clearing flags on attachment: 299366

Committed r210976: <http://trac.webkit.org/changeset/210976>
Comment 17 WebKit Commit Bot 2017-01-20 13:05:43 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 Ryan Haddad 2017-01-20 14:12:18 PST
This change appears to have broken the Windows build:

https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/83295
Comment 19 Joseph Pecoraro 2017-01-20 14:23:34 PST
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.
Comment 20 Joseph Pecoraro 2017-01-20 14:38:31 PST
Reopening just to make sure EWS can try out the patch I'm about to put up for Windows.
Comment 21 Joseph Pecoraro 2017-01-20 14:38:47 PST
Created attachment 299388 [details]
[PATCH] For Bots
Comment 22 Joseph Pecoraro 2017-01-20 14:50:00 PST
(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.
Comment 23 Joseph Pecoraro 2017-01-20 14:56:54 PST
Attempted Windows Build Fix:
<https://trac.webkit.org/changeset/210991>

I'll watch the bots.
Comment 24 Joseph Pecoraro 2017-01-20 15:44:04 PST
Alex landed a better fix:
<https://trac.webkit.org/changeset/210993>
Comment 25 Antoine Quint 2017-01-21 12:21:02 PST
Thanks Alex!