VR: Fix autofill related keyboard bugs

Autofill exposed two keyboard input handling related bugs.
1) We weren't clearing the model capturing the web input field state after the
field lost focus. As a result, upon gaining focus the second time, we were
pushing the old state into the keyboard.
2) When the web input field indices change, we ask web contents for the new
text around the indices. This has a side effect of "OnWebInputIndicesChanged"
beinh called again. We were ignoring this second call in this case.
The bug was that we weren't clearing the bit that told us to ingore, so we would
fail to request the text state when we actually should have.

This CL also has a functional change that clicking on the autofill suggestion
doesn't hide the keyboard.

Bug: 831187
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: I04e448addf8e30a56afe98cb32630195f3065f67
Reviewed-on: https://chromium-review.googlesource.com/1012989Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Commit-Queue: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550811}

chrome/browser/vr/content_input_delegate.cc

@@ -194,6 +194,7 @@ void ContentInputDelegate::OnWebInputIndicesChanged(
       i.selection_end == selection_end &&
       i.composition_start == composition_start &&
       i.composition_end == composition_end) {
+    pending_text_input_info_ = TextInputInfo();
     return;
   }
 
@@ -207,6 +208,11 @@ void ContentInputDelegate::OnWebInputIndicesChanged(
       &ContentInputDelegate::OnWebInputTextChanged, base::Unretained(this)));
 }
 
+void ContentInputDelegate::ClearTextInputState() {
+  pending_text_input_info_ = TextInputInfo();
+  last_keyboard_edit_ = EditedText();
+}
+
 void ContentInputDelegate::OnWebInputTextChanged(const base::string16& text) {
   pending_text_input_info_.text = text;
   DCHECK(!update_state_callbacks_.empty());

chrome/browser/vr/content_input_delegate.h

@@ -106,6 +106,8 @@ class ContentInputDelegate {
     OnWebInputTextChanged(text);
   }
 
+  void ClearTextInputState();
+
  private:
   void UpdateGesture(const gfx::PointF& normalized_content_hit_point,
                      blink::WebGestureEvent& gesture);

chrome/browser/vr/elements/content_element_unittest.cc

@@ -168,6 +168,35 @@ TEST_F(ContentElementSceneTest, WebInputFocus) {
   scene_->CallPerFrameCallbacks();
 }
 
+// Verify that we clear the model for the web input field when it loses focus to
+// prevent updating the keyboard with the old state when it gains focus again.
+TEST_F(ContentElementSceneTest, ClearWebInputInfoModel) {
+  auto* content =
+      static_cast<ContentElement*>(scene_->GetUiElementByName(kContentQuad));
+  // Initial state.
+  TextInputInfo info(base::ASCIIToUTF16("asdfg"));
+  model_->web_input_text_field_info.current = info;
+  EXPECT_TRUE(OnBeginFrame());
+
+  // Initial state gets pushed when the content element is focused.
+  EXPECT_CALL(*text_input_delegate_, UpdateInput(info))
+      .InSequence(in_sequence_);
+  content->OnFocusChanged(true);
+  EXPECT_TRUE(OnBeginFrame());
+
+  // Unfocus the content element.
+  content->OnFocusChanged(false);
+  EXPECT_TRUE(OnBeginFrame());
+
+  // 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_);
+  content->OnFocusChanged(true);
+  EXPECT_TRUE(OnBeginFrame());
+}
+
 class ContentElementInputEditingTest : public UiTest {
  public:
   void SetUp() override {
@@ -270,4 +299,30 @@ TEST_F(ContentElementInputEditingTest, IndicesUpdated) {
                                      base::UTF8ToUTF16("q"), 1));
 }
 
+// This test verifies that we request the text state when the indices are
+// updated the second time the indices are changed. That is
+// OnWebInputIndicesChanged is called as a side effect of requesting the text
+// state, and we should ignore that call, but any call after that should still
+// request the text state, because for example, it may be for a different text
+// field.
+TEST_F(ContentElementInputEditingTest, PendingRequestStateCleared) {
+  content_delegate_->OnWebInputIndicesChanged(
+      2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {}));
+  EXPECT_TRUE(input_forwarder_->text_state_requested());
+
+  input_forwarder_->Reset();
+  content_delegate_->OnWebInputIndicesChanged(
+      2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {}));
+  // We don't request the curent text state becasuse OnWebInputIndicesChanged
+  // gets called as a side-effect of requesting the state above.
+  EXPECT_FALSE(input_forwarder_->text_state_requested());
+
+  input_forwarder_->Reset();
+  content_delegate_->OnWebInputIndicesChanged(
+      2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {}));
+  // We should request the current text state this time because the call to
+  // OnWebInputIndicesChanged this time can be for a different reason.
+  EXPECT_TRUE(input_forwarder_->text_state_requested());
+}
+
 }  // namespace vr

chrome/browser/vr/ui_scene_creator.cc

@@ -682,6 +682,10 @@ std::unique_ptr<UiElement> CreateHostedUi(
       kNone, kPhaseForeground, content_input_delegate, base::DoNothing());
   hosted_ui->SetSize(kContentWidth * kHostedUiWidthRatio,
                      kContentHeight * kHostedUiHeightRatio);
+  // The hosted UI doesn't steal focus so that clikcing on an autofill
+  // suggestion doesn't hide the keyboard. We will probably need to change this
+  // when we support the keyboard on native UI elements.
+  hosted_ui->set_focusable(false);
   hosted_ui->SetVisible(false);
   hosted_ui->set_opacity_when_visible(1.0);
   hosted_ui->set_requires_layout(false);
@@ -1189,14 +1193,16 @@ void UiSceneCreator::CreateContentQuad() {
                           base::Unretained(browser_)));
   EventHandlers event_handlers;
   event_handlers.focus_change = base::BindRepeating(
-      [](Model* model, ContentElement* e, bool focused) {
-        if (focused) {
-          e->UpdateInput(model->web_input_text_field_info);
-        } else {
-          e->UpdateInput(EditedText());
+      [](Model* model, ContentElement* e, ContentInputDelegate* delegate,
+         bool focused) {
+        if (!focused) {
+          model->web_input_text_field_info = EditedText();
+          delegate->ClearTextInputState();
         }
+        e->UpdateInput(model->web_input_text_field_info);
       },
-      model_, base::Unretained(main_content.get()));
+      model_, base::Unretained(main_content.get()),
+      base::Unretained(content_input_delegate_));
   main_content->set_event_handlers(event_handlers);
   main_content->SetName(kContentQuad);
   main_content->set_hit_testable(true);