Web Animations: Add a fast path for LengthStyleIntepolation application

Web Animations: Add a fast path for LengthStyleIntepolation application

This change bypasses creating a CSSValue when applying length animations
where only pixels or percentage values are interpolating and the
LayoutStyle has a Length setter for the animated property.

This is a performance change, there should be no behavioural changes.
Avoiding creating a CSSValue at application time improves the
css_value_type_length_simple?api=web_animations benchmark by ~4%
bringing it to the same level as css_value_type_length_simple?api=css_animations.
Tested on a Nexus 7 device.

BUG=309981
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191521

[alancutter (OOO until 2018), 2015-03-06 01:51:34]

alancutter@chromium.org changed reviewers:
+ dstockwell@chromium.org, shans@chromium.org

[Eric Willigers, 2015-03-06 02:22:08]

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

[Eric Willigers, 2015-03-06 02:22:08]

lgtm

s/there should be behavioural changes./there should not be behavioural changes./

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:103: return
&LayoutStyle::setMotionPosition;
Will need rebasing. MotionPosition has been renamed to MotionOffset

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:223: 
Unneeded blank line

[dstockwell, 2015-03-06 02:32:07]

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:121: return nullptr;
When does this happen?

[dstockwell, 2015-03-06 02:34:57]

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:222:
StyleBuilder::applyProperty(m_id, state, fromInterpolableValue(*m_cachedValue,
m_range).get());
Is there a way that we could assert that both paths produce the same result? eg.
run both when assert is enabled and compare the computed values?

[alancutter (OOO until 2018), 2015-03-06 07:18:59]

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:103: return
&LayoutStyle::setMotionPosition;
On 2015/03/06 02:22:08, Eric Willigers wrote:
> Will need rebasing. MotionPosition has been renamed to MotionOffset

Done.

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:121: return nullptr;
On 2015/03/06 02:32:07, dstockwell wrote:
> When does this happen?

There are several properties handled by LengthStyleInterpolation that don't have
a corresponding LayoutStyle setter method that accepts const Length&, they
usually take unsigned or int instead. These properties don't get the fast path.

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:222:
StyleBuilder::applyProperty(m_id, state, fromInterpolableValue(*m_cachedValue,
m_range).get());
On 2015/03/06 02:34:57, dstockwell wrote:
> Is there a way that we could assert that both paths produce the same result?
eg.
> run both when assert is enabled and compare the computed values?

Done, though you might want to check if this ASSERT expression is acceptable.

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:223: 
On 2015/03/06 02:22:08, Eric Willigers wrote:
> Unneeded blank line

Done.

[dstockwell, 2015-03-09 02:39:25]

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:121: return nullptr;
On 2015/03/06 07:18:58, alancutter wrote:
> On 2015/03/06 02:32:07, dstockwell wrote:
> > When does this happen?
> 
> There are several properties handled by LengthStyleInterpolation that don't
have
> a corresponding LayoutStyle setter method that accepts const Length&, they
> usually take unsigned or int instead. These properties don't get the fast
path.

Could we add them to the list and then assert not reached?

https://codereview.chromium.org/982913003/diff/40001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/40001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:222:
RefPtrWillBeRawPtr<AnimatableValue> before, after;
I think this would be simpler with ENABLE(ASSERT) -- aren't these unused
otherwise?

[alancutter (OOO until 2018), 2015-03-09 03:09:00]

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/20001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:121: return nullptr;
On 2015/03/09 02:39:25, dstockwell wrote:
> On 2015/03/06 07:18:58, alancutter wrote:
> > On 2015/03/06 02:32:07, dstockwell wrote:
> > > When does this happen?
> > 
> > There are several properties handled by LengthStyleInterpolation that don't
> have
> > a corresponding LayoutStyle setter method that accepts const Length&, they
> > usually take unsigned or int instead. These properties don't get the fast
> path.
> 
> Could we add them to the list and then assert not reached?

Done.

https://codereview.chromium.org/982913003/diff/40001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/982913003/diff/40001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:222:
RefPtrWillBeRawPtr<AnimatableValue> before, after;
On 2015/03/09 02:39:25, dstockwell wrote:
> I think this would be simpler with ENABLE(ASSERT) -- aren't these unused
> otherwise?

Done.

[dstockwell, 2015-03-09 04:38:45]

The CQ bit was checked by dstockwell@chromium.org

[dstockwell, 2015-03-09 04:38:45]

lgtm

[dstockwell, 2015-03-09 04:38:46]

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/982913003/#ps80001 (title:
"Review changes")

[commit-bot: I haz the power, 2015-03-09 04:39:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/982913003/80001

[commit-bot: I haz the power, 2015-03-09 05:04:30]

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

[commit-bot: I haz the power, 2015-03-09 05:04:31]

Try jobs failed on following builders:
  android_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)

[alancutter (OOO until 2018), 2015-03-09 06:27:43]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2015-03-09 06:27:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
dstockwell@chromium.org, ericwilligers@chromium.org
Link to the patchset: https://codereview.chromium.org/982913003/#ps100001
(title: "Make compile work.")

[commit-bot: I haz the power, 2015-03-09 06:28:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/982913003/100001

[commit-bot: I haz the power, 2015-03-09 08:02:31]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191521