[vr] Stop using UiElementIterator

This turned out to be slow.

Bug: None
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;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id2b89db4cd7fcd8b96eed66415a30afe33af0e59
Reviewed-on: https://chromium-review.googlesource.com/997755Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548461}

chrome/browser/vr/BUILD.gn

@@ -105,7 +105,6 @@ static_library("vr_common") {
     "elements/transient_element.h",
     "elements/ui_element.cc",
     "elements/ui_element.h",
-    "elements/ui_element_iterator.h",
     "elements/ui_element_name.cc",
     "elements/ui_element_name.h",
     "elements/ui_element_type.cc",
@@ -278,7 +277,6 @@ test("vr_common_unittests") {
     "elements/text_unittest.cc",
     "elements/throbber_unittest.cc",
     "elements/transient_element_unittest.cc",
-    "elements/ui_element_iterator_unittest.cc",
     "elements/ui_element_unittest.cc",
     "elements/url_text_unittest.cc",
     "elements/vector_icon_button_unittest.cc",

chrome/browser/vr/elements/ui_element.cc

@@ -113,9 +113,13 @@ void UiElement::SetType(UiElementType type) {
 }
 
 UiElement* UiElement::GetDescendantByType(UiElementType type) {
-  for (auto& descendant : *this) {
-    if (descendant.type() == type)
-      return &descendant;
+  if (type_ == type)
+    return this;
+
+  for (auto& child : children_) {
+    auto* result = child->GetDescendantByType(type);
+    if (result)
+      return result;
   }
   return nullptr;
 }
@@ -596,11 +600,15 @@ gfx::SizeF UiElement::stale_size() const {
 }
 
 void UiElement::AddChild(std::unique_ptr<UiElement> child) {
+  for (UiElement* current = this; current; current = current->parent())
+    current->set_descendants_updated(true);
   child->parent_ = this;
   children_.push_back(std::move(child));
 }
 
 std::unique_ptr<UiElement> UiElement::RemoveChild(UiElement* to_remove) {
+  for (UiElement* current = this; current; current = current->parent())
+    current->set_descendants_updated(true);
   DCHECK_EQ(this, to_remove->parent_);
   to_remove->parent_ = nullptr;
   size_t old_size = children_.size();
@@ -621,12 +629,16 @@ void UiElement::AddBinding(std::unique_ptr<BindingBase> binding) {
   bindings_.push_back(std::move(binding));
 }
 
-void UiElement::UpdateBindings() {
+void UiElement::UpdateBindingsRecursive() {
   updated_bindings_this_frame_ = false;
   for (auto& binding : bindings_) {
     if (binding->Update())
       updated_bindings_this_frame_ = true;
   }
+  set_update_phase(UiElement::kUpdatedBindings);
+  for (auto& child : children_) {
+    child->UpdateBindingsRecursive();
+  }
 }
 
 gfx::Point3F UiElement::GetCenter() const {

chrome/browser/vr/elements/ui_element.h

@@ -19,7 +19,6 @@
 #include "chrome/browser/vr/databinding/binding_base.h"
 #include "chrome/browser/vr/elements/corner_radii.h"
 #include "chrome/browser/vr/elements/draw_phase.h"
-#include "chrome/browser/vr/elements/ui_element_iterator.h"
 #include "chrome/browser/vr/elements/ui_element_name.h"
 #include "chrome/browser/vr/elements/ui_element_type.h"
 #include "chrome/browser/vr/model/camera_model.h"
@@ -355,7 +354,7 @@ class UiElement : public cc::AnimationTarget {
     return bindings_;
   }
 
-  void UpdateBindings();
+  void UpdateBindingsRecursive();
 
   gfx::Point3F GetCenter() const;
   gfx::Vector3dF GetNormal() const;
@@ -421,23 +420,6 @@ class UiElement : public cc::AnimationTarget {
     return children_;
   }
 
-  typedef ForwardUiElementIterator iterator;
-  typedef ConstForwardUiElementIterator const_iterator;
-  typedef ReverseUiElementIterator reverse_iterator;
-  typedef ConstReverseUiElementIterator const_reverse_iterator;
-
-  iterator begin() { return iterator(this); }
-  iterator end() { return iterator(nullptr); }
-  const_iterator begin() const { return const_iterator(this); }
-  const_iterator end() const { return const_iterator(nullptr); }
-
-  reverse_iterator rbegin() { return reverse_iterator(this); }
-  reverse_iterator rend() { return reverse_iterator(nullptr); }
-  const_reverse_iterator rbegin() const { return const_reverse_iterator(this); }
-  const_reverse_iterator rend() const {
-    return const_reverse_iterator(nullptr);
-  }
-
   void set_update_phase(UpdatePhase phase) { phase_ = phase; }
 
   // This is true for all elements that respect the given view model matrix. If
@@ -486,6 +468,9 @@ class UiElement : public cc::AnimationTarget {
     resizable_by_layout_ = resizable;
   }
 
+  bool descendants_updated() const { return descendants_updated_; }
+  void set_descendants_updated(bool updated) { descendants_updated_ = updated; }
+
  protected:
   Animation& animation() { return animation_; }
 
@@ -623,6 +608,10 @@ class UiElement : public cc::AnimationTarget {
   UiElement* parent_ = nullptr;
   std::vector<std::unique_ptr<UiElement>> children_;
 
+  // This is true if a descendant has been added and the total list has not yet
+  // been collected by the scene.
+  bool descendants_updated_ = false;
+
   std::vector<std::unique_ptr<BindingBase>> bindings_;
 
   UpdatePhase phase_ = kClean;

chrome/browser/vr/elements/ui_element_iterator.h

-// 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.
-
-#ifndef CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_
-#define CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_
-
-#include <vector>
-
-namespace vr {
-
-class UiElement;
-
-// The UiElementIterator traverses a UiElement subtree. Do not use this class
-// directly. You should, instead, use UiElement::end/begin/rend/rbegin. NB: you
-// may find base::Reversed handy if you're doing reverse iteration.
-template <typename T, typename U>
-class UiElementIterator {
- public:
-  explicit UiElementIterator(T* root) {
-    current_ = U::Increment(&index_stack_, root, true);
-  }
-  ~UiElementIterator() {}
-
-  void operator++() { current_ = U::Increment(&index_stack_, current_, false); }
-  void operator++(int) {
-    current_ = U::Increment(&index_stack_, current_, false);
-  }
-  T& operator*() { return *current_; }
-  bool operator!=(const UiElementIterator& rhs) {
-    return current_ != rhs.current_ ||
-           index_stack_.empty() != rhs.index_stack_.empty();
-  }
-
- private:
-  T* current_ = nullptr;
-
-  // The iterator maintains a stack of indices which represent the current index
-  // in each list of children along the ancestor path.
-  std::vector<size_t> index_stack_;
-};
-
-template <typename T>
-struct ForwardIncrementer {
-  static T* Increment(std::vector<size_t>* index_stack, T* e, bool init) {
-    if (!e || init)
-      return e;
-
-    if (!e->children().empty()) {
-      index_stack->push_back(0lu);
-      return e->children().front().get();
-    }
-
-    while (e->parent() && !index_stack->empty() &&
-           index_stack->back() + 1lu >= e->parent()->children().size()) {
-      index_stack->pop_back();
-      e = e->parent();
-    }
-
-    if (!e->parent() || index_stack->empty())
-      return nullptr;
-
-    index_stack->back()++;
-    return e->parent()->children()[index_stack->back()].get();
-  }
-};
-
-template <typename T>
-struct ReverseIncrementer {
-  static T* Increment(std::vector<size_t>* index_stack, T* e, bool init) {
-    if (!e || (index_stack->empty() && !init))
-      return nullptr;
-
-    bool should_descend = false;
-    if (index_stack->empty()) {
-      should_descend = true;
-    } else if (e->parent() && index_stack->back() > 0lu) {
-      index_stack->back()--;
-      e = e->parent()->children()[index_stack->back()].get();
-      should_descend = true;
-    }
-
-    if (should_descend) {
-      while (!e->children().empty()) {
-        index_stack->push_back(e->children().size() - 1lu);
-        e = e->children().back().get();
-      }
-      return e;
-    }
-
-    index_stack->pop_back();
-    return e->parent();
-  }
-};
-
-typedef UiElementIterator<UiElement, ForwardIncrementer<UiElement>>
-    ForwardUiElementIterator;
-typedef UiElementIterator<const UiElement, ForwardIncrementer<const UiElement>>
-    ConstForwardUiElementIterator;
-typedef UiElementIterator<UiElement, ReverseIncrementer<UiElement>>
-    ReverseUiElementIterator;
-typedef UiElementIterator<const UiElement, ReverseIncrementer<const UiElement>>
-    ConstReverseUiElementIterator;
-
-}  // namespace vr
-
-#endif  // CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_

chrome/browser/vr/elements/ui_element_iterator_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/elements/ui_element.h"
-
-#include "base/containers/adapters.h"
-#include "base/test/gtest_util.h"
-#include "chrome/browser/vr/ui_scene.h"
-
-namespace vr {
-
-namespace {
-
-// Constructs a tree of the following form
-// 1 kRoot
-//   2 k2dBrowsingRoot
-//     3 kFloor
-//     4 k2dBrowsingContentGroup
-//       5 kBackplane
-//       6 kContentQuad
-//       7 kUrlBar
-//     8 kCeiling
-void MakeTree(UiScene* scene) {
-  auto element = std::make_unique<UiElement>();
-  element->SetName(k2dBrowsingRoot);
-  scene->AddUiElement(kRoot, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(kFloor);
-  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(k2dBrowsingContentGroup);
-  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(kBackplane);
-  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(kContentQuad);
-  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(kUrlBar);
-  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
-
-  element = std::make_unique<UiElement>();
-  element->SetName(kCeiling);
-  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
-}
-
-template <typename T>
-void CollectElements(T* e, std::vector<T*>* elements) {
-  elements->push_back(e);
-  for (auto& child : e->children()) {
-    CollectElements(child.get(), elements);
-  }
-}
-
-}  // namespace
-
-struct UiElementIteratorTestCase {
-  UiElementName root;
-  size_t num_elements_in_subtree;
-};
-
-class UiElementIteratorTest
-    : public ::testing::TestWithParam<UiElementIteratorTestCase> {};
-
-TEST_P(UiElementIteratorTest, VerifyTraversal) {
-  UiScene scene;
-  MakeTree(&scene);
-  std::vector<UiElement*> elements;
-  CollectElements(scene.GetUiElementByName(GetParam().root), &elements);
-
-  size_t i = 0;
-  for (auto& e : *(scene.GetUiElementByName(GetParam().root))) {
-    EXPECT_GT(elements.size(), i);
-    EXPECT_EQ(elements[i++]->id(), e.id());
-  }
-  EXPECT_EQ(elements.size(), i);
-  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
-
-  i = 0;
-  for (auto& e : *const_cast<const UiElement*>(
-           scene.GetUiElementByName(GetParam().root))) {
-    EXPECT_GT(elements.size(), i);
-    EXPECT_EQ(elements[i++]->id(), e.id());
-  }
-  EXPECT_EQ(elements.size(), i);
-  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
-
-  i = 0;
-  for (auto& e : base::Reversed(*scene.GetUiElementByName(GetParam().root))) {
-    EXPECT_GT(elements.size(), i);
-    EXPECT_EQ(elements[elements.size() - i++ - 1lu]->id(), e.id());
-  }
-  EXPECT_EQ(elements.size(), i);
-  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
-
-  i = 0;
-  for (auto& e : base::Reversed(*const_cast<const UiElement*>(
-           scene.GetUiElementByName(GetParam().root)))) {
-    EXPECT_GT(elements.size(), i);
-    EXPECT_EQ(elements[elements.size() - i++ - 1lu]->id(), e.id());
-  }
-  EXPECT_EQ(elements.size(), i);
-  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
-}
-
-const std::vector<UiElementIteratorTestCase> iterator_test_cases = {
-    {kRoot, 8},
-    {k2dBrowsingContentGroup, 4},
-    {kCeiling, 1},
-};
-
-INSTANTIATE_TEST_CASE_P(UiElementIteratorTestCases,
-                        UiElementIteratorTest,
-                        ::testing::ValuesIn(iterator_test_cases));
-
-}  // namespace vr

chrome/browser/vr/test/ui_test.cc

@@ -61,6 +61,13 @@ bool WillElementBeVisible(const UiElement* element) {
   return WillElementFaceCamera(element);
 }
 
+int NumVisibleInTreeRecursive(const UiElement* element) {
+  int visible = WillElementBeVisible(element) ? 1 : 0;
+  for (auto& child : element->children())
+    visible += NumVisibleInTreeRecursive(child.get());
+  return visible;
+}
+
 }  // namespace
 
 UiTest::UiTest() {}
@@ -133,18 +140,18 @@ void UiTest::VerifyOnlyElementsVisible(
     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();
-    UiElementName owner_name = element.owner_name_for_test();
-    if (element.draw_phase() == kPhaseNone && owner_name == kNone) {
+  for (auto* element : scene_->GetAllElements()) {
+    SCOPED_TRACE(element->DebugName());
+    UiElementName name = element->name();
+    UiElementName owner_name = element->owner_name_for_test();
+    if (element->draw_phase() == kPhaseNone && owner_name == kNone) {
       EXPECT_TRUE(names.find(name) == names.end());
       continue;
     }
     if (name == kNone)
       name = owner_name;
     bool should_be_visible = (names.find(name) != names.end());
-    EXPECT_EQ(WillElementBeVisible(&element), should_be_visible);
+    EXPECT_EQ(WillElementBeVisible(element), should_be_visible);
   }
 }
 
@@ -155,12 +162,7 @@ int UiTest::NumVisibleInTree(UiElementName name) const {
   if (!root) {
     return 0;
   }
-  int visible = 0;
-  for (const auto& element : *root) {
-    if (WillElementBeVisible(&element))
-      visible++;
-  }
-  return visible;
+  return NumVisibleInTreeRecursive(root);
 }
 
 bool UiTest::VerifyIsAnimating(const std::set<UiElementName>& names,

chrome/browser/vr/ui_input_manager.cc

@@ -57,13 +57,15 @@ bool IsScrollEvent(const GestureList& list) {
   return false;
 }
 
-void HitTestElements(UiElement* root_element,
+void HitTestElements(UiScene* scene,
                      ReticleModel* reticle_model,
                      HitTestRequest* request) {
+  auto& all_elements = scene->GetAllElements();
+
   std::vector<const UiElement*> elements;
-  for (auto& element : *root_element) {
-    if (element.IsVisible()) {
-      elements.push_back(&element);
+  for (auto* element : all_elements) {
+    if (element->IsVisible()) {
+      elements.push_back(element);
     }
   }
 
@@ -374,7 +376,7 @@ void UiInputManager::GetVisualTargetElement(
   request.ray_origin = ray_origin;
   request.ray_target = reticle_model->target_point;
   request.max_distance_to_plane = distance_limit;
-  HitTestElements(&scene_->root_element(), reticle_model, &request);
+  HitTestElements(scene_, reticle_model, &request);
 }
 
 void UiInputManager::UpdateQuiescenceState(

chrome/browser/vr/ui_scene.cc

@@ -24,16 +24,42 @@ namespace vr {
 namespace {
 
 template <typename P>
-UiScene::Elements GetVisibleElements(UiElement* root,
-                                     P predicate) {
+UiScene::Elements GetVisibleElements(
+    const std::vector<UiElement*>& all_elements,
+    P predicate) {
   UiScene::Elements elements;
-  for (auto& element : *root) {
-    if (element.IsVisible() && predicate(&element))
-      elements.push_back(&element);
+  for (auto* element : all_elements) {
+    if (element->IsVisible() && predicate(element))
+      elements.push_back(element);
   }
   return elements;
 }
 
+void GetAllElementsRecursive(std::vector<UiElement*>* elements, UiElement* e) {
+  e->set_descendants_updated(false);
+  elements->push_back(e);
+  for (auto& child : e->children())
+    GetAllElementsRecursive(elements, child.get());
+}
+
+template <typename P>
+UiElement* FindElement(UiElement* e, P predicate) {
+  if (predicate.Run(e))
+    return e;
+  for (auto& child : e->children()) {
+    if (UiElement* result = FindElement(child.get(), predicate)) {
+      return result;
+    }
+  }
+  return nullptr;
+}
+
+void InitializeElementRecursive(UiElement* e, SkiaSurfaceProvider* provider) {
+  e->Initialize(provider);
+  for (auto& child : e->children())
+    InitializeElementRecursive(child.get(), provider);
+}
+
 }  // namespace
 
 void UiScene::AddUiElement(UiElementName parent,
@@ -73,22 +99,20 @@ bool UiScene::OnBeginFrame(const base::TimeTicks& current_time,
     TRACE_EVENT0("gpu", "UiScene::OnBeginFrame.UpdateBindings");
 
     // Propagate updates across bindings.
-    for (auto& element : *root_element_) {
-      element.UpdateBindings();
-      element.set_update_phase(UiElement::kUpdatedBindings);
-    }
+    root_element_->UpdateBindingsRecursive();
   }
 
+  auto& elements = GetAllElements();
   {
     TRACE_EVENT0("gpu", "UiScene::OnBeginFrame.UpdateAnimationsAndOpacity");
 
     // Process all animations and pre-binding work. I.e., induce any
     // time-related "dirtiness" on the scene graph.
-    for (auto& element : *root_element_) {
-      element.set_update_phase(UiElement::kDirty);
-      if ((element.DoBeginFrame(current_time, head_pose) ||
-           element.updated_bindings_this_frame()) &&
-          (element.IsVisible() || element.updated_visiblity_this_frame())) {
+    for (auto* element : elements) {
+      element->set_update_phase(UiElement::kDirty);
+      if ((element->DoBeginFrame(current_time, head_pose) ||
+           element->updated_bindings_this_frame()) &&
+          (element->IsVisible() || element->updated_visiblity_this_frame())) {
         scene_dirty = true;
       }
     }
@@ -104,21 +128,21 @@ bool UiScene::OnBeginFrame(const base::TimeTicks& current_time,
     // Textures will have to know what their size would be, if they were to draw
     // with their current state, and changing anything other than texture
     // synchronously in response to input should be prohibited.
-    for (auto& element : *root_element_) {
-      element.set_update_phase(UiElement::kUpdatedTexturesAndSizes);
+    for (auto* element : elements) {
+      element->set_update_phase(UiElement::kUpdatedTexturesAndSizes);
     }
     if (root_element_->SizeAndLayOut())
       scene_dirty = true;
-    for (auto& element : *root_element_) {
-      element.set_update_phase(UiElement::kUpdatedLayout);
+    for (auto* element : elements) {
+      element->set_update_phase(UiElement::kUpdatedLayout);
     }
   }
 
   if (!scene_dirty) {
     // Nothing to update, so set all elements to the final update phase and
     // return early.
-    for (auto& element : *root_element_) {
-      element.set_update_phase(UiElement::kUpdatedWorldSpaceTransform);
+    for (auto* element : elements) {
+      element->set_update_phase(UiElement::kUpdatedWorldSpaceTransform);
     }
     return false;
   }
@@ -139,8 +163,8 @@ bool UiScene::UpdateTextures() {
   TRACE_EVENT0("gpu", "UiScene::UpdateTextures");
   bool needs_redraw = false;
   // Update textures and sizes.
-  for (auto& element : *root_element_) {
-    if (element.PrepareToDraw())
+  for (auto* element : GetAllElements()) {
+    if (element->PrepareToDraw())
       needs_redraw = true;
   }
   return needs_redraw;
@@ -151,43 +175,40 @@ UiElement& UiScene::root_element() {
 }
 
 UiElement* UiScene::GetUiElementById(int element_id) const {
-  for (auto& element : *root_element_) {
-    if (element.id() == element_id)
-      return &element;
-  }
-  return nullptr;
+  return FindElement(
+      root_element_.get(),
+      base::BindRepeating([](int id, UiElement* e) { return e->id() == id; },
+                          element_id));
 }
 
 UiElement* UiScene::GetUiElementByName(UiElementName name) const {
-  for (auto& element : *root_element_) {
-    if (element.name() == name)
-      return &element;
+  return FindElement(
+      root_element_.get(),
+      base::BindRepeating(
+          [](UiElementName name, UiElement* e) { return e->name() == name; },
+          name));
+}
+
+std::vector<UiElement*>& UiScene::GetAllElements() {
+  if (root_element_->descendants_updated()) {
+    all_elements_.clear();
+    GetAllElementsRecursive(&all_elements_, root_element_.get());
   }
-  return nullptr;
+  return all_elements_;
 }
 
-UiScene::Elements UiScene::GetVisibleElementsToDraw() const {
-  return GetVisibleElements(GetUiElementByName(kRoot), [](UiElement* element) {
+UiScene::Elements UiScene::GetVisibleElementsToDraw() {
+  return GetVisibleElements(GetAllElements(), [](UiElement* element) {
     return element->draw_phase() == kPhaseForeground ||
            element->draw_phase() == kPhaseBackplanes ||
            element->draw_phase() == kPhaseBackground;
   });
 }
 
-UiScene::Elements UiScene::GetVisibleWebVrOverlayElementsToDraw() const {
-  return GetVisibleElements(
-      GetUiElementByName(kWebVrRoot), [](UiElement* element) {
-        return element->draw_phase() == kPhaseOverlayForeground;
-      });
-}
-
-UiScene::Elements UiScene::GetPotentiallyVisibleElements() const {
-  UiScene::Elements elements;
-  for (auto& element : *root_element_) {
-    if (element.draw_phase() != kPhaseNone)
-      elements.push_back(&element);
-  }
-  return elements;
+UiScene::Elements UiScene::GetVisibleWebVrOverlayElementsToDraw() {
+  return GetVisibleElements(GetAllElements(), [](UiElement* element) {
+    return element->draw_phase() == kPhaseOverlayForeground;
+  });
 }
 
 UiScene::UiScene() {
@@ -200,19 +221,15 @@ UiScene::~UiScene() = default;
 void UiScene::OnGlInitialized(SkiaSurfaceProvider* provider) {
   gl_initialized_ = true;
   provider_ = provider;
-  for (auto& element : *root_element_)
-    element.Initialize(provider_);
+  InitializeElementRecursive(root_element_.get(), provider_);
 }
 
 void UiScene::InitializeElement(UiElement* element) {
   CHECK_GE(element->id(), 0);
   CHECK_EQ(GetUiElementById(element->id()), nullptr);
   CHECK_GE(element->draw_phase(), 0);
-  if (gl_initialized_) {
-    for (auto& child : *element) {
-      child.Initialize(provider_);
-    }
-  }
+  if (gl_initialized_)
+    InitializeElementRecursive(element, provider_);
 }
 
 }  // namespace vr

chrome/browser/vr/ui_scene.h

@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/vr/elements/ui_element.h"
-#include "chrome/browser/vr/elements/ui_element_iterator.h"
 #include "chrome/browser/vr/elements/ui_element_name.h"
 #include "chrome/browser/vr/keyboard_delegate.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -56,9 +55,9 @@ class UiScene {
 
   typedef std::vector<const UiElement*> Elements;
 
-  Elements GetVisibleElementsToDraw() const;
-  Elements GetVisibleWebVrOverlayElementsToDraw() const;
-  Elements GetPotentiallyVisibleElements() const;
+  std::vector<UiElement*>& GetAllElements();
+  Elements GetVisibleElementsToDraw();
+  Elements GetVisibleWebVrOverlayElementsToDraw();
 
   float background_distance() const { return background_distance_; }
   void set_background_distance(float d) { background_distance_ = d; }
@@ -84,6 +83,8 @@ class UiScene {
   // easily compute dirtiness.
   bool is_dirty_ = false;
 
+  std::vector<UiElement*> all_elements_;
+
   SkiaSurfaceProvider* provider_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(UiScene);

chrome/browser/vr/ui_unittest.cc

@@ -116,6 +116,12 @@ void VerifyButtonColor(DiscButton* button,
   EXPECT_EQ(button->background()->center_color(), background_color);
 }
 
+void VerifyNoHitTestableElementInSubtree(UiElement* element) {
+  EXPECT_FALSE(element->IsHitTestable());
+  for (auto& child : element->children())
+    VerifyNoHitTestableElementInSubtree(child.get());
+}
+
 }  // namespace
 
 TEST_F(UiTest, WebVrToastStateTransitions) {
@@ -1280,8 +1286,7 @@ TEST_F(UiTest, ResetRepositioner) {
 TEST_F(UiTest, ControllerHitTest) {
   CreateScene(kNotInCct, kNotInWebVr);
   auto* controller = scene_->GetUiElementByName(kControllerRoot);
-  for (auto& child : *controller)
-    EXPECT_FALSE(child.IsHitTestable());
+  VerifyNoHitTestableElementInSubtree(controller);
 }
 
 TEST_F(UiTest, BrowsingRootBounds) {