VR: Prevent keyboard from showing with exit-VR prompt

If the exit-VR prompt shows while content is editing text, we should
hide the keyboard.  A side effect of this change is that, when the
keyboard disappears, it notifies content that this has happened, and
content loses focus.

In the case of autofill settings, this means that if the user chooses
autofill settings but dismisses the exit-VR prompt to return to their
page, that the input field will no longer be focused for edit (and the
keyboard won't be showing).  In the 2D case, the keyboard is also
hidden, but the cursor is still blinking in the field.  Therefore, VR is
not at parity with 2D, but this feels better than having the keyboard
persist through VR modal dialogs.

BUG=835483
R=asimjour

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: I753ec31273e2473a21ea5cd2c2dddf70c962f71a
Reviewed-on: https://chromium-review.googlesource.com/1030873Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554786}

chrome/browser/android/vr/vr_shell_gl.cc

@@ -1509,8 +1509,6 @@ 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

@@ -110,7 +110,7 @@ class ContentElementSceneTest : public UiTest {
  protected:
   std::unique_ptr<StrictMock<MockTextInputDelegate>> text_input_delegate_;
   std::unique_ptr<TestContentInputForwarder> input_forwarder_;
-  testing::Sequence in_sequence_;
+  testing::InSequence in_sequence_;
 };
 
 TEST_F(ContentElementSceneTest, WebInputFocus) {
@@ -119,21 +119,21 @@ TEST_F(ContentElementSceneTest, WebInputFocus) {
   // Set mock keyboard delegate.
   auto* kb = static_cast<Keyboard*>(scene_->GetUiElementByName(kKeyboard));
   auto kb_delegate = std::make_unique<StrictMock<MockKeyboardDelegate>>();
-  EXPECT_CALL(*kb_delegate, HideKeyboard()).InSequence(in_sequence_);
+  EXPECT_CALL(*kb_delegate, HideKeyboard());
   kb->SetKeyboardDelegate(kb_delegate.get());
 
   // Editing web input.
-  EXPECT_CALL(*text_input_delegate_, RequestFocus(_)).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, ShowKeyboard()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, OnBeginFrame()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, SetTransform(_)).InSequence(in_sequence_);
+  EXPECT_CALL(*text_input_delegate_, RequestFocus(_));
+  EXPECT_CALL(*kb_delegate, ShowKeyboard());
+  EXPECT_CALL(*kb_delegate, OnBeginFrame());
+  EXPECT_CALL(*kb_delegate, SetTransform(_));
   ui_->ShowSoftInput(true);
   EXPECT_TRUE(OnBeginFrame());
 
   // Giving content focus should tell the delegate the focued field's content.
-  EXPECT_CALL(*text_input_delegate_, UpdateInput(_)).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, OnBeginFrame()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, SetTransform(_)).InSequence(in_sequence_);
+  EXPECT_CALL(*text_input_delegate_, UpdateInput(_));
+  EXPECT_CALL(*kb_delegate, OnBeginFrame());
+  EXPECT_CALL(*kb_delegate, SetTransform(_));
   content->OnFocusChanged(true);
   EXPECT_TRUE(OnBeginFrame());
 
@@ -145,17 +145,16 @@ TEST_F(ContentElementSceneTest, WebInputFocus) {
   info.selection_end = 1;
   info.composition_start = 0;
   info.composition_end = 1;
-  EXPECT_CALL(*text_input_delegate_, UpdateInput(info))
-      .InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, OnBeginFrame()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, SetTransform(_)).InSequence(in_sequence_);
+  EXPECT_CALL(*text_input_delegate_, UpdateInput(info));
+  EXPECT_CALL(*kb_delegate, OnBeginFrame());
+  EXPECT_CALL(*kb_delegate, SetTransform(_));
   ui_->UpdateWebInputIndices(1, 1, 0, 1);
   EXPECT_TRUE(OnBeginFrame());
 
   // End editing.
-  EXPECT_CALL(*kb_delegate, HideKeyboard()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, OnBeginFrame()).InSequence(in_sequence_);
-  EXPECT_CALL(*kb_delegate, SetTransform(_)).InSequence(in_sequence_);
+  EXPECT_CALL(*kb_delegate, HideKeyboard());
+  EXPECT_CALL(*kb_delegate, OnBeginFrame());
+  EXPECT_CALL(*kb_delegate, SetTransform(_));
   ui_->ShowSoftInput(false);
   EXPECT_TRUE(OnBeginFrame());
 
@@ -163,9 +162,11 @@ TEST_F(ContentElementSceneTest, WebInputFocus) {
   EXPECT_FALSE(input_forwarder_->clear_focus_called());
   content->OnFocusChanged(false);
   EXPECT_TRUE(input_forwarder_->clear_focus_called());
+
   // OnBeginFrame on the keyboard delegate should be called despite of
   // visibility.
-  scene_->CallPerFrameCallbacks();
+  EXPECT_CALL(*kb_delegate, OnBeginFrame());
+  OnBeginFrame();
 }
 
 // Verify that we clear the model for the web input field when it loses focus to
@@ -179,8 +180,7 @@ TEST_F(ContentElementSceneTest, ClearWebInputInfoModel) {
   EXPECT_TRUE(OnBeginFrame());
 
   // Initial state gets pushed when the content element is focused.
-  EXPECT_CALL(*text_input_delegate_, UpdateInput(info))
-      .InSequence(in_sequence_);
+  EXPECT_CALL(*text_input_delegate_, UpdateInput(info));
   content->OnFocusChanged(true);
   EXPECT_TRUE(OnBeginFrame());
 
@@ -191,8 +191,7 @@ TEST_F(ContentElementSceneTest, ClearWebInputInfoModel) {
   // A cleared state gets pushed when the content element is focused. This is
   // needed because the user may have clicked another text field in the content,
   // so we shouldn't be pushing the stale state.
-  EXPECT_CALL(*text_input_delegate_, UpdateInput(TextInputInfo()))
-      .InSequence(in_sequence_);
+  EXPECT_CALL(*text_input_delegate_, UpdateInput(TextInputInfo()));
   content->OnFocusChanged(true);
   EXPECT_TRUE(OnBeginFrame());
 }

chrome/browser/vr/elements/keyboard.cc

@@ -99,20 +99,15 @@ void Keyboard::OnButtonUp(const gfx::PointF& 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() != kDirty)
+  // This is the keyboard's equivalent to OnBeginFrame(), but is separate
+  // because it must run on every frame - not just if the keyboard is visible.
+  if (!delegate_)
     return;
 
   delegate_->OnBeginFrame();
 }
 
 bool Keyboard::OnBeginFrame(const gfx::Transform& head_pose) {
-  if (!delegate_)
-    return false;
-
-  delegate_->OnBeginFrame();
   // We return false here because any visible changes to the keyboard, such as
   // hover effects and showing/hiding of the keyboard will be drawn by the
   // controller's dirtyness, so it's safe to assume no visual changes here.

chrome/browser/vr/ui_scene.cc

@@ -128,6 +128,12 @@ bool UiScene::OnBeginFrame(const base::TimeTicks& current_time,
     FrameLifecycle::set_phase(kUpdatedBindings);
   }
 
+  // Per-frame callbacks run every frame, always, as opposed to bindings, which
+  // run selectively based on element visibility.
+  for (auto callback : per_frame_callback_) {
+    callback.Run();
+  }
+
   {
     TRACE_EVENT0("gpu", "UiScene::OnBeginFrame.UpdateAnimationsAndOpacity");
 
@@ -167,12 +173,6 @@ 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() {
   TRACE_EVENT0("gpu", "UiScene::UpdateTextures");
   bool needs_redraw = false;

chrome/browser/vr/ui_scene.h

@@ -47,8 +47,6 @@ 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();
 

chrome/browser/vr/ui_scene_creator.cc

@@ -1883,9 +1883,9 @@ void UiSceneCreator::CreateKeyboard() {
             }
           },
           base::Unretained(keyboard.get()))));
-
-  VR_BIND_VISIBILITY(keyboard,
-                     model->editing_input || model->editing_web_input);
+  VR_BIND_VISIBILITY(
+      keyboard, (model->editing_input || model->editing_web_input) &&
+                    model->active_modal_prompt_type == kModalPromptTypeNone);
   scene_->AddPerFrameCallback(base::BindRepeating(
       [](Keyboard* keyboard) { keyboard->AdvanceKeyboardFrameIfNeeded(); },
       base::Unretained(keyboard.get())));