Attachment 358696 Details for Bug 193195 – Patch

[patch] Patch

bug-193195-20190109141330.patch (text/plain), 32.89 KB, created by Antoine Quint on 2019-01-09 05:13:32 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2019-01-09 05:13:32 PST

Size: 32.89 KB

patch

obsolete

>Subversion Revision: 239723
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 197ab07d51b0c419a05bbfa39f78af1fcd95bf51..28e68233c1335f8d14658cf0782ea8378f756226 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,70 @@
>+2019-01-09  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Audit Web Animations classes for memory reduction
>+        https://bugs.webkit.org/show_bug.cgi?id=193195
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The classes, enums and structs added to support Web Animations were not as memory-efficient as they could be. We now order
>+        members in a way that reduces padding, use Markable<T, Traits> instead of Optional<T> where applicable, declare enums as uint8_t
>+        and removed unnecessary members.
>+
>+        As a result, classes and structs have shrunk as follows:
>+
>+        WebAnimation: 256 > 216
>+        DeclarativeAnimation: 392 > 344
>+        CSSAnimation: 416 > 368
>+        CSSTransition: 440 > 392
>+        AnimationEffect: 88 > 72
>+        KeyframeEffect: 208 > 184
>+        AnimationPlaybackEvent: 104 > 88
>+        EffectTiming: 72 > 64
>+        ComputedEffectTiming: 136 > 112
>+        AnimationTimeline: 264 > 248
>+        DocumentTimeline: 496 > 464
>+        OptionalEffectTiming: 112 > 80
>+        BaseKeyframe: 32 > 24
>+        ParsedKeyframe: 80 > 72
>+        BaseComputedKeyframe: 40 > 32
>+
>+        * animation/AnimationEffect.h: Order members in decreasing size, except for m_fill and m_direction, which we put at the top to
>+        save 8 bytes (2 bytes of padding instead of 4 before m_animation and saving 6 bytes of padding at the end).
>+        * animation/AnimationPlaybackEvent.cpp:
>+        (WebCore::AnimationPlaybackEvent::AnimationPlaybackEvent):
>+        * animation/AnimationPlaybackEvent.h:
>+        * animation/AnimationPlaybackEventInit.h:
>+        * animation/AnimationTimeline.cpp:
>+        (WebCore::AnimationTimeline::AnimationTimeline):
>+        (WebCore::AnimationTimeline::updateCSSTransitionsForElement):
>+        * animation/AnimationTimeline.h: We remove the m_classType member and instead make isDocumentTimeline() virtual.
>+        (WebCore::AnimationTimeline::isDocumentTimeline const):
>+        (): Deleted.
>+        (WebCore::AnimationTimeline::classType const): Deleted.
>+        * animation/CompositeOperation.h:
>+        * animation/CompositeOperationOrAuto.h:
>+        * animation/ComputedEffectTiming.h:
>+        * animation/DeclarativeAnimation.cpp:
>+        (WebCore::DeclarativeAnimation::DeclarativeAnimation):
>+        (WebCore::DeclarativeAnimation::invalidateDOMEvents):
>+        * animation/DeclarativeAnimation.h: We keep m_wasPending and m_previousPhase at the top to save some padding at the end.
>+        * animation/DocumentTimeline.cpp:
>+        (WebCore::DocumentTimeline::DocumentTimeline):
>+        * animation/DocumentTimeline.h:
>+        * animation/EffectTiming.h:
>+        * animation/FillMode.h:
>+        * animation/IterationCompositeOperation.h:
>+        * animation/KeyframeEffect.cpp:
>+        (WebCore::computeMissingKeyframeOffsets):
>+        (WebCore::KeyframeEffect::create):
>+        (WebCore::KeyframeEffect::KeyframeEffect):
>+        * animation/KeyframeEffect.h:
>+        * animation/OptionalEffectTiming.h:
>+        * animation/PlaybackDirection.h:
>+        * animation/WebAnimation.h:
>+        * animation/WebAnimationUtilities.h:
>+        (WebCore::WebAnimationsMarkableDoubleTraits::isEmptyValue):
>+        (WebCore::WebAnimationsMarkableDoubleTraits::emptyValue):
>+
> 2018-12-19  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Compute animation effect timing properties in batch
>diff --git a/Source/WebCore/animation/AnimationEffect.h b/Source/WebCore/animation/AnimationEffect.h
>index b364c9350f50fc2fb11c81355434d491210508bb..82de74289b09b906e81330068b744234ffad4fad 100644
>--- a/Source/WebCore/animation/AnimationEffect.h
>+++ b/Source/WebCore/animation/AnimationEffect.h
>@@ -94,17 +94,20 @@ protected:
>     explicit AnimationEffect();
> 
> private:
>-    enum class ComputedDirection { Forwards, Reverse };
>+    enum class ComputedDirection : uint8_t { Forwards, Reverse };
> 
>-    Seconds m_delay { 0_s };
>-    Seconds m_endDelay { 0_s };
>     FillMode m_fill { FillMode::Auto };
>+    PlaybackDirection m_direction { PlaybackDirection::Normal };
>+
>+    WeakPtr<WebAnimation> m_animation;
>+    RefPtr<TimingFunction> m_timingFunction;
>+
>     double m_iterationStart { 0 };
>     double m_iterations { 1 };
>+
>+    Seconds m_delay { 0_s };
>+    Seconds m_endDelay { 0_s };
>     Seconds m_iterationDuration { 0_s };
>-    PlaybackDirection m_direction { PlaybackDirection::Normal };
>-    RefPtr<TimingFunction> m_timingFunction;
>-    WeakPtr<WebAnimation> m_animation;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/AnimationPlaybackEvent.cpp b/Source/WebCore/animation/AnimationPlaybackEvent.cpp
>index 8cfa54f23bf5764ee32b58e7625b367529d4e8d7..3ad1aedb20257b886196ad9a546a015f061f9ce3 100644
>--- a/Source/WebCore/animation/AnimationPlaybackEvent.cpp
>+++ b/Source/WebCore/animation/AnimationPlaybackEvent.cpp
>@@ -33,15 +33,15 @@ namespace WebCore {
> AnimationPlaybackEvent::AnimationPlaybackEvent(const AtomicString& type, const AnimationPlaybackEventInit& initializer, IsTrusted isTrusted)
>     : Event(type, initializer, isTrusted)
> {
>-    if (initializer.currentTime == WTF::nullopt)
>-        m_currentTime = WTF::nullopt;
>+    if (initializer.currentTime)
>+        m_currentTime = Seconds::fromMilliseconds(*initializer.currentTime);
>     else
>-        m_currentTime = Seconds::fromMilliseconds(initializer.currentTime.value());
>+        m_currentTime = WTF::nullopt;
> 
>-    if (initializer.timelineTime == WTF::nullopt)
>-        m_timelineTime = WTF::nullopt;
>+    if (initializer.timelineTime)
>+        m_timelineTime = Seconds::fromMilliseconds(*initializer.timelineTime);
>     else
>-        m_timelineTime = Seconds::fromMilliseconds(initializer.timelineTime.value());
>+        m_timelineTime = WTF::nullopt;
> }
> 
> AnimationPlaybackEvent::AnimationPlaybackEvent(const AtomicString& type, Optional<Seconds> currentTime, Optional<Seconds> timelineTime)
>diff --git a/Source/WebCore/animation/AnimationPlaybackEvent.h b/Source/WebCore/animation/AnimationPlaybackEvent.h
>index cb7fb885bcac239bc1e9ec7561845c4d59aa8851..ea88d14ed91fdfa62999c1e4c8cb53d7a3afd5c2 100644
>--- a/Source/WebCore/animation/AnimationPlaybackEvent.h
>+++ b/Source/WebCore/animation/AnimationPlaybackEvent.h
>@@ -25,8 +25,9 @@
> 
> #pragma once
> 
>-#include "Event.h"
> #include "AnimationPlaybackEventInit.h"
>+#include "Event.h"
>+#include <wtf/Markable.h>
> 
> namespace WebCore {
> 
>@@ -55,8 +56,8 @@ private:
>     AnimationPlaybackEvent(const AtomicString&, Optional<Seconds>, Optional<Seconds>);
>     AnimationPlaybackEvent(const AtomicString&, const AnimationPlaybackEventInit&, IsTrusted);
> 
>-    Optional<Seconds> m_currentTime;
>-    Optional<Seconds> m_timelineTime;
>+    Markable<Seconds, Seconds::MarkableTraits> m_currentTime;
>+    Markable<Seconds, Seconds::MarkableTraits> m_timelineTime;
> };
> 
> }
>diff --git a/Source/WebCore/animation/AnimationPlaybackEventInit.h b/Source/WebCore/animation/AnimationPlaybackEventInit.h
>index dc1e788f01c008c56becbec201613d10394123ab..ef714c86cfcd66ecc3c82f2787eca8b077fdd4e2 100644
>--- a/Source/WebCore/animation/AnimationPlaybackEventInit.h
>+++ b/Source/WebCore/animation/AnimationPlaybackEventInit.h
>@@ -26,13 +26,13 @@
> #pragma once
> 
> #include "EventInit.h"
>-#include <wtf/Optional.h>
>+#include "WebAnimationUtilities.h"
> 
> namespace WebCore {
> 
> struct AnimationPlaybackEventInit : EventInit {
>-    Optional<double> currentTime;
>-    Optional<double> timelineTime;
>+    MarkableDouble currentTime;
>+    MarkableDouble timelineTime;
> };
> 
> }
>diff --git a/Source/WebCore/animation/AnimationTimeline.cpp b/Source/WebCore/animation/AnimationTimeline.cpp
>index f550fc9282109238631977173b6dc1807666541d..faad34c4acb29cffd1ad4ebe67a51c824c2ae6bf 100644
>--- a/Source/WebCore/animation/AnimationTimeline.cpp
>+++ b/Source/WebCore/animation/AnimationTimeline.cpp
>@@ -46,8 +46,7 @@
> 
> namespace WebCore {
> 
>-AnimationTimeline::AnimationTimeline(ClassType classType)
>-    : m_classType(classType)
>+AnimationTimeline::AnimationTimeline()
> {
> }
> 
>@@ -459,8 +458,10 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
>                 //   - end value is the value of the property in the after-change style
>                 auto& reversingAdjustedStartStyle = previouslyRunningTransition->targetStyle();
>                 double transformedProgress = 1;
>-                if (auto* effect = previouslyRunningTransition->effect())
>-                    transformedProgress = effect->getComputedTiming().progress.valueOr(transformedProgress);
>+                if (auto* effect = previouslyRunningTransition->effect()) {
>+                    if (auto computedTimingProgress = effect->getComputedTiming().progress)
>+                        transformedProgress = *computedTimingProgress;
>+                }
>                 auto reversingShorteningFactor = std::max(std::min(((transformedProgress * previouslyRunningTransition->reversingShorteningFactor()) + (1 - previouslyRunningTransition->reversingShorteningFactor())), 1.0), 0.0);
>                 auto delay = matchingBackingAnimation->delay() < 0 ? Seconds(matchingBackingAnimation->delay()) * reversingShorteningFactor : Seconds(matchingBackingAnimation->delay());
>                 auto duration = Seconds(matchingBackingAnimation->duration()) * reversingShorteningFactor;
>diff --git a/Source/WebCore/animation/AnimationTimeline.h b/Source/WebCore/animation/AnimationTimeline.h
>index 583e66620c5d5fda457d8316587f6113ec1bb6b4..0f318bf2ffda02fe8cb3eda8c90ef53930a147c8 100644
>--- a/Source/WebCore/animation/AnimationTimeline.h
>+++ b/Source/WebCore/animation/AnimationTimeline.h
>@@ -33,6 +33,7 @@
> #include <wtf/Forward.h>
> #include <wtf/HashMap.h>
> #include <wtf/ListHashSet.h>
>+#include <wtf/Markable.h>
> #include <wtf/Optional.h>
> #include <wtf/Ref.h>
> #include <wtf/RefCounted.h>
>@@ -47,7 +48,7 @@ class Element;
> 
> class AnimationTimeline : public RefCounted<AnimationTimeline> {
> public:
>-    bool isDocumentTimeline() const { return m_classType == DocumentTimelineClass; }
>+    virtual bool isDocumentTimeline() const { return false; }
> 
>     void forgetAnimation(WebAnimation*);
>     virtual void animationTimingDidChange(WebAnimation&);
>@@ -56,7 +57,7 @@ public:
>     Optional<double> bindingsCurrentTime();
>     virtual Optional<Seconds> currentTime() { return m_currentTime; }
> 
>-    enum class Ordering { Sorted, Unsorted };
>+    enum class Ordering : uint8_t { Sorted, Unsorted };
>     Vector<RefPtr<WebAnimation>> animationsForElement(Element&, Ordering ordering = Ordering::Unsorted) const;
>     void elementWasRemoved(Element&);
>     void removeAnimationsForElement(Element&);
>@@ -74,13 +75,7 @@ public:
>     virtual ~AnimationTimeline();
> 
> protected:
>-    enum ClassType {
>-        DocumentTimelineClass
>-    };
>-
>-    ClassType classType() const { return m_classType; }
>-
>-    explicit AnimationTimeline(ClassType);
>+    explicit AnimationTimeline();
> 
>     ListHashSet<WebAnimation*> m_allAnimations;
>     ListHashSet<RefPtr<WebAnimation>> m_animations;
>@@ -91,13 +86,13 @@ private:
>     PropertyToTransitionMap& ensureRunningTransitionsByProperty(Element&);
>     void cancelDeclarativeAnimation(DeclarativeAnimation&);
> 
>-    ClassType m_classType;
>-    Optional<Seconds> m_currentTime;
>     ElementToAnimationsMap m_elementToAnimationsMap;
>     ElementToAnimationsMap m_elementToCSSAnimationsMap;
>     ElementToAnimationsMap m_elementToCSSTransitionsMap;
>     HashMap<Element*, HashMap<String, RefPtr<CSSAnimation>>> m_elementToCSSAnimationByName;
>     HashMap<Element*, PropertyToTransitionMap> m_elementToRunningCSSTransitionByCSSPropertyID;
>+
>+    Markable<Seconds, Seconds::MarkableTraits> m_currentTime;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/CompositeOperation.h b/Source/WebCore/animation/CompositeOperation.h
>index 209a5779306ceb639c9009ee6d589ab41eb9cd35..0f75e7415f3304b3d9aae8db4052df9e93ae436d 100644
>--- a/Source/WebCore/animation/CompositeOperation.h
>+++ b/Source/WebCore/animation/CompositeOperation.h
>@@ -27,6 +27,6 @@
> 
> namespace WebCore {
> 
>-enum class CompositeOperation { Replace, Add, Accumulate };
>+enum class CompositeOperation : uint8_t { Replace, Add, Accumulate };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/CompositeOperationOrAuto.h b/Source/WebCore/animation/CompositeOperationOrAuto.h
>index a27de4e3113b1cf65e0a91fe1f4513ad4ca26155..0279ca7fd8eed017a88b065241b3ae6dc4a8d5ea 100644
>--- a/Source/WebCore/animation/CompositeOperationOrAuto.h
>+++ b/Source/WebCore/animation/CompositeOperationOrAuto.h
>@@ -27,6 +27,6 @@
> 
> namespace WebCore {
> 
>-enum class CompositeOperationOrAuto { Replace, Add, Accumulate, Auto };
>+enum class CompositeOperationOrAuto : uint8_t { Replace, Add, Accumulate, Auto };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/ComputedEffectTiming.h b/Source/WebCore/animation/ComputedEffectTiming.h
>index a909a2c4f1d6ac0ae7f6fb2c6054d7bb063739d2..57aaba1ea5385006f566643f0e12184d744cf8a3 100644
>--- a/Source/WebCore/animation/ComputedEffectTiming.h
>+++ b/Source/WebCore/animation/ComputedEffectTiming.h
>@@ -27,18 +27,18 @@
> 
> #include "AnimationEffectPhase.h"
> #include "EffectTiming.h"
>-#include <wtf/Optional.h>
>+#include "WebAnimationUtilities.h"
> 
> namespace WebCore {
> 
> struct ComputedEffectTiming : EffectTiming {
>     AnimationEffectPhase phase { AnimationEffectPhase::Idle };
>+    MarkableDouble localTime;
>+    MarkableDouble simpleIterationProgress;
>+    MarkableDouble progress;
>+    MarkableDouble currentIteration;
>     double endTime;
>     double activeDuration;
>-    Optional<double> localTime;
>-    Optional<double> simpleIterationProgress;
>-    Optional<double> progress;
>-    Optional<double> currentIteration;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/DeclarativeAnimation.cpp b/Source/WebCore/animation/DeclarativeAnimation.cpp
>index 417acefa1038aea99df80a9b5a34c689c5ecb1bf..f76191ffe50644a997bfe4bbc27b6611e8ca50cb 100644
>--- a/Source/WebCore/animation/DeclarativeAnimation.cpp
>+++ b/Source/WebCore/animation/DeclarativeAnimation.cpp
>@@ -38,9 +38,9 @@ namespace WebCore {
> 
> DeclarativeAnimation::DeclarativeAnimation(Element& owningElement, const Animation& backingAnimation)
>     : WebAnimation(owningElement.document())
>+    , m_eventQueue(owningElement)
>     , m_owningElement(&owningElement)
>     , m_backingAnimation(const_cast<Animation&>(backingAnimation))
>-    , m_eventQueue(owningElement)
> {
> }
> 
>@@ -236,7 +236,7 @@ void DeclarativeAnimation::invalidateDOMEvents(Seconds elapsedTime)
>     if (isPending && m_wasPending)
>         return;
> 
>-    double iteration;
>+    double iteration = 0;
>     AnimationEffectPhase currentPhase;
>     Seconds intervalStart;
>     Seconds intervalEnd;
>@@ -244,7 +244,8 @@ void DeclarativeAnimation::invalidateDOMEvents(Seconds elapsedTime)
>     auto* animationEffect = effect();
>     if (animationEffect) {
>         auto timing = animationEffect->getComputedTiming();
>-        iteration = timing.currentIteration.valueOr(0);
>+        if (auto computedIteration = timing.currentIteration)
>+            iteration = *computedIteration;
>         currentPhase = timing.phase;
>         intervalStart = std::max(0_s, Seconds::fromMilliseconds(std::min(-timing.delay, timing.activeDuration)));
>         intervalEnd = std::max(0_s, Seconds::fromMilliseconds(std::min(timing.endTime - timing.delay, timing.activeDuration)));
>diff --git a/Source/WebCore/animation/DeclarativeAnimation.h b/Source/WebCore/animation/DeclarativeAnimation.h
>index 2105750b010d6fa35ed8bdaedd50e4db2e70a43c..5d96d2698fe84057ee359d71df44980a9a06f057 100644
>--- a/Source/WebCore/animation/DeclarativeAnimation.h
>+++ b/Source/WebCore/animation/DeclarativeAnimation.h
>@@ -84,12 +84,14 @@ private:
>     void resume() final;
>     void stop() final;
> 
>-    Element* m_owningElement;
>-    Ref<Animation> m_backingAnimation;
>     bool m_wasPending { false };
>     AnimationEffectPhase m_previousPhase { AnimationEffectPhase::Idle };
>-    double m_previousIteration;
>+
>     GenericEventQueue m_eventQueue;
>+
>+    Element* m_owningElement;
>+    Ref<Animation> m_backingAnimation;
>+    double m_previousIteration;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/DocumentTimeline.cpp b/Source/WebCore/animation/DocumentTimeline.cpp
>index fb445c1d868eb24c5c4fb920c5043f65f4f3df72..3018cbb2ccd2af8f77abb60bc6c239238cfaf4e2 100644
>--- a/Source/WebCore/animation/DocumentTimeline.cpp
>+++ b/Source/WebCore/animation/DocumentTimeline.cpp
>@@ -60,13 +60,13 @@ Ref<DocumentTimeline> DocumentTimeline::create(Document& document, DocumentTimel
> }
> 
> DocumentTimeline::DocumentTimeline(Document& document, Seconds originTime)
>-    : AnimationTimeline(DocumentTimelineClass)
>-    , m_document(&document)
>-    , m_originTime(originTime)
>-    , m_tickScheduleTimer(*this, &DocumentTimeline::scheduleAnimationResolutionIfNeeded)
>+    : AnimationTimeline()
> #if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>     , m_animationResolutionTimer(*this, &DocumentTimeline::animationResolutionTimerFired)
> #endif
>+    , m_tickScheduleTimer(*this, &DocumentTimeline::scheduleAnimationResolutionIfNeeded)
>+    , m_document(&document)
>+    , m_originTime(originTime)
> {
>     if (m_document && m_document->page() && !m_document->page()->isVisible())
>         suspendAnimations();
>diff --git a/Source/WebCore/animation/DocumentTimeline.h b/Source/WebCore/animation/DocumentTimeline.h
>index 1e880f516030bbf655b9132e06b470c572caefa8..7cc81c9af38f9e87baf69bf052599fc2df4c9c21 100644
>--- a/Source/WebCore/animation/DocumentTimeline.h
>+++ b/Source/WebCore/animation/DocumentTimeline.h
>@@ -29,6 +29,7 @@
> #include "DocumentTimelineOptions.h"
> #include "GenericTaskQueue.h"
> #include "Timer.h"
>+#include <wtf/Markable.h>
> #include <wtf/Ref.h>
> 
> namespace WebCore {
>@@ -43,6 +44,8 @@ public:
>     static Ref<DocumentTimeline> create(Document&, DocumentTimelineOptions&&);
>     ~DocumentTimeline();
> 
>+    bool isDocumentTimeline() const final { return true; }
>+
>     Vector<RefPtr<WebAnimation>> getAnimations() const;
> 
>     Document* document() const { return m_document.get(); }
>@@ -100,23 +103,23 @@ private:
>     void transitionDidComplete(RefPtr<CSSTransition>);
>     void scheduleNextTick();
> 
>+#if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>+    void animationResolutionTimerFired();
>+    Timer m_animationResolutionTimer;
>+#endif
>+
>+    Timer m_tickScheduleTimer;
>+    GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
>+    HashSet<RefPtr<WebAnimation>> m_acceleratedAnimationsPendingRunningStateChange;
>+    HashSet<Element*> m_elementsWithRunningAcceleratedAnimations;
>+    Vector<Ref<AnimationPlaybackEvent>> m_pendingAnimationEvents;
>     RefPtr<Document> m_document;
>+    Markable<Seconds, Seconds::MarkableTraits> m_cachedCurrentTime;
>     Seconds m_originTime;
>+    unsigned m_numberOfAnimationTimelineInvalidationsForTesting { 0 };
>     bool m_isSuspended { false };
>     bool m_waitingOnVMIdle { false };
>     bool m_isUpdatingAnimations { false };
>-    Optional<Seconds> m_cachedCurrentTime;
>-    GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
>-    HashSet<RefPtr<WebAnimation>> m_acceleratedAnimationsPendingRunningStateChange;
>-    Vector<Ref<AnimationPlaybackEvent>> m_pendingAnimationEvents;
>-    unsigned m_numberOfAnimationTimelineInvalidationsForTesting { 0 };
>-    HashSet<Element*> m_elementsWithRunningAcceleratedAnimations;
>-    Timer m_tickScheduleTimer;
>-
>-#if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    void animationResolutionTimerFired();
>-    Timer m_animationResolutionTimer;
>-#endif
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/EffectTiming.h b/Source/WebCore/animation/EffectTiming.h
>index 7d67d160a7e13be1c8f14c0d21192ae8a3cfd51b..f5adf4c7b87768e73d21a17301d1e34f4e82f390 100644
>--- a/Source/WebCore/animation/EffectTiming.h
>+++ b/Source/WebCore/animation/EffectTiming.h
>@@ -33,14 +33,14 @@
> namespace WebCore {
> 
> struct EffectTiming {
>+    Variant<double, String> duration { "auto" };
>     double delay { 0 };
>     double endDelay { 0 };
>-    FillMode fill { FillMode::Auto };
>     double iterationStart { 0 };
>     double iterations { 1 };
>-    Variant<double, String> duration { "auto" };
>-    PlaybackDirection direction { PlaybackDirection::Normal };
>     String easing { "linear" };
>+    FillMode fill { FillMode::Auto };
>+    PlaybackDirection direction { PlaybackDirection::Normal };
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/FillMode.h b/Source/WebCore/animation/FillMode.h
>index 6984af406a52495ebb8e65dd0688db2de1cf6912..c24d9f4100bbb1c4325ccd71e0c6e3fd460c84be 100644
>--- a/Source/WebCore/animation/FillMode.h
>+++ b/Source/WebCore/animation/FillMode.h
>@@ -25,8 +25,12 @@
> 
> #pragma once
> 
>+#include <wtf/Markable.h>
>+
> namespace WebCore {
> 
>-enum class FillMode { None, Forwards, Backwards, Both, Auto };
>+enum class FillMode : uint8_t { None, Forwards, Backwards, Both, Auto };
>+
>+typedef Markable<FillMode, EnumMarkableTraits<FillMode, 5>> OptionalFillMode;
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/IterationCompositeOperation.h b/Source/WebCore/animation/IterationCompositeOperation.h
>index ded5f4efa8726fb2ef05cef3a90e3591042ae9b2..32ed2347f120f8bb7e1058473379d0f633b89dc5 100644
>--- a/Source/WebCore/animation/IterationCompositeOperation.h
>+++ b/Source/WebCore/animation/IterationCompositeOperation.h
>@@ -27,6 +27,6 @@
> 
> namespace WebCore {
> 
>-enum class IterationCompositeOperation { Replace, Accumulate };
>+enum class IterationCompositeOperation : uint8_t { Replace, Accumulate };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
>index 7f27f14b048cb5afa091921f84a22d0a9793c0aa..90c6ad3c3c0d45829fe184b16a8d1a58913f514b 100644
>--- a/Source/WebCore/animation/KeyframeEffect.cpp
>+++ b/Source/WebCore/animation/KeyframeEffect.cpp
>@@ -115,8 +115,10 @@ static inline void computeMissingKeyframeOffsets(Vector<KeyframeEffect::ParsedKe
>     // In our implementation, we only set non-null values to avoid making computedOffset Optional<double>. Instead, we'll know
>     // that a keyframe hasn't had a computed offset by checking if it has a null offset and a 0 computedOffset, since the first
>     // keyframe will already have a 0 computedOffset.
>-    for (auto& keyframe : keyframes)
>-        keyframe.computedOffset = keyframe.offset.valueOr(0);
>+    for (auto& keyframe : keyframes) {
>+        auto computedOffset = keyframe.offset;
>+        keyframe.computedOffset = computedOffset ? *computedOffset : 0;
>+    }
> 
>     // 2. If keyframes contains more than one keyframe and the computed keyframe offset of the first keyframe in keyframes is null,
>     //    set the computed keyframe offset of the first keyframe to 0.
>@@ -472,17 +474,16 @@ ExceptionOr<Ref<KeyframeEffect>> KeyframeEffect::create(ExecState& state, Elemen
>         } else {
>             auto keyframeEffectOptions = WTF::get<KeyframeEffectOptions>(optionsValue);
>             timing = {
>+                keyframeEffectOptions.duration,
>+                keyframeEffectOptions.iterations,
>                 keyframeEffectOptions.delay,
>                 keyframeEffectOptions.endDelay,
>-                keyframeEffectOptions.fill,
>                 keyframeEffectOptions.iterationStart,
>-                keyframeEffectOptions.iterations,
>-                keyframeEffectOptions.duration,
>-                keyframeEffectOptions.direction,
>-                keyframeEffectOptions.easing
>+                keyframeEffectOptions.easing,
>+                keyframeEffectOptions.fill,
>+                keyframeEffectOptions.direction
>             };
>         }
>-
>         auto updateTimingResult = keyframeEffect->updateTiming(timing);
>         if (updateTimingResult.hasException())
>             return updateTimingResult.releaseException();
>@@ -509,7 +510,6 @@ Ref<KeyframeEffect> KeyframeEffect::create(const Element& target)
> 
> KeyframeEffect::KeyframeEffect(Element* target)
>     : m_target(target)
>-    , m_blendingKeyframes(emptyString())
> {
> }
> 
>diff --git a/Source/WebCore/animation/KeyframeEffect.h b/Source/WebCore/animation/KeyframeEffect.h
>index 4f64ca4eade092e82548c743df5d1b7aaeac8a74..2dffdb757e81d5b0c9f711ce5340e2c60a3c9ce5 100644
>--- a/Source/WebCore/animation/KeyframeEffect.h
>+++ b/Source/WebCore/animation/KeyframeEffect.h
>@@ -36,6 +36,7 @@
> #include "KeyframeList.h"
> #include "RenderStyle.h"
> #include "StyleProperties.h"
>+#include "WebAnimationUtilities.h"
> #include <wtf/Ref.h>
> 
> namespace WebCore {
>@@ -60,7 +61,7 @@ public:
>     };
> 
>     struct BaseKeyframe {
>-        Optional<double> offset;
>+        MarkableDouble offset;
>         String easing { "linear" };
>         CompositeOperationOrAuto composite { CompositeOperationOrAuto::Auto };
>     };
>@@ -76,7 +77,7 @@ public:
>     };
> 
>     struct ParsedKeyframe {
>-        Optional<double> offset;
>+        MarkableDouble offset;
>         double computedOffset;
>         CompositeOperationOrAuto composite { CompositeOperationOrAuto::Auto };
>         String easing;
>@@ -91,7 +92,7 @@ public:
>     };
> 
>     struct BaseComputedKeyframe {
>-        Optional<double> offset;
>+        MarkableDouble offset;
>         double computedOffset;
>         String easing { "linear" };
>         CompositeOperationOrAuto composite { CompositeOperationOrAuto::Auto };
>@@ -139,7 +140,7 @@ public:
> private:
>     KeyframeEffect(Element*);
> 
>-    enum class AcceleratedAction { Play, Pause, Seek, Stop };
>+    enum class AcceleratedAction : uint8_t { Play, Pause, Seek, Stop };
> 
>     void copyPropertiesFromSource(Ref<KeyframeEffect>&&);
>     ExceptionOr<void> processKeyframes(JSC::ExecState&, JSC::Strong<JSC::JSObject>&&);
>@@ -157,14 +158,20 @@ private:
>     void setBlendingKeyframes(KeyframeList&);
>     void checkForMatchingTransformFunctionLists();
>     void checkForMatchingFilterFunctionLists();
>+    void checkForMatchingColorFilterFunctionLists();
>+    bool checkForMatchingFilterFunctionLists(CSSPropertyID, const std::function<const FilterOperations& (const RenderStyle&)>&) const;
> #if ENABLE(FILTERS_LEVEL_2)
>     void checkForMatchingBackdropFilterFunctionLists();
> #endif
>-    void checkForMatchingColorFilterFunctionLists();
> 
>-    bool checkForMatchingFilterFunctionLists(CSSPropertyID, const std::function<const FilterOperations& (const RenderStyle&)>&) const;
>+    KeyframeList m_blendingKeyframes { emptyString() };
>+    Vector<ParsedKeyframe> m_parsedKeyframes;
>+    Vector<AcceleratedAction> m_pendingAcceleratedActions;
>+    RefPtr<Element> m_target;
> 
>     AcceleratedAction m_lastRecordedAcceleratedAction { AcceleratedAction::Stop };
>+    IterationCompositeOperation m_iterationCompositeOperation { IterationCompositeOperation::Replace };
>+    CompositeOperation m_compositeOperation { CompositeOperation::Replace };
>     bool m_shouldRunAccelerated { false };
>     bool m_needsForcedLayout { false };
>     bool m_triggersStackingContext { false };
>@@ -174,14 +181,6 @@ private:
>     bool m_backdropFilterFunctionListsMatch { false };
> #endif
>     bool m_colorFilterFunctionListsMatch { false };
>-
>-    RefPtr<Element> m_target;
>-    KeyframeList m_blendingKeyframes;
>-    Vector<ParsedKeyframe> m_parsedKeyframes;
>-    IterationCompositeOperation m_iterationCompositeOperation { IterationCompositeOperation::Replace };
>-    CompositeOperation m_compositeOperation { CompositeOperation::Replace };
>-
>-    Vector<AcceleratedAction> m_pendingAcceleratedActions;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/OptionalEffectTiming.h b/Source/WebCore/animation/OptionalEffectTiming.h
>index 6d911819c4e097b85e3dcf0b68d137eec3650f79..16632d9423d7b6048b782831398994ffea642124 100644
>--- a/Source/WebCore/animation/OptionalEffectTiming.h
>+++ b/Source/WebCore/animation/OptionalEffectTiming.h
>@@ -27,20 +27,21 @@
> 
> #include "FillMode.h"
> #include "PlaybackDirection.h"
>+#include "WebAnimationUtilities.h"
> #include <wtf/Variant.h>
> #include <wtf/text/WTFString.h>
> 
> namespace WebCore {
> 
> struct OptionalEffectTiming {
>-    Optional<double> delay;
>-    Optional<double> endDelay;
>-    Optional<FillMode> fill;
>-    Optional<double> iterationStart;
>-    Optional<double> iterations;
>     Optional<Variant<double, String>> duration;
>-    Optional<PlaybackDirection> direction;
>+    Optional<double> iterations; // This value cannot be a MarkableDouble since we need to check for a NaN value.
>+    MarkableDouble delay;
>+    MarkableDouble endDelay;
>+    MarkableDouble iterationStart;
>     String easing;
>+    OptionalFillMode fill;
>+    OptionalPlaybackDirection direction;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/PlaybackDirection.h b/Source/WebCore/animation/PlaybackDirection.h
>index 4ba9ad63ef7f917b36f2d99b1254579e6b197da4..6f91e780d23799a35c7ca23e2fc2a41050cb0535 100644
>--- a/Source/WebCore/animation/PlaybackDirection.h
>+++ b/Source/WebCore/animation/PlaybackDirection.h
>@@ -25,8 +25,12 @@
> 
> #pragma once
> 
>+#include <wtf/Markable.h>
>+
> namespace WebCore {
> 
>-enum class PlaybackDirection { Normal, Reverse, Alternate, AlternateReverse };
>+enum class PlaybackDirection : uint8_t { Normal, Reverse, Alternate, AlternateReverse };
>+
>+typedef Markable<PlaybackDirection, EnumMarkableTraits<PlaybackDirection, 4>> OptionalPlaybackDirection;
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/WebAnimation.h b/Source/WebCore/animation/WebAnimation.h
>index ae1b7abfaafe14a05a0fbc47550df3e72d5dfcc0..ae0e40f3958c62ee2ae9f803f39f01edd789b423 100644
>--- a/Source/WebCore/animation/WebAnimation.h
>+++ b/Source/WebCore/animation/WebAnimation.h
>@@ -29,7 +29,9 @@
> #include "DOMPromiseProxy.h"
> #include "EventTarget.h"
> #include "ExceptionOr.h"
>+#include "WebAnimationUtilities.h"
> #include <wtf/Forward.h>
>+#include <wtf/Markable.h>
> #include <wtf/Optional.h>
> #include <wtf/Ref.h>
> #include <wtf/RefCounted.h>
>@@ -68,11 +70,11 @@ public:
>     Optional<Seconds> currentTime() const;
>     ExceptionOr<void> setCurrentTime(Optional<Seconds>);
> 
>-    enum class Silently { Yes, No };
>+    enum class Silently : uint8_t { Yes, No };
>     double playbackRate() const { return m_playbackRate + 0; }
>     void setPlaybackRate(double);
> 
>-    enum class PlayState { Idle, Running, Paused, Finished };
>+    enum class PlayState : uint8_t { Idle, Running, Paused, Finished };
>     PlayState playState() const;
> 
>     bool pending() const { return hasPendingPauseTask() || hasPendingPlayTask(); }
>@@ -128,11 +130,11 @@ protected:
>     void stop() override;
> 
> private:
>-    enum class DidSeek { Yes, No };
>-    enum class SynchronouslyNotify { Yes, No };
>-    enum class RespectHoldTime { Yes, No };
>-    enum class AutoRewind { Yes, No };
>-    enum class TimeToRunPendingTask { NotScheduled, ASAP, WhenReady };
>+    enum class DidSeek : uint8_t { Yes, No };
>+    enum class SynchronouslyNotify : uint8_t { Yes, No };
>+    enum class RespectHoldTime : uint8_t { Yes, No };
>+    enum class AutoRewind : uint8_t { Yes, No };
>+    enum class TimeToRunPendingTask : uint8_t { NotScheduled, ASAP, WhenReady };
> 
>     void timingDidChange(DidSeek, SynchronouslyNotify);
>     void updateFinishedState(DidSeek, SynchronouslyNotify);
>@@ -158,22 +160,24 @@ private:
>     double effectivePlaybackRate() const;
>     void applyPendingPlaybackRate();
> 
>-    String m_id;
>     RefPtr<AnimationEffect> m_effect;
>     RefPtr<AnimationTimeline> m_timeline;
>-    Optional<Seconds> m_previousCurrentTime;
>-    Optional<Seconds> m_startTime;
>-    Optional<Seconds> m_holdTime;
>-    int m_suspendCount { 0 };
>+    UniqueRef<ReadyPromise> m_readyPromise;
>+    UniqueRef<FinishedPromise> m_finishedPromise;
>+    Markable<Seconds, Seconds::MarkableTraits> m_previousCurrentTime;
>+    Markable<Seconds, Seconds::MarkableTraits> m_startTime;
>+    Markable<Seconds, Seconds::MarkableTraits> m_holdTime;
>+    MarkableDouble m_pendingPlaybackRate;
>     double m_playbackRate { 1 };
>-    Optional<double> m_pendingPlaybackRate;
>+    String m_id;
>+
>+    int m_suspendCount { 0 };
>+
>     bool m_isStopped { false };
>     bool m_isSuspended { false };
>     bool m_finishNotificationStepsMicrotaskPending;
>     bool m_isRelevant;
>     bool m_shouldSkipUpdatingFinishedStateWhenResolving;
>-    UniqueRef<ReadyPromise> m_readyPromise;
>-    UniqueRef<FinishedPromise> m_finishedPromise;
>     TimeToRunPendingTask m_timeToRunPendingPlayTask { TimeToRunPendingTask::NotScheduled };
>     TimeToRunPendingTask m_timeToRunPendingPauseTask { TimeToRunPendingTask::NotScheduled };
> 
>diff --git a/Source/WebCore/animation/WebAnimationUtilities.h b/Source/WebCore/animation/WebAnimationUtilities.h
>index 97bdd3888a62cd3e104afebc2f7bc8ba0b44da4e..3421e2621d069efd08ab8b915443a4fcd82cac18 100644
>--- a/Source/WebCore/animation/WebAnimationUtilities.h
>+++ b/Source/WebCore/animation/WebAnimationUtilities.h
>@@ -25,6 +25,7 @@
> 
> #pragma once
> 
>+#include <wtf/Markable.h>
> #include <wtf/Seconds.h>
> 
> namespace WebCore {
>@@ -45,5 +46,19 @@ inline double secondsToWebAnimationsAPITime(const Seconds time)
> 
> const auto timeEpsilon = Seconds::fromMilliseconds(0.001);
> 
>+struct WebAnimationsMarkableDoubleTraits {
>+    static bool isEmptyValue(double value)
>+    {
>+        return std::isnan(value);
>+    }
>+
>+    static constexpr double emptyValue()
>+    {
>+        return std::numeric_limits<double>::quiet_NaN();
>+    }
>+};
>+
>+typedef Markable<double, WebAnimationsMarkableDoubleTraits> MarkableDouble;
>+
> } // namespace WebCore
>

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193195: 358494 | 358696