[vr] Ensure unticked transitions complete

If a transition was enqueued but not serviced and, much later, another
transition is enqueued, it should transition from the final value of
the earlier transition.

Bug: 832650,830592
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: I751c5d26101bfa197818ccd2f72188513ed044f2
Reviewed-on: https://chromium-review.googlesource.com/1014008
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551035}

chrome/browser/vr/animation.cc

@@ -297,17 +297,24 @@ void Animation::TransitionValueTo(base::TimeTicks monotonic_time,
   cc::KeyframeModel* running_keyframe_model =
       GetRunningKeyframeModelForProperty(target_property);
 
+  ValueType effective_current = current;
+
   if (running_keyframe_model) {
     const auto* curve = AnimationTraits<ValueType>::ToDerivedCurve(
         *running_keyframe_model->curve());
-    if (SufficientlyEqual(
-            target, curve->GetValue(GetEndTime(running_keyframe_model)))) {
-      return;
-    }
-    if (SufficientlyEqual(
-            target, curve->GetValue(GetStartTime(running_keyframe_model)))) {
-      ReverseKeyframeModel(monotonic_time, running_keyframe_model);
-      return;
+
+    if (running_keyframe_model->IsFinishedAt(monotonic_time)) {
+      effective_current = curve->GetValue(GetEndTime(running_keyframe_model));
+    } else {
+      if (SufficientlyEqual(
+              target, curve->GetValue(GetEndTime(running_keyframe_model)))) {
+        return;
+      }
+      if (SufficientlyEqual(
+              target, curve->GetValue(GetStartTime(running_keyframe_model)))) {
+        ReverseKeyframeModel(monotonic_time, running_keyframe_model);
+        return;
+      }
     }
   } else if (SufficientlyEqual(target, current)) {
     return;
@@ -319,7 +326,7 @@ void Animation::TransitionValueTo(base::TimeTicks monotonic_time,
       curve(AnimationTraits<ValueType>::KeyframedCurveType::Create());
 
   curve->AddKeyframe(AnimationTraits<ValueType>::KeyframeType::Create(
-      base::TimeDelta(), current, CreateTransitionTimingFunction()));
+      base::TimeDelta(), effective_current, CreateTransitionTimingFunction()));
 
   curve->AddKeyframe(AnimationTraits<ValueType>::KeyframeType::Create(
       transition_.duration, target, CreateTransitionTimingFunction()));

chrome/browser/vr/animation_unittest.cc

@@ -192,6 +192,35 @@ TEST(AnimationTest, AnimationQueue) {
   EXPECT_TRUE(animation.keyframe_models().empty());
 }
 
+TEST(AnimationTest, FinishedTransition) {
+  TestAnimationTarget target;
+  Animation animation;
+  animation.set_target(&target);
+  Transition transition;
+  transition.target_properties = {OPACITY};
+  transition.duration = MsToDelta(10);
+  animation.set_transition(transition);
+
+  base::TimeTicks start_time = MsToTicks(1000);
+  animation.Tick(start_time);
+
+  float from = 1.0f;
+  float to = 0.0f;
+  animation.TransitionFloatTo(start_time, OPACITY, from, to);
+
+  animation.Tick(start_time);
+  EXPECT_EQ(from, target.opacity());
+
+  // We now simulate a long pause where the element hasn't been ticked (eg, it
+  // may have been hidden). If this happens, the unticked transition must still
+  // be treated as having finished.
+  animation.TransitionFloatTo(start_time + MsToDelta(1000), OPACITY,
+                              target.opacity(), 1.0f);
+
+  animation.Tick(start_time + MsToDelta(1000));
+  EXPECT_EQ(to, target.opacity());
+}
+
 TEST(AnimationTest, OpacityTransitions) {
   TestAnimationTarget target;
   Animation animation;

chrome/browser/vr/elements/ui_element.cc

@@ -23,7 +23,7 @@ namespace vr {
 
 namespace {
 
-constexpr bool kEnableOptimizedTreeWalks = false;
+constexpr bool kEnableOptimizedTreeWalks = true;
 constexpr float kHitTestResolutionInMeter = 0.000001f;
 
 int AllocateId() {

chrome/browser/vr/ui_scene_creator.cc

@@ -2412,17 +2412,8 @@ void UiSceneCreator::CreateOmnibox() {
       VR_BIND_LAMBDA(
           [](UiElement* e, const bool& v) {
             float y_offset = -0.5 * kOmniboxHeightDMM;
-            // TODO(crbug.com/830592): we should not have to alter the set of
-            // transitioned properties here, but there is a bug in the
-            // transitions code in that it doesn't take into account any
-            // currently running animations when starting a transition.
-            if (v) {
-              e->SetTransitionedProperties({TRANSFORM});
-              y_offset += kOmniboxVerticalOffsetDMM;
-            } else {
-              e->SetTransitionedProperties({});
-              y_offset += kUrlBarVerticalOffsetDMM;
-            }
+            y_offset +=
+                v ? kOmniboxVerticalOffsetDMM : kUrlBarVerticalOffsetDMM;
             e->SetTranslate(0, y_offset, -kOmniboxShadowOffset);
           },
           shadow.get())));