[vr] Permit nested visibility animations to start simultaneously

Like bindings, we now recur updating anims if we are or will be seen.

Also removes flag to disable truncated treewalks.

Bug: 842644
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: I26ac8a140c86b3ef223d00677a710709ca207cea
Reviewed-on: https://chromium-review.googlesource.com/1057347
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558346}

chrome/browser/vr/elements/transient_element_unittest.cc

@@ -56,7 +56,7 @@ TEST(SimpleTransientElementTest, Visibility) {
   EXPECT_FALSE(DoBeginFrame(&element, 4030));
   element.SetTransitionedProperties({OPACITY});
   element.SetVisible(true);
-  EXPECT_FALSE(DoBeginFrame(&element, 4030));
+  EXPECT_TRUE(DoBeginFrame(&element, 4030));
   EXPECT_NE(element.opacity_when_visible(), element.opacity());
   element.SetVisibleImmediately(true);
   EXPECT_FALSE(DoBeginFrame(&element, 4030));

chrome/browser/vr/elements/ui_element.cc

@@ -24,7 +24,6 @@ namespace vr {
 
 namespace {
 
-constexpr bool kEnableOptimizedTreeWalks = true;
 constexpr float kHitTestResolutionInMeter = 0.000001f;
 
 int AllocateId() {
@@ -253,12 +252,14 @@ bool UiElement::DoBeginFrame(const gfx::Transform& head_pose) {
   set_update_phase(kUpdatedAnimations);
   bool begin_frame_updated = OnBeginFrame(head_pose);
   UpdateComputedOpacity();
-  bool was_visible_at_any_point = IsVisible() || updated_visibility_this_frame_;
+  bool was_visible_at_any_point = IsVisible() ||
+                                  updated_visibility_this_frame_ ||
+                                  IsOrWillBeLocallyVisible();
   bool dirty = (begin_frame_updated || keyframe_models_updated ||
                 updated_bindings_this_frame_) &&
                was_visible_at_any_point;
 
-  if (!kEnableOptimizedTreeWalks || was_visible_at_any_point) {
+  if (was_visible_at_any_point) {
     for (auto& child : children_)
       dirty |= child->DoBeginFrame(head_pose);
   }
@@ -699,7 +700,7 @@ void UiElement::UpdateBindings() {
   should_recur |= IsOrWillBeLocallyVisible();
 
   set_update_phase(kUpdatedBindings);
-  if (!should_recur && kEnableOptimizedTreeWalks)
+  if (!should_recur)
     return;
 
   for (auto& child : children_)
@@ -803,7 +804,7 @@ bool UiElement::IsAnimatingProperty(TargetProperty property) const {
 }
 
 bool UiElement::SizeAndLayOut() {
-  if (!IsVisible() && kEnableOptimizedTreeWalks)
+  if (!IsVisible())
     return false;
 
   // May be overridden by layout elements.
@@ -961,8 +962,7 @@ void UiElement::UpdateComputedOpacity() {
 }
 
 void UiElement::UpdateWorldSpaceTransform(bool parent_changed) {
-  if (!IsVisible() && !updated_visibility_this_frame_ &&
-      kEnableOptimizedTreeWalks)
+  if (!IsVisible() && !updated_visibility_this_frame_)
     return;
 
   bool changed = false;

chrome/browser/vr/elements/ui_element_unittest.cc

@@ -11,6 +11,7 @@
 #include "cc/animation/keyframe_model.h"
 #include "cc/animation/keyframed_animation_curve.h"
 #include "cc/test/geometry_test_utils.h"
+#include "chrome/browser/vr/databinding/binding.h"
 #include "chrome/browser/vr/test/animation_utils.h"
 #include "chrome/browser/vr/test/constants.h"
 #include "chrome/browser/vr/ui_scene.h"
@@ -425,6 +426,55 @@ class ElementEventHandlers {
   DISALLOW_COPY_AND_ASSIGN(ElementEventHandlers);
 };
 
+TEST(UiElement, CoordinatedVisibilityTransitions) {
+  UiScene scene;
+  bool value = false;
+
+  auto parent = std::make_unique<UiElement>();
+  auto* parent_ptr = parent.get();
+  parent->SetVisible(false);
+  parent->SetTransitionedProperties({OPACITY});
+  parent->AddBinding(std::make_unique<Binding<bool>>(
+      base::BindRepeating([](bool* value) { return *value; },
+                          base::Unretained(&value)),
+      base::BindRepeating(
+          [](UiElement* e, const bool& value) { e->SetVisible(value); },
+          parent_ptr)));
+
+  auto child = std::make_unique<UiElement>();
+  auto* child_ptr = child.get();
+  child->SetVisible(false);
+  child->SetTransitionedProperties({OPACITY});
+  child->AddBinding(std::make_unique<Binding<bool>>(
+      base::BindRepeating([](bool* value) { return *value; },
+                          base::Unretained(&value)),
+      base::BindRepeating(
+          [](UiElement* e, const bool& value) { e->SetVisible(value); },
+          child_ptr)));
+
+  parent->AddChild(std::move(child));
+  scene.AddUiElement(kRoot, std::move(parent));
+
+  scene.OnBeginFrame(MsToTicks(0), kStartHeadPose);
+
+  value = true;
+
+  scene.OnBeginFrame(MsToTicks(16), kStartHeadPose);
+
+  // We should have started animating both, and they should both be at opacity
+  // zero given that this is the first frame. This does not guarantee that
+  // they've started animating together, however. Even if the animation was
+  // unticked, we would still be at opacity zero. We must tick a second time to
+  // reach a non-zero value.
+  EXPECT_TRUE(parent_ptr->IsAnimatingProperty(OPACITY));
+  EXPECT_TRUE(child_ptr->IsAnimatingProperty(OPACITY));
+  EXPECT_EQ(child_ptr->opacity(), parent_ptr->opacity());
+
+  scene.OnBeginFrame(MsToTicks(32), kStartHeadPose);
+  EXPECT_EQ(child_ptr->opacity(), parent_ptr->opacity());
+  EXPECT_LT(0.0f, child_ptr->opacity());
+}
+
 TEST(UiElement, EventBubbling) {
   auto element = std::make_unique<UiElement>();
   auto child = std::make_unique<UiElement>();