[vr] Switch to exclusive visibility checks in the UI unittests

Previously, there were many spots in the UI unittests where we checked
that a set of UI elements (and only that set of UI elements) was
visible. This had regressed, however, and in the interim, elements had
been added that were not accounted for in the test expectations.

In this CL, I have switched back to exclusive visibility checks, and
have updated the appropriate expectations. I have also introduced the
notion of an owner elements. This both permits unit tests to refer to
unnamed sub-elements meaningfully and allows for better debug output
(the hierarchy dumps are much more meaningful).

This also permitted including these unnamed sub-elements in the
stacking unit test. I have also moved the remaining stacking unit
tests into the UI unittests.

I have also updated the backplane click test to simulate the response
to the click so we can check that the visuals update as we expect.

Bug: 782395
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8f4f0fb92df18dc6b8d7a8e086768d4a97136898
Reviewed-on: https://chromium-review.googlesource.com/789440
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519503}

chrome/browser/vr/BUILD.gn

@@ -247,7 +247,6 @@ test("vr_common_unittests") {
     "test/ui_test.cc",
     "test/ui_test.h",
     "ui_input_manager_unittest.cc",
-    "ui_renderer_unittest.cc",
     "ui_scene_unittest.cc",
     "ui_unittest.cc",
     "vr_gl_util_unittest.cc",

chrome/browser/vr/elements/button.cc

@@ -126,6 +126,12 @@ void Button::OnSetDrawPhase() {
   hit_plane_->set_draw_phase(draw_phase());
 }
 
+void Button::OnSetName() {
+  background_->set_owner_name_for_test(name());
+  foreground_->set_owner_name_for_test(name());
+  hit_plane_->set_owner_name_for_test(name());
+}
+
 void Button::NotifyClientSizeAnimated(const gfx::SizeF& size,
                                       int target_property_id,
                                       cc::Animation* animation) {

chrome/browser/vr/elements/button.h

@@ -53,6 +53,7 @@ class Button : public UiElement {
   void OnStateUpdated();
 
   void OnSetDrawPhase() override;
+  void OnSetName() override;
   void NotifyClientSizeAnimated(const gfx::SizeF& size,
                                 int target_property_id,
                                 cc::Animation* animation) override;

chrome/browser/vr/elements/button_unittest.cc

@@ -77,4 +77,13 @@ TEST(Button, DrawPhasePropagatesToSubElements) {
   }
 }
 
+TEST(Button, NamePropagatesToSubElements) {
+  Button button(base::Callback<void()>(), vector_icons::kMicrophoneIcon);
+  button.set_name(kCloseButton);
+
+  for (auto& child : button.children()) {
+    EXPECT_EQ(child->owner_name_for_test(), kCloseButton);
+  }
+}
+
 }  // namespace vr

chrome/browser/vr/elements/ui_element.cc

@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/numerics/ranges.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "third_party/WebKit/public/platform/WebGestureEvent.h"
 #include "third_party/skia/include/core/SkRRect.h"
@@ -22,7 +23,6 @@ namespace {
 
 static constexpr char kRed[] = "\x1b[31m";
 static constexpr char kGreen[] = "\x1b[32m";
-static constexpr char kYellow[] = "\x1b[33m";
 static constexpr char kBlue[] = "\x1b[34m";
 static constexpr char kCyan[] = "\x1b[36m";
 static constexpr char kReset[] = "\x1b[0m";
@@ -65,6 +65,13 @@ UiElement::~UiElement() {
   animation_player_.set_target(nullptr);
 }
 
+void UiElement::set_name(UiElementName name) {
+  name_ = name;
+  OnSetName();
+}
+
+void UiElement::OnSetName() {}
+
 void UiElement::set_type(UiElementType type) {
   type_ = type;
   OnSetType();
@@ -341,7 +348,12 @@ bool UiElement::IsWorldPositioned() const {
 }
 
 std::string UiElement::DebugName() const {
-  return UiElementNameToString(name());
+  return base::StringPrintf(
+      "%s%s%s",
+      UiElementNameToString(name() == kNone ? owner_name_for_test() : name())
+          .c_str(),
+      type() == kTypeNone ? "" : ":",
+      type() == kTypeNone ? "" : UiElementTypeToString(type()).c_str());
 }
 
 void UiElement::DumpHierarchy(std::vector<size_t> counts,
@@ -367,16 +379,12 @@ void UiElement::DumpHierarchy(std::vector<size_t> counts,
   }
   *os << kReset;
 
-  if (!IsVisible()) {
-    *os << kYellow << "(h) " << kReset;
-  }
-
-  *os << DebugName();
-  if (type_ != kTypeNone) {
-    *os << ":" << UiElementTypeToString(type_);
+  if (!IsVisible() || draw_phase() == kPhaseNone) {
+    *os << kBlue;
   }
 
-  *os << " " << kCyan << DrawPhaseToString(draw_phase_) << " " << kReset;
+  *os << DebugName() << kReset << " " << kCyan << DrawPhaseToString(draw_phase_)
+      << " " << kReset;
 
   if (draw_phase_ != kPhaseNone && !size().IsEmpty()) {
     *os << kRed << "[" << size().width() << ", " << size().height() << "] "

chrome/browser/vr/elements/ui_element.h

@@ -113,7 +113,13 @@ class UiElement : public cc::AnimationTarget {
   };
 
   UiElementName name() const { return name_; }
-  void set_name(UiElementName name) { name_ = name; }
+  void set_name(UiElementName name);
+  virtual void OnSetName();
+
+  UiElementName owner_name_for_test() const { return owner_name_for_test_; }
+  void set_owner_name_for_test(UiElementName name) {
+    owner_name_for_test_ = name;
+  }
 
   UiElementType type() const { return type_; }
   void set_type(UiElementType type);
@@ -483,6 +489,11 @@ class UiElement : public cc::AnimationTarget {
   // of a string.
   UiElementName name_ = UiElementName::kNone;
 
+  // This name is used in tests and debugging output to associate a "component"
+  // element with its logical owner, such as a button icon within a specific,
+  // named button instance.
+  UiElementName owner_name_for_test_ = UiElementName::kNone;
+
   // An optional identifier to categorize a reusable element, such as a button
   // background. It can also be used to identify categories of element for
   // common styling. Eg, applying a corner-radius to all tab thumbnails.

chrome/browser/vr/test/ui_test.cc

@@ -103,22 +103,20 @@ bool UiTest::VerifyVisibility(const std::set<UiElementName>& names,
   return true;
 }
 
-void UiTest::VerifyVisible(const std::string& trace_context,
-                           const std::set<UiElementName>& names) const {
-  SCOPED_TRACE(trace_context);
-  VerifyVisibility(names, true);
-}
-
-// TODO(cjgrant): Restore this method. We previously checked that only a
-// specific set of elements is visible, and others are not. After eliminating
-// violations, it can replace VerifyVisible().
 void UiTest::VerifyOnlyElementsVisible(
     const std::string& trace_context,
     const std::set<UiElementName>& names) const {
+  OnBeginFrame();
   SCOPED_TRACE(trace_context);
   for (const auto& element : scene_->root_element()) {
-    SCOPED_TRACE(element.DebugName());
     UiElementName name = element.name();
+    if (name == kNone)
+      name = element.owner_name_for_test();
+    if (element.draw_phase() == kPhaseNone) {
+      EXPECT_TRUE(names.find(name) == names.end());
+      continue;
+    }
+    SCOPED_TRACE(element.DebugName());
     bool should_be_visibile = (names.find(name) != names.end());
     EXPECT_EQ(WillElementBeVisible(&element), should_be_visibile);
   }

chrome/browser/vr/test/ui_test.h

@@ -63,10 +63,6 @@ class UiTest : public testing::Test {
   bool VerifyVisibility(const std::set<UiElementName>& names,
                         bool visible) const;
 
-  // Verify all elements in the set are visible.
-  void VerifyVisible(const std::string& debug_name,
-                     const std::set<UiElementName>& names) const;
-
   // Check that only a specific set of elements is visible, and others are not.
   void VerifyOnlyElementsVisible(const std::string& trace_context,
                                  const std::set<UiElementName>& names) const;

chrome/browser/vr/ui_renderer_unittest.cc

-// Copyright 2017 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 "chrome/browser/vr/ui_renderer.h"
-
-#include <memory>
-
-#include "base/memory/ptr_util.h"
-#include "base/time/time.h"
-#include "chrome/browser/vr/elements/ui_element.h"
-#include "chrome/browser/vr/elements/ui_element_name.h"
-#include "chrome/browser/vr/model/model.h"
-#include "chrome/browser/vr/test/animation_utils.h"
-#include "chrome/browser/vr/test/constants.h"
-#include "chrome/browser/vr/test/ui_test.h"
-#include "chrome/browser/vr/ui_scene.h"
-
-namespace vr {
-
-typedef UiScene::Elements (UiScene::*GeneratorFn)() const;
-struct TestParams {
-  GeneratorFn f;
-  std::vector<UiElementName> expected_order;
-};
-
-class UiRendererTest : public UiTest,
-                       public ::testing::WithParamInterface<TestParams> {
- public:
-  void SetUp() override {
-    UiTest::SetUp();
-    CreateScene(kNotInCct, kNotInWebVr);
-  }
-};
-
-TEST_F(UiRendererTest, ReticleStacking) {
-  UiElement* content = scene_->GetUiElementByName(kContentQuad);
-  EXPECT_TRUE(content);
-  model_->reticle.target_element_id = content->id();
-  auto unsorted = scene_->GetVisible2dBrowsingElements();
-  auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
-  bool saw_target = false;
-  for (auto* e : sorted) {
-    if (e == content) {
-      saw_target = true;
-    } else if (saw_target) {
-      EXPECT_EQ(kReticle, e->name());
-      break;
-    }
-  }
-
-  EXPECT_TRUE(saw_target);
-
-  auto controller_elements = scene_->GetVisibleControllerElements();
-  bool saw_reticle = false;
-  for (auto* e : controller_elements) {
-    if (e->name() == kReticle) {
-      saw_reticle = true;
-    }
-  }
-  EXPECT_FALSE(saw_reticle);
-}
-
-TEST_F(UiRendererTest, ReticleStackingAtopForeground) {
-  UiElement* element = scene_->GetUiElementByName(kContentQuad);
-  EXPECT_TRUE(element);
-  element->set_draw_phase(kPhaseOverlayForeground);
-  model_->reticle.target_element_id = element->id();
-  auto unsorted = scene_->GetVisible2dBrowsingOverlayElements();
-  auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
-  bool saw_target = false;
-  for (auto* e : sorted) {
-    if (e == element) {
-      saw_target = true;
-    } else if (saw_target) {
-      EXPECT_EQ(kReticle, e->name());
-      break;
-    }
-  }
-  EXPECT_TRUE(saw_target);
-
-  auto controller_elements = scene_->GetVisibleControllerElements();
-  bool saw_reticle = false;
-  for (auto* e : controller_elements) {
-    if (e->name() == kReticle) {
-      saw_reticle = true;
-    }
-  }
-  EXPECT_FALSE(saw_reticle);
-}
-
-TEST_F(UiRendererTest, ReticleStackingWithControllerElements) {
-  UiElement* element = scene_->GetUiElementByName(kReticle);
-  EXPECT_TRUE(element);
-  element->set_draw_phase(kPhaseBackground);
-  EXPECT_NE(scene_->GetUiElementByName(kLaser)->draw_phase(),
-            element->draw_phase());
-  model_->reticle.target_element_id = 0;
-  auto unsorted = scene_->GetVisibleControllerElements();
-  auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
-  EXPECT_EQ(element->DebugName(), sorted.back()->DebugName());
-  EXPECT_EQ(scene_->GetUiElementByName(kLaser)->draw_phase(),
-            element->draw_phase());
-}
-
-}  // namespace vr