Web Animations API: Refactor IDL input conversion out of Animation

Source/core/animation/Timing.cpp

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "core/animation/Timing.h"
+
+#include "bindings/v8/Dictionary.h"
+#include "core/css/parser/BisonCSSParser.h"
+#include "core/css/resolver/CSSToStyleMap.h"
+
+namespace WebCore {
+
+

[Timothy Loh, 2014/02/28 00:55:24]

Extra blank line :<

[alancutter (OOO until 2018), 2014/03/03 05:09:45]

Done.

+void Timing::setStartDelay(double startDelay)
+{
+    if (std::isfinite(startDelay))
+        this->startDelay = startDelay;
+    else
+        this->startDelay = 0;
+}
+
+void Timing::setEndDelay(double endDelay)
+{
+    if (std::isfinite(endDelay))
+        this->endDelay = endDelay;
+    else
+        this->endDelay = 0;

[esprehn, 2014/02/28 00:57:16]

This is wrong, you shouldn't make everything public if you expect folks to use a
setter. You want endDelay() getter and m_endDelay

[alancutter (OOO until 2018), 2014/03/03 05:09:45]

Done, moved setters to a separate TimingInput class.

+}
+
+void Timing::setFillMode(const String& fillMode)
+{
+    if (fillMode == "none") {
+        this->fillMode = Timing::FillModeNone;
+    } else if (fillMode == "backwards") {
+        this->fillMode = Timing::FillModeBackwards;
+    } else if (fillMode == "both") {
+        this->fillMode = Timing::FillModeBoth;
+    } else if (fillMode == "forwards") {
+        this->fillMode = Timing::FillModeForwards;
+    } else {
+        this->fillMode = Timing::FillModeAuto;
+    }
+}
+
+void Timing::setIterationStart(double iterationStart)
+{
+    if (!std::isnan(iterationStart) && !std::isinf(iterationStart))

[Timothy Loh, 2014/02/28 00:55:24]

While you're here, std::isfinite instead of the two checks.

[alancutter (OOO until 2018), 2014/03/03 05:09:45]

Done.

+        this->iterationStart = std::max<double>(iterationStart, 0);
+    else
+        this->iterationStart = 0;
+}
+
+void Timing::setIterationCount(double iterationCount)
+{
+    if (!std::isnan(iterationCount))
+        this->iterationCount = std::max<double>(iterationCount, 0);
+    else
+        this->iterationCount = 1;
+}
+
+void Timing::setIterationDuration(double iterationDuration)
+{
+    if (!std::isnan(iterationDuration) && iterationDuration >= 0)
+        this->iterationDuration = iterationDuration;
+    else
+        this->iterationDuration = std::numeric_limits<double>::quiet_NaN();
+}
+
+void Timing::setPlaybackRate(double playbackRate)
+{
+    if (!std::isnan(playbackRate) && !std::isinf(playbackRate))

[Timothy Loh, 2014/02/28 00:55:24]

std::isfinite here too.

[alancutter (OOO until 2018), 2014/03/03 05:09:45]

Done.

+        this->playbackRate = playbackRate;
+    else
+        this->playbackRate = 1;
+}
+
+void Timing::setPlaybackDirection(const String& direction)
+{
+    if (direction == "reverse") {
+        this->direction = Timing::PlaybackDirectionReverse;
+    } else if (direction == "alternate") {
+        this->direction = Timing::PlaybackDirectionAlternate;
+    } else if (direction == "alternate-reverse") {
+        this->direction = Timing::PlaybackDirectionAlternateReverse;
+    } else {
+        this->direction = Timing::PlaybackDirectionNormal;
+    }
+}
+
+void Timing::setTimingFunction(const String& timingFunctionString)
+{
+    RefPtrWillBeRawPtr<CSSValue> timingFunctionValue = BisonCSSParser::parseAnimationTimingFunctionValue(timingFunctionString);
+    if (timingFunctionValue) {
+        RefPtr<TimingFunction> timingFunction = CSSToStyleMap::animationTimingFunction(timingFunctionValue.get(), false);
+        if (timingFunction) {
+            this->timingFunction = timingFunction;
+            return;
+        }
+    }
+    this->timingFunction = LinearTimingFunction::create();
+}
+
+Timing Timing::convertInput(const Dictionary& timingInputDictionary)

[esprehn, 2014/02/28 00:57:16]

This is a lot of copying of values. You copy a ton of member vars and a RefPtr?

[alancutter (OOO until 2018), 2014/03/03 05:09:45]

This function is copying data from V8 into Blink, I'm not aware of an
alternative.
This code is just being moved by this patch out of Animation.cpp, any proposed
changes to it would be for a separate patch.

+{
+    Timing result;
+
+    // FIXME: This method needs to be refactored to handle invalid
+    // null, NaN, Infinity values better.
+    // See: http://www.w3.org/TR/WebIDL/#es-double
+    double startDelay = 0;
+    timingInputDictionary.get("delay", startDelay);
+    result.setStartDelay(startDelay);
+
+    double endDelay = 0;
+    timingInputDictionary.get("endDelay", endDelay);
+    result.setEndDelay(endDelay);
+
+    String fillMode;
+    timingInputDictionary.get("fill", fillMode);
+    result.setFillMode(fillMode);
+
+    double iterationStart = 0;
+    timingInputDictionary.get("iterationStart", iterationStart);
+    result.setIterationStart(iterationStart);
+
+    double iterationCount = 1;
+    timingInputDictionary.get("iterations", iterationCount);
+    result.setIterationCount(iterationCount);
+
+    v8::Local<v8::Value> iterationDurationValue;
+    if (timingInputDictionary.get("duration", iterationDurationValue)) {
+        double iterationDuration = iterationDurationValue->NumberValue();
+        result.setIterationDuration(iterationDuration);
+    }
+
+    double playbackRate = 1;
+    timingInputDictionary.get("playbackRate", playbackRate);
+    result.setPlaybackRate(playbackRate);
+
+    String direction;
+    timingInputDictionary.get("direction", direction);
+    result.setPlaybackDirection(direction);
+
+    String timingFunctionString;
+    timingInputDictionary.get("easing", timingFunctionString);
+    result.setTimingFunction(timingFunctionString);
+
+    result.assertValid();
+
+    return result;
+}
+
+} // namespace WebCore