Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

This patch refactors the way we manage CSS Transition animations to be
based on PropertyHandles instead of CSSPropertyIDs. This is to support
future work in enabling CSS Transitions on custom properties.

CSSPropertyID is unable to distinguish between different custom
properties as they all use the value CSSPropertyVariable while
PropertyHandle stores the AtomicString name of the custom property.

BUG=671904
Review-Url: https://codereview.chromium.org/2719083005
Cr-Commit-Position: refs/heads/master@{#454477}
Committed: https://chromium.googlesource.com/chromium/src/+/08bbd3b3b0a9e991dd042084dd69a11e24d5696d

[alancutter (OOO until 2018), 2017-03-01 02:19:11]

Description was changed from

==========
Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

This patch refactors the way we manage CSS Transition animations to be
based on PropertyHandles instead of CSSPropertyIDs. This is to support
future work in enabling CSS Transitions on custom properties.
CSSPropertyID is unable to distinguish between different custom
properties as they all use the value CSSPropertyVariable while
PropertyHandle stores the AtomicString name of the custom property.

BUG=671904
==========

to

==========
Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

This patch refactors the way we manage CSS Transition animations to be
based on PropertyHandles instead of CSSPropertyIDs. This is to support
future work in enabling CSS Transitions on custom properties.

CSSPropertyID is unable to distinguish between different custom
properties as they all use the value CSSPropertyVariable while
PropertyHandle stores the AtomicString name of the custom property.

BUG=671904
==========

[alancutter (OOO until 2018), 2017-03-01 02:19:22]

alancutter@chromium.org changed reviewers:
+ ericwilligers@chromium.org

[Eric Willigers, 2017-03-01 02:33:35]

lgtm

https://codereview.chromium.org/2719083005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/2719083005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:594: if
(property.isCSSCustomProperty()) {
braces optional

[alancutter (OOO until 2018), 2017-03-01 02:36:25]

Will wait for dependant patches to be ready before landing in case I missed
anything.

https://codereview.chromium.org/2719083005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/2719083005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:594: if
(property.isCSSCustomProperty()) {
On 2017/03/01 at 02:33:35, Eric Willigers wrote:
> braces optional

I prefer always braces under 2 space indentation.

[alancutter (OOO until 2018), 2017-03-02 05:46:33]

The CQ bit was checked by alancutter@chromium.org

[commit-bot: I haz the power, 2017-03-02 05:47:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 05:50:29]

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

[commit-bot: I haz the power, 2017-03-02 05:50:30]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[alancutter (OOO until 2018), 2017-03-02 06:25:33]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2017-03-02 06:25:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
ericwilligers@chromium.org
Link to the patchset: https://codereview.chromium.org/2719083005/#ps20001
(title: "Rebased")

[commit-bot: I haz the power, 2017-03-02 06:26:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 08:07:22]

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

[commit-bot: I haz the power, 2017-03-02 08:07:26]

Failed to apply patch for
third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h:162
  error: third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h:
patch does not apply

Patch:       third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
Index: third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
diff --git a/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
b/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
index
8b2de64d7103ad26023ebd037c66668dbfb27764..220afe9121abda818e25055a353dbc624b43f38d
100644
--- a/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
+++ b/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
@@ -162,26 +162,30 @@ class CSSAnimationUpdate final {
     m_updatedCompositorKeyframes.push_back(animation);
   }
 
-  void startTransition(CSSPropertyID id,
+  void startTransition(const PropertyHandle& property,
                        RefPtr<AnimatableValue> from,
                        RefPtr<AnimatableValue> to,
                        PassRefPtr<AnimatableValue> reversingAdjustedStartValue,
                        double reversingShorteningFactor,
                        const InertEffect& effect) {
     NewTransition newTransition;
-    newTransition.id = id;
+    newTransition.property = property;
     newTransition.from = std::move(from);
     newTransition.to = std::move(to);
     newTransition.reversingAdjustedStartValue = reversingAdjustedStartValue;
     newTransition.reversingShorteningFactor = reversingShorteningFactor;
     newTransition.effect = &effect;
-    m_newTransitions.set(id, newTransition);
+    m_newTransitions.set(property, newTransition);
   }
-  bool isCancelledTransition(CSSPropertyID id) const {
-    return m_cancelledTransitions.contains(id);
+  bool isCancelledTransition(const PropertyHandle& property) const {
+    return m_cancelledTransitions.contains(property);
+  }
+  void cancelTransition(const PropertyHandle& property) {
+    m_cancelledTransitions.insert(property);
+  }
+  void finishTransition(const PropertyHandle& property) {
+    m_finishedTransitions.insert(property);
   }
-  void cancelTransition(CSSPropertyID id) { m_cancelledTransitions.insert(id);
}
-  void finishTransition(CSSPropertyID id) { m_finishedTransitions.insert(id); }
 
   const HeapVector<NewCSSAnimation>& newAnimations() const {
     return m_newAnimations;
@@ -208,19 +212,19 @@ class CSSAnimationUpdate final {
    public:
     DEFINE_INLINE_TRACE() { visitor->trace(effect); }
 
-    CSSPropertyID id;
+    PropertyHandle property = HashTraits<blink::PropertyHandle>::emptyValue();
     RefPtr<AnimatableValue> from;
     RefPtr<AnimatableValue> to;
     RefPtr<AnimatableValue> reversingAdjustedStartValue;
     double reversingShorteningFactor;
     Member<const InertEffect> effect;
   };
-  using NewTransitionMap = HeapHashMap<CSSPropertyID, NewTransition>;
+  using NewTransitionMap = HeapHashMap<PropertyHandle, NewTransition>;
   const NewTransitionMap& newTransitions() const { return m_newTransitions; }
-  const HashSet<CSSPropertyID>& cancelledTransitions() const {
+  const HashSet<PropertyHandle>& cancelledTransitions() const {
     return m_cancelledTransitions;
   }
-  const HashSet<CSSPropertyID>& finishedTransitions() const {
+  const HashSet<PropertyHandle>& finishedTransitions() const {
     return m_finishedTransitions;
   }
 
@@ -274,8 +278,8 @@ class CSSAnimationUpdate final {
   HeapVector<Member<Animation>> m_updatedCompositorKeyframes;
 
   NewTransitionMap m_newTransitions;
-  HashSet<CSSPropertyID> m_cancelledTransitions;
-  HashSet<CSSPropertyID> m_finishedTransitions;
+  HashSet<PropertyHandle> m_cancelledTransitions;
+  HashSet<PropertyHandle> m_finishedTransitions;
 
   ActiveInterpolationsMap m_activeInterpolationsForAnimations;
   ActiveInterpolationsMap m_activeInterpolationsForTransitions;

[alancutter (OOO until 2018), 2017-03-03 00:15:55]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2017-03-03 00:16:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
ericwilligers@chromium.org
Link to the patchset: https://codereview.chromium.org/2719083005/#ps40001
(title: "Rebasedagainaaaa")

[commit-bot: I haz the power, 2017-03-03 00:16:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 02:46:05]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488500155608340,
"parent_rev": "3c8103f16188658d2dbf45d14a8a2a3a8467ba44", "commit_rev":
"08bbd3b3b0a9e991dd042084dd69a11e24d5696d"}

[commit-bot: I haz the power, 2017-03-03 02:46:53]

Description was changed from

==========
Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

This patch refactors the way we manage CSS Transition animations to be
based on PropertyHandles instead of CSSPropertyIDs. This is to support
future work in enabling CSS Transitions on custom properties.

CSSPropertyID is unable to distinguish between different custom
properties as they all use the value CSSPropertyVariable while
PropertyHandle stores the AtomicString name of the custom property.

BUG=671904
==========

to

==========
Use PropertyHandle instead of CSSPropertyID to identify CSS Transitions

This patch refactors the way we manage CSS Transition animations to be
based on PropertyHandles instead of CSSPropertyIDs. This is to support
future work in enabling CSS Transitions on custom properties.

CSSPropertyID is unable to distinguish between different custom
properties as they all use the value CSSPropertyVariable while
PropertyHandle stores the AtomicString name of the custom property.

BUG=671904

Review-Url: https://codereview.chromium.org/2719083005
Cr-Commit-Position: refs/heads/master@{#454477}
Committed:
https://chromium.googlesource.com/chromium/src/+/08bbd3b3b0a9e991dd042084dd69...
==========

[commit-bot: I haz the power, 2017-03-03 02:47:01]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/08bbd3b3b0a9e991dd042084dd69...