Bug 151688

Summary:

[web-animations] Add AnimationTimeline, DocumentTimeline and add extensions to Document interface

Product:

WebKit

Reporter:

Nikos Andronikos <nikos.andronikos>

Component:

Animations

Assignee:

Nikos Andronikos <nikos.andronikos>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andrew, commit-queue, dino, graouts

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

151739

Bug Blocks:

122912

Attachments:

Description	Flags
Test Patch - trying to diagnose script issues.	none
Patch	none
Patch Checking build against fix (bug 151739) made for errors generated by bug 151682	none
Patch	none
Patch - Checking build against fix (bug 151739) and fixing issues with GTK and Win build	none
Patch	none
Patch	commit-queue: commit-queue-

Nikos Andronikos

Reported 2015-11-30 17:20:17 PST

This patch: - Adds DocumentTimeline interface and class implementation - Implements the DocumentAnimation extension to the Document Interface that contains a default DocumentTimeline - Add AnimationTimeline interface stub (i.e. without getAnimations and currentTime) - Adds AnimationTimeline class implementation for AnimationTimeline interface stub - Adds ObjectiveC and Javascript bindings for the above classes and interfaces - Enables the WEB_ANIMATIONS compiler switch Note: This patch adds classes but no real functionality yet. Tests will be added as class functionality is fleshed out. This patch supersedes https://bugs.webkit.org/show_bug.cgi?id=123225 (which was simply to implement a stub DocumentTimeline class and reference it from Document)

Attachments
Test Patch - trying to diagnose script issues. (256.14 KB, patch) 2015-11-30 18:00 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch (86.54 KB, patch) 2015-11-30 18:29 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch Checking build against fix (bug 151739) made for errors generated by bug 151682 (93.54 KB, patch) 2015-12-01 21:49 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch (93.85 KB, patch) 2015-12-01 23:11 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch - Checking build against fix (bug 151739) and fixing issues with GTK and Win build (97.90 KB, patch) 2015-12-06 22:35 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch (95.22 KB, patch) 2016-02-08 19:35 PST, Nikos Andronikos	no flags	Details Formatted Diff Diff
Patch (77.61 KB, patch) 2016-02-16 22:12 PST, Nikos Andronikos	commit-queue: commit-queue-	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Nikos Andronikos

Comment 1 2015-11-30 18:00:59 PST

Created attachment 266311 [details] Test Patch - trying to diagnose script issues.

Nikos Andronikos

Comment 2 2015-11-30 18:29:51 PST

Created attachment 266317 [details] Patch

Nikos Andronikos

Comment 3 2015-12-01 21:49:39 PST

Created attachment 266429 [details] Patch Checking build against fix (bug 151739) made for errors generated by bug 151682

Nikos Andronikos

Comment 4 2015-12-01 23:11:07 PST

Created attachment 266431 [details] Patch

Nikos Andronikos

Comment 5 2015-12-01 23:12:09 PST

Comment on attachment 266431 [details] Patch Checking build against fix (bug 151739) made for errors generated by bug 151682 and fix up issue with missing file in CMake builds

Nikos Andronikos

Comment 6 2015-12-06 22:35:38 PST

Created attachment 266751 [details] Patch - Checking build against fix (bug 151739) and fixing issues with GTK and Win build

Nikos Andronikos

Comment 7 2016-02-08 19:35:21 PST

Created attachment 270909 [details] Patch

Dean Jackson

Comment 8 2016-02-11 16:27:57 PST

Comment on attachment 270909 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270909&action=review > Source/WebCore/PlatformMac.cmake:175 > + bindings/objc/DOMWebAnimations.mm Don't worry about this. We don't need ObjC bindings. > Source/WebCore/animation/AnimationTimeline.cpp:2 > + * Copyright (C) Canon Inc. 2015 2016 > Source/WebCore/animation/AnimationTimeline.cpp:33 > + */ > +#include "config.h" > + > +#if ENABLE(WEB_ANIMATIONS) > +#include "AnimationTimeline.h" > + > +#include "DocumentTimeline.h" Nits: - needs blank line after license - might as well put the #if ENABLE first - blank line after #if ENABLE > Source/WebCore/animation/AnimationTimeline.cpp:40 > + // NOTE: We only have one type of subclass at the moment Nit: needs . > Source/WebCore/animation/AnimationTimeline.h:2 > + * Copyright (C) Canon Inc. 2015 2016 (all of them are like this) > Source/WebCore/animation/DocumentAnimation.cpp:32 > + */ > +#include "config.h" > +#include "DocumentAnimation.h" > + > +#if ENABLE(WEB_ANIMATIONS) > + Nits: same as before. > Source/WebCore/animation/DocumentAnimation.cpp:48 > + DocumentAnimation* documentAnimation = DocumentAnimation::from(&document); Use auto. > Source/WebCore/animation/DocumentAnimation.cpp:50 > + documentAnimation->m_defaultTimeline = DocumentTimeline::create(0); nullptr rather than 0 > Source/WebCore/bindings/objc/DOMWebAnimations.h:1 > +/* I don't think we need to bother with ObjC bindings for this. Just the JS. > Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:104 > + "NodeIterator" => 1, "TreeWalker" => 1, "AbstractView" => 1, "Blob" => 1, "AnimationTimeline" => 1); ... so no need for this. > WebKitLibraries/win/tools/vsprops/FeatureDefines.props:1 > -ï»¿<?xml version="1.0" encoding="utf-8"?> > +<?xml version="1.0" encoding="utf-8"?> I'm a bit worried about this. Was there a reason we had the BOM? > WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.props:1 > -ï»¿<?xml version="1.0" encoding="utf-8"?> > +<?xml version="1.0" encoding="utf-8"?> Same.

Nikos Andronikos

Comment 9 2016-02-16 22:12:02 PST

Created attachment 271537 [details] Patch

Nikos Andronikos

Comment 10 2016-02-16 22:14:41 PST

(In reply to comment #8) > Comment on attachment 270909 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270909&action=review > > > Source/WebCore/PlatformMac.cmake:175 > > + bindings/objc/DOMWebAnimations.mm > > Don't worry about this. We don't need ObjC bindings. Okay, done. > > > Source/WebCore/animation/AnimationTimeline.cpp:2 > > + * Copyright (C) Canon Inc. 2015 > > 2016 Done. > > > Source/WebCore/animation/AnimationTimeline.cpp:33 > > + */ > > +#include "config.h" > > + > > +#if ENABLE(WEB_ANIMATIONS) > > +#include "AnimationTimeline.h" > > + > > +#include "DocumentTimeline.h" > > Nits: > > - needs blank line after license > - might as well put the #if ENABLE first > - blank line after #if ENABLE config.h needs to be before the #define. Otherwise, done. > > > Source/WebCore/animation/AnimationTimeline.cpp:40 > > + // NOTE: We only have one type of subclass at the moment > > Nit: needs . > > > Source/WebCore/animation/AnimationTimeline.h:2 > > + * Copyright (C) Canon Inc. 2015 > > 2016 (all of them are like this) > > > Source/WebCore/animation/DocumentAnimation.cpp:32 > > + */ > > +#include "config.h" > > +#include "DocumentAnimation.h" > > + > > +#if ENABLE(WEB_ANIMATIONS) > > + > > Nits: same as before. > > > Source/WebCore/animation/DocumentAnimation.cpp:48 > > + DocumentAnimation* documentAnimation = DocumentAnimation::from(&document); > > Use auto. > > > Source/WebCore/animation/DocumentAnimation.cpp:50 > > + documentAnimation->m_defaultTimeline = DocumentTimeline::create(0); > > nullptr rather than 0 This takes a double not a ptr. > > > Source/WebCore/bindings/objc/DOMWebAnimations.h:1 > > +/* > > I don't think we need to bother with ObjC bindings for this. Just the JS. > > > Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:104 > > + "NodeIterator" => 1, "TreeWalker" => 1, "AbstractView" => 1, "Blob" => 1, "AnimationTimeline" => 1); > > ... so no need for this. > > > WebKitLibraries/win/tools/vsprops/FeatureDefines.props:1 > > -ï»¿<?xml version="1.0" encoding="utf-8"?> > > +<?xml version="1.0" encoding="utf-8"?> > > I'm a bit worried about this. Was there a reason we had the BOM? > > > WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.props:1 > > -ï»¿<?xml version="1.0" encoding="utf-8"?> > > +<?xml version="1.0" encoding="utf-8"?> > > Same. Fixed.

WebKit Commit Bot

Comment 11 2016-02-24 15:08:48 PST

Comment on attachment 271537 [details] Patch Rejecting attachment 271537 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 271537, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 197051 = c7ebd6663a86cc3e2258730f9e46406d8453e32e r197052 = 87f285b2c68e4e99a87821bf7664f9fb67535a62 r197053 = e42d611c93de15481d12d5dd2fb7a10306c28255 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/879063

Dean Jackson

Comment 12 2016-02-24 17:12:23 PST

Committed r197058: <http://trac.webkit.org/changeset/197058>

Rawinder Singh

Comment 13 2016-04-04 23:19:21 PDT

*** Bug 123225 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.