Web Animations API: Bindings for TimedItem.specified with readonly attributes

Web Animations API: Bindings for TimedItem.specified with readonly attributes

This CL exposes TimedItem.specified (experimental), and getters for each of its attributes.

Follow up patch will add setters.

Follow up patch will refactor support for duration if it is possible to define an attribute with a union type in the current WebIDL implementation.

NOTRY=true
BUG=334927, 257235
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167166

[rjwright, 2014-02-03 23:58:24]

ptal

[rjwright, 2014-02-12 07:05:36]

ptal :)

[dstockwell, 2014-02-12 08:02:27]

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.cpp (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.cpp:92: default:
do we need default?

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.cpp:95: return "normal";
ASSERT_NOT_REACHED

(same comments for fill above)

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.h (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.h:18: double delay();
Tim is about to or has just landed endDelay, should be a simple change to add
it.

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.idl (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.idl:7: ] interface TimedItemTiming {
interface Timing

[rjwright, 2014-02-13 01:04:49]

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.cpp (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.cpp:92: default:
On 2014/02/12 08:02:28, dstockwell wrote:
> do we need default?

Done.

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.cpp:95: return "normal";
On 2014/02/12 08:02:28, dstockwell wrote:
> ASSERT_NOT_REACHED
> 
> (same comments for fill above)

Done.

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.h (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.h:18: double delay();
On 2014/02/12 08:02:28, dstockwell wrote:
> Tim is about to or has just landed endDelay, should be a simple change to add
> it.

Done.

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.idl (right):

https://codereview.chromium.org/152853003/diff/490001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.idl:7: ] interface TimedItemTiming {
On 2014/02/12 08:02:28, dstockwell wrote:
> interface Timing

Done.

[dstockwell, 2014-02-13 01:34:50]

lgtm

https://codereview.chromium.org/152853003/diff/620001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.h (right):

https://codereview.chromium.org/152853003/diff/620001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.h:17: static PassRefPtr<TimedItemTiming>
create(TimedItem* parent);
The param and member are never null, can be a reference.

[rjwright, 2014-02-13 02:03:05]

https://codereview.chromium.org/152853003/diff/620001/Source/core/animation/T...
File Source/core/animation/TimedItemTiming.h (right):

https://codereview.chromium.org/152853003/diff/620001/Source/core/animation/T...
Source/core/animation/TimedItemTiming.h:17: static PassRefPtr<TimedItemTiming>
create(TimedItem* parent);
On 2014/02/13 01:34:50, dstockwell wrote:
> The param and member are never null, can be a reference.

Done.

[rjwright, 2014-02-13 02:03:14]

The CQ bit was checked by rjwright@chromium.org

[rjwright, 2014-02-13 02:15:16]

The CQ bit was unchecked by rjwright@chromium.org

[rjwright, 2014-02-13 02:21:43]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-13 02:21:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/810001

[dstockwell, 2014-02-13 02:25:43]

On 2014/02/13 01:34:50, dstockwell wrote:
>
https://codereview.chromium.org/152853003/diff/620001/Source/core/animation/T...
> Source/core/animation/TimedItemTiming.h:17: static PassRefPtr<TimedItemTiming>
> create(TimedItem* parent);
> The param and member are never null, can be a reference.

We discussed in person that Timing actually needs to keep the TimedItem alive,
so these became RefPtr.

[rjwright, 2014-02-13 02:35:00]

The CQ bit was unchecked by rjwright@chromium.org

[dstockwell, 2014-02-13 03:03:39]

+tkent for Source/platform

[abarth-chromium, 2014-02-13 03:09:37]

Source/platform LGTM modulo nits

https://codereview.chromium.org/152853003/diff/810001/Source/platform/animati...
File Source/platform/animation/TimingFunction.h (right):

https://codereview.chromium.org/152853003/diff/810001/Source/platform/animati...
Source/platform/animation/TimingFunction.h:55: virtual String toString() const {
return ""; }
Please move virtual functions out of line.

https://codereview.chromium.org/152853003/diff/810001/Source/platform/animati...
Source/platform/animation/TimingFunction.h:80: virtual String toString() const
OVERRIDE { return "linear"; }
ditto

[dstockwell, 2014-02-13 07:21:30]

The CQ bit was checked by dstockwell@chromium.org

[dstockwell, 2014-02-13 07:21:31]

The CQ bit was unchecked by dstockwell@chromium.org

[dstockwell, 2014-02-13 07:21:40]

lgtm

[commit-bot: I haz the power, 2014-02-13 07:21:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/910001

[commit-bot: I haz the power, 2014-02-13 07:21:58]

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

[commit-bot: I haz the power, 2014-02-13 07:21:58]

Failed to apply patch for Source/platform/animation/TimingFunction.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/animation/TimingFunction.h
  Hunk #3 FAILED at 77.
  Hunk #4 succeeded at 90 (offset -1 lines).
  Hunk #5 FAILED at 140.
  Hunk #6 FAILED at 210.
  Hunk #7 succeeded at 246 (offset -3 lines).
  Hunk #8 succeeded at 284 (offset -3 lines).
  3 out of 8 hunks FAILED -- saving rejects to file
Source/platform/animation/TimingFunction.h.rej

Patch:       Source/platform/animation/TimingFunction.h
Index: Source/platform/animation/TimingFunction.h
diff --git a/Source/platform/animation/TimingFunction.h
b/Source/platform/animation/TimingFunction.h
index
1187ea92f69b811496dbed32c149117ef8fe0f16..1289b2f2facdf305990ec9aa953ac59b6a73abd2
100644
--- a/Source/platform/animation/TimingFunction.h
+++ b/Source/platform/animation/TimingFunction.h
@@ -34,6 +34,8 @@
 #include "wtf/RefCounted.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/Vector.h"
+#include "wtf/text/StringBuilder.h"
+#include "wtf/text/WTFString.h"
 #include <algorithm>
 
 
@@ -50,9 +52,11 @@ public:
 
     Type type() const { return m_type; }
 
+    virtual String toString() const;
+
     // Evaluates the timing function at the given fraction. The accuracy
parameter provides a hint as to the required
     // accuracy and is not guaranteed.
-    virtual double evaluate(double fraction, double accuracy) const = 0;
+    virtual double evaluate(double fraction, double accuracy) const =0;
 
 protected:
     TimingFunction(Type type)
@@ -73,12 +77,9 @@ public:
 
     virtual ~LinearTimingFunction() { }
 
-    virtual double evaluate(double fraction, double) const OVERRIDE
-    {
-        ASSERT(RuntimeEnabledFeatures::webAnimationsCSSEnabled() || (fraction
>= 0 && fraction <= 1));
-        ASSERT_WITH_MESSAGE(!RuntimeEnabledFeatures::webAnimationsCSSEnabled()
|| (fraction >= 0 && fraction <= 1), "Web Animations not yet implemented: Timing
function behavior outside the range [0, 1] is not yet specified");
-        return fraction;
-    }
+    virtual String toString() const OVERRIDE;
+
+    virtual double evaluate(double fraction, double) const OVERRIDE;
 
 private:
     LinearTimingFunction()
@@ -87,7 +88,6 @@ private:
     }
 };
 
-
 // Forward declare so we can friend it below. Don't use in production code!
 class ChainedTimingFunctionTestHelper;
 
@@ -137,14 +137,9 @@ public:
 
     virtual ~CubicBezierTimingFunction() { }
 
-    virtual double evaluate(double fraction, double accuracy) const OVERRIDE
-    {
-        ASSERT(RuntimeEnabledFeatures::webAnimationsCSSEnabled() || (fraction
>= 0 && fraction <= 1));
-        ASSERT_WITH_MESSAGE(!RuntimeEnabledFeatures::webAnimationsCSSEnabled()
|| (fraction >= 0 && fraction <= 1), "Web Animations not yet implemented: Timing
function behavior outside the range [0, 1] is not yet specified");
-        if (!m_bezier)
-            m_bezier = adoptPtr(new UnitBezier(m_x1, m_y1, m_x2, m_y2));
-        return m_bezier->solve(fraction, accuracy);
-    }
+    virtual String toString() const OVERRIDE;
+
+    virtual double evaluate(double fraction, double accuracy) const OVERRIDE;
 
     double x1() const { return m_x1; }
     double y1() const { return m_y1; }
@@ -207,12 +202,9 @@ public:
 
     virtual ~StepsTimingFunction() { }
 
-    virtual double evaluate(double fraction, double) const OVERRIDE
-    {
-        ASSERT(RuntimeEnabledFeatures::webAnimationsCSSEnabled() || (fraction
>= 0 && fraction <= 1));
-        ASSERT_WITH_MESSAGE(!RuntimeEnabledFeatures::webAnimationsCSSEnabled()
|| (fraction >= 0 && fraction <= 1), "Web Animations not yet implemented: Timing
function behavior outside the range [0, 1] is not yet specified");
-        return std::min(1.0, (floor(m_steps * fraction) + m_stepAtStart) /
m_steps);
-    }
+    virtual String toString() const OVERRIDE;
+
+    virtual double evaluate(double fraction, double) const OVERRIDE;
 
     int numberOfSteps() const { return m_steps; }
     bool stepAtStart() const { return m_stepAtStart; }
@@ -246,18 +238,10 @@ public:
         ASSERT(upperBound > max);
         m_segments.append(Segment(max, upperBound, timingFunction));
     }
-    virtual double evaluate(double fraction, double accuracy) const OVERRIDE
-    {
-        ASSERT_WITH_MESSAGE(fraction >= 0 && fraction <= 1, "Web Animations not
yet implemented: Timing function behavior outside the range [0, 1] is not yet
specified");
-        ASSERT(!m_segments.isEmpty());
-        ASSERT(m_segments.last().max() == 1);
-        size_t i = 0;
-        const Segment* segment = &m_segments[i++];
-        while (fraction >= segment->max() && i < m_segments.size()) {
-            segment = &m_segments[i++];
-        }
-        return segment->evaluate(fraction, accuracy);
-    }
+
+    virtual String toString() const OVERRIDE;
+
+    virtual double evaluate(double fraction, double accuracy) const OVERRIDE;
 
 private:
     class Segment {
@@ -292,6 +276,7 @@ private:
         // Allow PrintTo/operator== of the segments. Can be removed once
         // ChainedTimingFunction has a public API for segments.
         friend class ChainedTimingFunctionTestHelper;
+        friend class ChainedTimingFunction;
     };
 
     ChainedTimingFunction()

[rjwright, 2014-02-13 07:41:49]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-13 07:42:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1040001

[rjwright, 2014-02-13 07:49:48]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-13 07:50:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1080001

[commit-bot: I haz the power, 2014-02-13 08:43:48]

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

[commit-bot: I haz the power, 2014-02-13 08:43:49]

Retried try job too often on linux_blink for step(s) webkit_tests,
webkit_unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[rjwright, 2014-02-13 09:49:37]

On 2014/02/13 08:43:49, I haz the power (commit-bot) wrote:
> Retried try job too often on linux_blink for step(s) webkit_tests,
> webkit_unit_tests
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

Unsure why this is failing to link. Locally, it works when I just run

`build/gyp_chromium`

but fails with

`GYP_GENERATORS=ninja GYP_DEFINES="profiling=1
release_extra_cflags=-fno-omit-frame-pointer disable_pie=1 use_aura=1"
build/gyp_chromium -D component=shared_library -D disable_nacl=1`

(Which are my usual gyp options. I don't actually understand what most of them
do though.)

Doesn't seem to have anything to do with Clang. Compiler hits TimingFunction.cpp
but link fails.

Help welcome.

[Mike Lawther (Google), 2014-02-13 23:03:00]

Nothing looks obviously wrong in the gypi files :( But I did notice the
return/break pattern which should be fixed, either in this patch or as a
followup.

https://codereview.chromium.org/152853003/diff/1080001/Source/platform/animat...
File Source/platform/animation/TimingFunction.cpp (right):

https://codereview.chromium.org/152853003/diff/1080001/Source/platform/animat...
Source/platform/animation/TimingFunction.cpp:31: break;
These breaks after returns aren't needed.

[rjwright1, 2014-02-13 23:35:40]

Yeah I fixed the weird switch cases. Ooops.

We've narrowed the build failure down to component=shared_library.


On Fri, Feb 14, 2014 at 10:03 AM, <mikelawther@chromium.org> wrote:

> Nothing looks obviously wrong in the gypi files :( But I did notice the
> return/break pattern which should be fixed, either in this patch or as a
> followup.
>
>
> https://codereview.chromium.org/152853003/diff/1080001/
> Source/platform/animation/TimingFunction.cpp
> File Source/platform/animation/TimingFunction.cpp (right):
>
> https://codereview.chromium.org/152853003/diff/1080001/
> Source/platform/animation/TimingFunction.cpp#newcode31
> Source/platform/animation/TimingFunction.cpp:31: break;
> These breaks after returns aren't needed.
>
> https://codereview.chromium.org/152853003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[rjwright, 2014-02-13 23:47:21]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-13 23:47:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1310001

[rjwright, 2014-02-14 00:49:38]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 00:49:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1440001

[rjwright, 2014-02-14 02:09:32]

The CQ bit was unchecked by rjwright@chromium.org

[rjwright, 2014-02-14 02:12:39]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 02:13:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1620001

[commit-bot: I haz the power, 2014-02-14 04:21:03]

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

[commit-bot: I haz the power, 2014-02-14 04:21:04]

Retried try job too often on linux_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[rjwright, 2014-02-14 04:47:52]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 04:48:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1740001

[rjwright, 2014-02-14 04:51:34]

The CQ bit was unchecked by rjwright@chromium.org

[rjwright, 2014-02-14 04:56:24]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 04:56:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1880001

[rjwright, 2014-02-14 05:19:59]

The CQ bit was unchecked by rjwright@chromium.org

[rjwright, 2014-02-14 05:31:00]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 05:31:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1770002

[commit-bot: I haz the power, 2014-02-14 08:44:33]

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

[commit-bot: I haz the power, 2014-02-14 08:44:35]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[rjwright, 2014-02-14 08:45:02]

The CQ bit was checked by rjwright@chromium.org

[commit-bot: I haz the power, 2014-02-14 08:45:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1770002

[dstockwell, 2014-02-14 09:05:49]

The CQ bit was unchecked by dstockwell@chromium.org

[dstockwell, 2014-02-14 09:06:16]

The CQ bit was checked by dstockwell@chromium.org

[commit-bot: I haz the power, 2014-02-14 09:06:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rjwright@chromium.org/152853003/1770002

[commit-bot: I haz the power, 2014-02-14 09:46:08]

Change committed as 167166