Revert "Use retargeted start point when starting composited transitions."

This reverts commit 6562517f.

Reason for revert: I think this has an invalid cast and is causing CFI failures, see https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20CFI/9012 .

Original change's description:
> Use retargeted start point when starting composited transitions.
> 
> When starting transitions we check if they are interpolable, and don't
> start a TransitionInterpolation if not. When retargeting, the start
> keyframe from the previously active interpolation may cause an
> incompatible interpolation so we must take this into account.
> 
> Bug: 853698
> Change-Id: Iaeeba35c64c659818c68d30e8f999e704b382926
> Reviewed-on: https://chromium-review.googlesource.com/1126519
> Commit-Queue: Robert Flack <flackr@chromium.org>
> Reviewed-by: Majid Valipour <majidvp@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#573980}

TBR=flackr@chromium.org,majidvp@chromium.org

Change-Id: I4de74036835b09da2fe4751280bea9d9b0e205a0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 853698
Reviewed-on: https://chromium-review.googlesource.com/1132016Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574034}

third_party/blink/renderer/core/BUILD.gn

@@ -1643,7 +1643,6 @@ jumbo_source_set("unit_tests") {
     "animation/animation_test_helper.cc",
     "animation/animation_test_helper.h",
     "animation/compositor_animations_test.cc",
-    "animation/css/css_animations_test.cc",
     "animation/css/css_transition_data_test.cc",
     "animation/document_timeline_test.cc",
     "animation/effect_input_test.cc",

third_party/blink/renderer/core/animation/css/css_animations.cc

@@ -199,24 +199,6 @@ StringKeyframeEffectModel* CreateKeyframeEffectModel(
   return model;
 }
 
-// Sample the given |animation| at the given |inherited_time|. Returns nullptr
-// if the |inherited_time| falls outside of the animation.
-std::unique_ptr<TypedInterpolationValue> SampleAnimation(
-    Animation* animation,
-    double inherited_time) {
-  KeyframeEffect* effect = ToKeyframeEffect(animation->effect());
-  InertEffect* inert_animation_for_sampling = InertEffect::Create(
-      effect->Model(), effect->SpecifiedTiming(), false, inherited_time);
-  Vector<scoped_refptr<Interpolation>> sample;
-  inert_animation_for_sampling->Sample(sample);
-  // Transition animation has only animated a single property or is not in
-  // effect.
-  DCHECK_LE(sample.size(), 1u);
-  if (sample.IsEmpty())
-    return nullptr;
-  return ToTransitionInterpolation(*sample.at(0)).GetInterpolatedValue();
-}
-
 }  // namespace
 
 CSSAnimations::CSSAnimations() = default;
@@ -543,7 +525,8 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
   // be when transitions are retargeted. Instead of triggering complete style
   // recalculation, we find these cases by searching for new transitions that
   // have matching cancelled animation property IDs on the compositor.
-  HashSet<PropertyHandle> retargeted_compositor_transitions;
+  HeapHashMap<PropertyHandle, std::pair<Member<KeyframeEffect>, double>>
+      retargeted_compositor_transitions;
   for (const PropertyHandle& property :
        pending_update_.CancelledTransitions()) {
     DCHECK(transitions_.Contains(property));
@@ -554,7 +537,10 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
         pending_update_.NewTransitions().find(property) !=
             pending_update_.NewTransitions().end() &&
         !animation->Limited()) {
-      retargeted_compositor_transitions.insert(property);
+      retargeted_compositor_transitions.insert(
+          property,
+          std::pair<KeyframeEffect*, double>(
+              effect, animation->StartTimeInternal().value_or(NullValue())));
     }
     animation->cancel();
     // after cancelation, transitions must be downgraded or they'll fail
@@ -593,6 +579,44 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
 
     KeyframeEffectModelBase* model = inert_animation->Model();
 
+    if (retargeted_compositor_transitions.Contains(property)) {
+      const std::pair<Member<KeyframeEffect>, double>& old_transition =
+          retargeted_compositor_transitions.at(property);
+      KeyframeEffect* old_animation = old_transition.first;
+      double old_start_time = old_transition.second;
+      double inherited_time =
+          IsNull(old_start_time)
+              ? 0
+              : element->GetDocument().Timeline().CurrentTimeInternal() -
+                    old_start_time;
+
+      TransitionKeyframeEffectModel* old_effect =
+          ToTransitionKeyframeEffectModel(inert_animation->Model());
+      const KeyframeVector& frames = old_effect->GetFrames();
+
+      TransitionKeyframeVector new_frames;
+      new_frames.push_back(ToTransitionKeyframe(frames[0]->Clone().get()));
+      new_frames.push_back(ToTransitionKeyframe(frames[1]->Clone().get()));
+      new_frames.push_back(ToTransitionKeyframe(frames[2]->Clone().get()));
+
+      InertEffect* inert_animation_for_sampling = InertEffect::Create(
+          old_animation->Model(), old_animation->SpecifiedTiming(), false,
+          inherited_time);
+      Vector<scoped_refptr<Interpolation>> sample;
+      inert_animation_for_sampling->Sample(sample);
+      if (sample.size() == 1) {
+        const TransitionInterpolation& interpolation =
+            ToTransitionInterpolation(*sample.at(0));
+        new_frames[0]->SetValue(interpolation.GetInterpolatedValue());
+        new_frames[0]->SetCompositorValue(
+            interpolation.GetInterpolatedCompositorValue());
+        new_frames[1]->SetValue(interpolation.GetInterpolatedValue());
+        new_frames[1]->SetCompositorValue(
+            interpolation.GetInterpolatedCompositorValue());
+        model = TransitionKeyframeEffectModel::Create(new_frames);
+      }
+    }
+
     KeyframeEffect* transition = KeyframeEffect::Create(
         element, model, inert_animation->SpecifiedTiming(),
         KeyframeEffect::kTransitionPriority, event_delegate);
@@ -654,7 +678,6 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
   }
 
   const RunningTransition* interrupted_transition = nullptr;
-  const RunningTransition* retargeted_compositor_transition = nullptr;
   if (state.active_transitions) {
     TransitionMap::const_iterator active_transition_iter =
         state.active_transitions->find(property);
@@ -666,10 +689,6 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
         return;
       }
       state.update.CancelTransition(property);
-      KeyframeEffect* effect =
-          ToKeyframeEffect(running_transition->animation->effect());
-      if (effect->HasActiveAnimationsOnCompositor())
-        retargeted_compositor_transition = running_transition;
       DCHECK(!state.animating_element->GetElementAnimations() ||
              !state.animating_element->GetElementAnimations()
                   ->IsAnimationStyleChange());
@@ -699,52 +718,22 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
                                state.animating_element->GetDocument());
   CSSInterpolationEnvironment old_environment(map, state.old_style);
   CSSInterpolationEnvironment new_environment(map, state.style);
-  const InterpolationType* transition_type = nullptr;
   InterpolationValue start = nullptr;
   InterpolationValue end = nullptr;
-  if (retargeted_compositor_transition) {
-    double old_start_time =
-        retargeted_compositor_transition->animation->StartTimeInternal()
-            .value_or(NullValue());
-    // TODO(flackr): This should be able to just use
-    // animation->currentTime() / 1000 rather than trying to calculate current
-    // time.
-    double inherited_time = IsNull(old_start_time)
-                                ? 0
-                                : state.animating_element->GetDocument()
-                                          .Timeline()
-                                          .CurrentTimeInternal() -
-                                      old_start_time;
-    std::unique_ptr<TypedInterpolationValue> retargeted_start = SampleAnimation(
-        retargeted_compositor_transition->animation, inherited_time);
-    if (retargeted_start) {
-      const InterpolationType& interpolation_type = retargeted_start->GetType();
-      start = retargeted_start->Value().Clone();
-      end = interpolation_type.MaybeConvertUnderlyingValue(new_environment);
-      if (end &&
-          interpolation_type.MaybeMergeSingles(start.Clone(), end.Clone()))
-        transition_type = &interpolation_type;
-    } else {
-      // If the previous transition was not in effect it is not used for
-      // retargeting.
-      retargeted_compositor_transition = nullptr;
+  const InterpolationType* transition_type = nullptr;
+  for (const auto& interpolation_type : map.Get(property)) {
+    start = interpolation_type->MaybeConvertUnderlyingValue(old_environment);
+    if (!start) {
+      continue;
     }
-  }
-  if (!retargeted_compositor_transition) {
-    for (const auto& interpolation_type : map.Get(property)) {
-      start = interpolation_type->MaybeConvertUnderlyingValue(old_environment);
-      if (!start) {
-        continue;
-      }
-      end = interpolation_type->MaybeConvertUnderlyingValue(new_environment);
-      if (!end) {
-        continue;
-      }
-      // Merge will only succeed if the two values are considered interpolable.
-      if (interpolation_type->MaybeMergeSingles(start.Clone(), end.Clone())) {
-        transition_type = interpolation_type.get();
-        break;
-      }
+    end = interpolation_type->MaybeConvertUnderlyingValue(new_environment);
+    if (!end) {
+      continue;
+    }
+    // Merge will only succeed if the two values are considered interpolable.
+    if (interpolation_type->MaybeMergeSingles(start.Clone(), end.Clone())) {
+      transition_type = interpolation_type.get();
+      break;
     }
   }
 

third_party/blink/renderer/core/animation/css/css_animations_test.cc

-// Copyright 2018 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 "third_party/blink/renderer/core/animation/css/css_animations.h"
-#include "third_party/blink/renderer/core/animation/animation.h"
-#include "third_party/blink/renderer/core/animation/element_animations.h"
-#include "third_party/blink/renderer/core/dom/node_computed_style.h"
-#include "third_party/blink/renderer/core/layout/layout_object.h"
-#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
-#include "third_party/blink/renderer/platform/animation/compositor_animation_delegate.h"
-#include "third_party/blink/renderer/platform/testing/testing_platform_support_with_mock_scheduler.h"
-
-namespace blink {
-
-namespace {
-
-class TestingPlatformSupportWithMockSchedulerAndThreadedAnimations
-    : public TestingPlatformSupportWithMockScheduler {
- public:
-  bool IsThreadedAnimationEnabled() override { return true; }
-};
-
-}  // namespace
-
-class CSSAnimationsTest : public RenderingTest {
- public:
-  void SetUp() override {
-    platform_->SetAutoAdvanceNowToPendingTasks(false);
-    // Advance timer manually as RenderingTest expects the time to be non-zero.
-    platform_->AdvanceClockSeconds(1.);
-    RenderingTest::SetUp();
-    EnableCompositing();
-  }
-
-  void TearDown() override {
-    platform_->SetAutoAdvanceNowToPendingTasks(true);
-    platform_->RunUntilIdle();
-  }
-
-  void StartAnimationOnCompositor(Animation* animation) {
-    static_cast<CompositorAnimationDelegate*>(animation)
-        ->NotifyAnimationStarted(
-            (CurrentTimeTicks() - base::TimeTicks()).InSecondsF(),
-            animation->CompositorGroup());
-  }
-
-  void AdvanceClockSeconds(double seconds) {
-    platform_->AdvanceClockSeconds(seconds);
-    platform_->RunUntilIdle();
-  }
-
-  double GetContrastFilterAmount(Element* element) {
-    EXPECT_EQ(1u, element->GetComputedStyle()->Filter().size());
-    const FilterOperation* filter =
-        element->GetComputedStyle()->Filter().Operations()[0];
-    EXPECT_EQ(FilterOperation::OperationType::CONTRAST, filter->GetType());
-    return static_cast<const BasicColorMatrixFilterOperation*>(filter)
-        ->Amount();
-  }
-
- private:
-  ScopedTestingPlatformSupport<
-      TestingPlatformSupportWithMockSchedulerAndThreadedAnimations>
-      platform_;
-};
-
-// Verify that a composited animation is retargeted according to its composited
-// time.
-TEST_F(CSSAnimationsTest, RetargetedTransition) {
-  SetBodyInnerHTML(R"HTML(
-    <style>
-      #test { transition: filter linear 1s; }
-      .contrast1 { filter: contrast(50%); }
-      .contrast2 { filter: contrast(0%); }
-    </style>
-    <div id='test'></div>
-  )HTML");
-  Element* element = GetDocument().getElementById("test");
-  element->setAttribute(HTMLNames::classAttr, "contrast1");
-  GetDocument().View()->UpdateAllLifecyclePhases();
-  ElementAnimations* animations = element->GetElementAnimations();
-  EXPECT_EQ(1u, animations->Animations().size());
-  Animation* animation = (*animations->Animations().begin()).key;
-  // Start animation on compositor and advance .8 seconds.
-  StartAnimationOnCompositor(animation);
-  EXPECT_TRUE(animation->HasActiveAnimationsOnCompositor());
-  AdvanceClockSeconds(0.8);
-
-  // Starting the second transition should retarget the active transition.
-  element->setAttribute(HTMLNames::classAttr, "contrast2");
-  GetPage().Animator().ServiceScriptedAnimations(CurrentTimeTicks());
-  GetDocument().View()->UpdateAllLifecyclePhases();
-  EXPECT_DOUBLE_EQ(0.6, GetContrastFilterAmount(element));
-
-  // As it has been retargeted, advancing halfway should go to 0.3.
-  AdvanceClockSeconds(0.5);
-  GetPage().Animator().ServiceScriptedAnimations(CurrentTimeTicks());
-  GetDocument().View()->UpdateAllLifecyclePhases();
-  EXPECT_DOUBLE_EQ(0.3, GetContrastFilterAmount(element));
-}
-
-// Test that when an incompatible in progress compositor transition
-// would be retargeted it does not incorrectly combine with a new
-// transition target.
-TEST_F(CSSAnimationsTest, IncompatibleRetargetedTransition) {
-  SetBodyInnerHTML(R"HTML(
-    <style>
-      #test { transition: filter 1s; }
-      .saturate { filter: saturate(20%); }
-      .contrast { filter: contrast(20%); }
-    </style>
-    <div id='test'></div>
-  )HTML");
-  Element* element = GetDocument().getElementById("test");
-  element->setAttribute(HTMLNames::classAttr, "saturate");
-  GetDocument().View()->UpdateAllLifecyclePhases();
-  ElementAnimations* animations = element->GetElementAnimations();
-  EXPECT_EQ(1u, animations->Animations().size());
-  Animation* animation = (*animations->Animations().begin()).key;
-
-  // Start animation on compositor and advance partially.
-  StartAnimationOnCompositor(animation);
-  EXPECT_TRUE(animation->HasActiveAnimationsOnCompositor());
-  AdvanceClockSeconds(0.003);
-
-  // The computed style still contains no filter until the next frame.
-  EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
-
-  // Now we start a contrast filter. Since it will try to combine with
-  // the in progress saturate filter, and be incompatible, there should
-  // be no transition and it should immediately apply on the next frame.
-  element->setAttribute(HTMLNames::classAttr, "contrast");
-  EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
-  GetDocument().View()->UpdateAllLifecyclePhases();
-  EXPECT_EQ(0.2, GetContrastFilterAmount(element));
-}
-
-}  // namespace blink