VR: Call KeyboardDelegate::OnBeginFrame every frame regardless of visibility

The gvr keyboard API requires that gvr_keyboard_advance_frame is called every
time after the keyboard is initialized. This regressed in the optimization that
doesn't call UiElement::OnBeginFrame for invisible elements.

We add a special hook to do certain thing every frame.

Bug: 831749
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: I877299165dde517fab90451788de9680700666d9
Reviewed-on: https://chromium-review.googlesource.com/1009042
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550215}

chrome/browser/android/vr/vr_shell_gl.cc

@@ -1500,6 +1500,8 @@ void VrShellGl::DrawFrame(int16_t frame_index, base::TimeTicks current_time) {
     ui_controller_update_time_.AddSample(controller_time);
   }
 
+  ui_->scene()->CallPerFrameCallbacks();
+
   bool textures_changed = ui_->scene()->UpdateTextures();
 
   // TODO(mthiesse): Determine if a visible controller is actually drawn in the

chrome/browser/vr/elements/content_element_unittest.cc

@@ -164,6 +164,10 @@ TEST_F(ContentElementSceneTest, WebInputFocus) {
   content->OnFocusChanged(false);
   EXPECT_TRUE(input_forwarder_->clear_focus_called());
   EXPECT_TRUE(OnBeginFrame());
+  // OnBeginFrame on the keyboard delegate should be called despite of
+  // visibility.
+  EXPECT_CALL(*kb_delegate, OnBeginFrame()).InSequence(in_sequence_);
+  scene_->CallPerFrameCallbacks();
 }
 
 class ContentElementInputEditingTest : public UiTest {

chrome/browser/vr/elements/keyboard.cc

@@ -97,6 +97,16 @@ void Keyboard::OnButtonUp(const gfx::PointF& position) {
   delegate_->OnButtonUp(position);
 }
 
+void Keyboard::AdvanceKeyboardFrameIfNeeded() {
+  // If the update phase is not dirty, the frame will be advanced in
+  // OnBeginFrame. That is, we only call OnBeginFrame below when the element is
+  // not visible (i.e UiElement::kDirty is true).
+  if (!delegate_ || update_phase() != UiElement::kDirty)
+    return;
+
+  delegate_->OnBeginFrame();
+}
+
 bool Keyboard::OnBeginFrame(const gfx::Transform& head_pose) {
   if (!delegate_)
     return false;

chrome/browser/vr/elements/keyboard.h

@@ -19,6 +19,9 @@ class Keyboard : public UiElement {
   Keyboard();
   ~Keyboard() override;
 
+  // The gvr keyboard requires that we advance its frame after initilization,
+  // for example, regardless of visibility.
+  void AdvanceKeyboardFrameIfNeeded();
   void SetKeyboardDelegate(KeyboardDelegate* keyboard_delegate);
   void OnTouchStateUpdated(bool is_touching, const gfx::PointF& touch_position);
   void HitTest(const HitTestRequest& request,

chrome/browser/vr/elements/ui_element.cc

@@ -308,7 +308,7 @@ bool UiElement::IsOrWillBeLocallyVisible() const {
 }
 
 gfx::SizeF UiElement::size() const {
-  DCHECK_LE(kUpdatedSize, phase_);
+  DCHECK_LE(kUpdatedSize, update_phase_);
   return size_;
 }
 
@@ -431,7 +431,7 @@ float UiElement::ComputeTargetOpacity() const {
 }
 
 float UiElement::computed_opacity() const {
-  DCHECK_LE(kUpdatedComputedOpacity, phase_) << DebugName();
+  DCHECK_LE(kUpdatedComputedOpacity, update_phase_) << DebugName();
   return computed_opacity_;
 }
 
@@ -495,7 +495,7 @@ void UiElement::HitTest(const HitTestRequest& request,
 }
 
 const gfx::Transform& UiElement::world_space_transform() const {
-  DCHECK_LE(kUpdatedWorldSpaceTransform, phase_);
+  DCHECK_LE(kUpdatedWorldSpaceTransform, update_phase_);
   return world_space_transform_;
 }
 
@@ -554,21 +554,21 @@ void UiElement::DumpHierarchy(std::vector<size_t> counts,
   }
   *os << kReset;
 
-  if (phase_ < kUpdatedComputedOpacity || !IsVisible()) {
+  if (update_phase_ < kUpdatedComputedOpacity || !IsVisible()) {
     *os << kBlue;
   }
 
   *os << DebugName() << kReset << " " << kCyan << DrawPhaseToString(draw_phase_)
       << " " << kReset;
 
-  if (phase_ >= kUpdatedSize) {
+  if (update_phase_ >= kUpdatedSize) {
     if (size().width() != 0.0f || size().height() != 0.0f) {
       *os << kRed << "[" << size().width() << ", " << size().height() << "] "
           << kReset;
     }
   }
 
-  if (phase_ >= kUpdatedWorldSpaceTransform) {
+  if (update_phase_ >= kUpdatedWorldSpaceTransform) {
     *os << kGreen;
     DumpGeometry(os);
   }
@@ -844,7 +844,7 @@ void UiElement::DoLayOutChildren() {
 }
 
 void UiElement::LayOutChildren() {
-  DCHECK_LE(kUpdatedSize, phase_);
+  DCHECK_LE(kUpdatedSize, update_phase_);
   for (auto& child : children_) {
     if (!child->IsVisible())
       continue;

chrome/browser/vr/elements/ui_element.h

@@ -430,7 +430,7 @@ class UiElement : public cc::AnimationTarget {
     return children_;
   }
 
-  void set_update_phase(UpdatePhase phase) { phase_ = phase; }
+  void set_update_phase(UpdatePhase phase) { update_phase_ = phase; }
 
   // This is true for all elements that respect the given view model matrix. If
   // this is ignored (say for head-locked elements that draw in screen space),
@@ -494,6 +494,8 @@ class UiElement : public cc::AnimationTarget {
     world_space_transform_dirty_ = true;
   }
 
+  UpdatePhase update_phase() const { return update_phase_; }
+
   EventHandlers event_handlers_;
 
  private:
@@ -634,7 +636,7 @@ class UiElement : public cc::AnimationTarget {
   // TODO(crbug.com/829880): remove this once we've simplified our bindings.
   bool visibility_bindings_depend_on_child_visibility_ = false;
 
-  UpdatePhase phase_ = kClean;
+  UpdatePhase update_phase_ = kClean;
 
   AudioDelegate* audio_delegate_ = nullptr;
   Sounds sounds_;

chrome/browser/vr/ui_scene.cc

@@ -159,6 +159,12 @@ bool UiScene::OnBeginFrame(const base::TimeTicks& current_time,
   return scene_dirty;
 }
 
+void UiScene::CallPerFrameCallbacks() {
+  for (auto callback : per_frame_callback_) {
+    callback.Run();
+  }
+}
+
 bool UiScene::UpdateTextures() {
   bool needs_redraw = false;
   std::vector<UiElement*> elements = GetVisibleElementsMutable();
@@ -235,6 +241,10 @@ void UiScene::OnGlInitialized(SkiaSurfaceProvider* provider) {
   InitializeElementRecursive(root_element_.get(), provider_);
 }
 
+void UiScene::AddPerFrameCallback(PerFrameCallback callback) {
+  per_frame_callback_.push_back(callback);
+}
+
 void UiScene::InitializeElement(UiElement* element) {
   CHECK_GE(element->id(), 0);
   CHECK_EQ(GetUiElementById(element->id()), nullptr);

chrome/browser/vr/ui_scene.h

@@ -29,6 +29,8 @@ class UiElement;
 
 class UiScene {
  public:
+  typedef base::RepeatingCallback<void()> PerFrameCallback;
+
   UiScene();
   ~UiScene();
 
@@ -45,6 +47,8 @@ class UiScene {
   bool OnBeginFrame(const base::TimeTicks& current_time,
                     const gfx::Transform& head_pose);
 
+  void CallPerFrameCallbacks();
+
   // Returns true if any textures were redrawn.
   bool UpdateTextures();
 
@@ -68,6 +72,9 @@ class UiScene {
   void set_dirty() { is_dirty_ = true; }
 
   void OnGlInitialized(SkiaSurfaceProvider* provider);
+  // The callback to call on every new frame. This is used for things we want to
+  // do every frame regardless of element or subtree visibility.
+  void AddPerFrameCallback(PerFrameCallback callback);
 
   SkiaSurfaceProvider* SurfaceProviderForTesting() { return provider_; }
 
@@ -88,6 +95,8 @@ class UiScene {
 
   std::vector<UiElement*> all_elements_;
 
+  std::vector<PerFrameCallback> per_frame_callback_;
+
   SkiaSurfaceProvider* provider_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(UiScene);

chrome/browser/vr/ui_scene_creator.cc

@@ -1943,6 +1943,9 @@ void UiSceneCreator::CreateKeyboard() {
           base::Unretained(keyboard.get()))));
   VR_BIND_VISIBILITY(keyboard,
                      model->editing_input || model->editing_web_input);
+  scene_->AddPerFrameCallback(base::BindRepeating(
+      [](Keyboard* keyboard) { keyboard->AdvanceKeyboardFrameIfNeeded(); },
+      base::Unretained(keyboard.get())));
   scaler->AddChild(std::move(keyboard));
   visibility_control_root->AddChild(std::move(scaler));
   scene_->AddUiElement(k2dBrowsingRepositioner,