Bug 167115

Summary: Web Animations should be off by default and enabled as an experimental feature
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, dino, esprehn+autocc, joepeck, kondapallykalyan, rniwa, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Patch
none
Patch
none
Patch for landing
none
[PATCH] For Bots none

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!