VR: Added a test for clamping composition and selection indices

Also moved the relavant tests from content_element_unittest.cc to
text_input_info_unittest.cc

This is a post-commit followup to
https://chromium-review.googlesource.com/c/chromium/src/+/933701

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

chrome/browser/vr/BUILD.gn

@@ -262,6 +262,7 @@ test("vr_common_unittests") {
     "elements/vector_icon_unittest.cc",
     "elements/viewport_aware_root_unittest.cc",
     "fps_meter_unittest.cc",
+    "model/text_input_info_unittest.cc",
     "pose_util_unittest.cc",
     "service/vr_device_manager_unittest.cc",
     "sliding_average_unittest.cc",

chrome/browser/vr/elements/content_element_unittest.cc

@@ -199,65 +199,6 @@ class ContentElementInputEditingTest : public UiTest {
   ContentElement* content_;
 };
 
-// Test that the diff between the current and previous edited text is calculated
-// correctly.
-TEST_F(ContentElementInputEditingTest, CommitDiff) {
-  // Add a character.
-  SetInput(GetInputInfo("a", 1, 1), GetInputInfo("", 0, 0));
-  auto edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("a"), 1));
-
-  // Add more characters.
-  SetInput(GetInputInfo("asdf", 4, 4), GetInputInfo("a", 1, 1));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("sdf"), 3));
-
-  // Delete a character.
-  SetInput(GetInputInfo("asd", 3, 3), GetInputInfo("asdf", 4, 4));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::DELETE_TEXT,
-                                     base::UTF8ToUTF16(""), -1));
-
-  // Add characters while the cursor is not at the end.
-  SetInput(GetInputInfo("asqwed", 5, 5), GetInputInfo("asd", 2, 2));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("qwe"), 3));
-}
-
-TEST_F(ContentElementInputEditingTest, CommitDiffWithSelection) {
-  // There was a selection and the new text is shorter than the selection text.
-  SetInput(GetInputInfo("This a text", 6, 6),
-           GetInputInfo("This is text", 5, 7));
-  auto edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("a"), 1));
-
-  // There was a selection and the new text is longer than the selection text.
-  // This could happen when the user clicks on a keyboard suggestion.
-  SetInput(GetInputInfo("This was the text", 12, 12),
-           GetInputInfo("This is text", 5, 7));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("was the"), 7));
-  // There was a selection and the new text is of the same length as the
-  // selection.
-  SetInput(GetInputInfo("This ha text", 7, 7),
-           GetInputInfo("This is text", 5, 7));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("ha"), 2));
-}
-
 TEST_F(ContentElementInputEditingTest, IndicesUpdated) {
   // If the changed indices match with that from the last keyboard edit, the
   // given callback is triggered right away.
@@ -312,56 +253,4 @@ TEST_F(ContentElementInputEditingTest, IndicesUpdated) {
                                      base::UTF8ToUTF16("q"), 1));
 }
 
-TEST_F(ContentElementInputEditingTest, CompositionDiff) {
-  // Start composition
-  SetInput(GetInputInfo("a", 1, 1, 0, 1), GetInputInfo("", 0, 0));
-  auto edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
-                                     base::UTF8ToUTF16("a"), 1));
-
-  // Add more characters.
-  SetInput(GetInputInfo("asdf", 4, 4, 0, 4), GetInputInfo("a", 1, 1, 0, 1));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
-                                     base::UTF8ToUTF16("asdf"), 3));
-
-  // Delete a few characters.
-  SetInput(GetInputInfo("as", 2, 2, 0, 2), GetInputInfo("asdf", 4, 4, 0, 4));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 1u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
-                                     base::UTF8ToUTF16("as"), -2));
-
-  // Finish composition.
-  SetInput(GetInputInfo("as ", 3, 3, -1, -1), GetInputInfo("as", 2, 2, 0, 2));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 2u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
-  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("as "), 3));
-
-  // Finish composition, but the text is different. This could happen when the
-  // user hits a suggestion that's different from the current composition, but
-  // has the same length.
-  SetInput(GetInputInfo("lk", 2, 2, -1, -1), GetInputInfo("as", 2, 2, 0, 2));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 2u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
-  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("lk"), 2));
-
-  // Finish composition, but the new text is shorter than the previous
-  // composition. This could happen when the user hits a suggestion that's
-  // shorter than the text they were composing.
-  SetInput(GetInputInfo("hi hello", 2, 2, -1, -1),
-           GetInputInfo("hii hello", 3, 3, 0, 3));
-  edits = input_forwarder_->edits();
-  EXPECT_EQ(edits.size(), 2u);
-  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
-  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
-                                     base::UTF8ToUTF16("hi"), 2));
-}
-
 }  // namespace vr

chrome/browser/vr/model/text_input_info.cc

@@ -103,7 +103,7 @@ void TextInputInfo::ClampIndices() {
     selection_end = selection_start;
   composition_start = std::min(composition_start, len);
   composition_end = std::min(composition_end, len);
-  if (composition_end < composition_start) {
+  if (composition_end <= composition_start) {
     composition_start = kDefaultCompositionIndex;
     composition_end = kDefaultCompositionIndex;
   }

chrome/browser/vr/model/text_input_info_unittest.cc

+// Copyright 2018 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/model/text_input_info.h"
+
+#include <memory>
+
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace vr {
+
+class TextInputInfoTest : public testing::Test {
+ protected:
+  TextInputInfo Text(const std::string& text,
+                     int selection_start,
+                     int selection_end,
+                     int composition_start,
+                     int composition_end) {
+    return TextInputInfo(base::UTF8ToUTF16(text), selection_start,
+                         selection_end, composition_start, composition_end);
+  }
+
+  TextInputInfo Text(const std::string& text,
+                     int selection_start,
+                     int selection_end) {
+    return Text(text, selection_start, selection_end, -1, -1);
+  }
+};
+
+TEST(TextInputInfo, Clamping) {
+  // Out of bounds indices.
+  auto info = TextInputInfo(base::UTF8ToUTF16("hi"), 4, 4, 4, 4);
+  auto info_expected = TextInputInfo(base::UTF8ToUTF16("hi"), 2, 2, -1, -1);
+  EXPECT_EQ(info, info_expected);
+
+  // Invalid indices.
+  info = TextInputInfo(base::UTF8ToUTF16("hi"), 4, 2, 2, 1);
+  info_expected = TextInputInfo(base::UTF8ToUTF16("hi"), 2, 2, -1, -1);
+  EXPECT_EQ(info, info_expected);
+}
+
+// Test that the diff between the current and previous edited text is calculated
+// correctly.
+TEST_F(TextInputInfoTest, CommitDiff) {
+  // Add a character.
+  auto edits = EditedText(Text("a", 1, 1), Text("", 0, 0)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("a"), 1));
+
+  // Add more characters.
+  edits = EditedText(Text("asdf", 4, 4), Text("a", 1, 1)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("sdf"), 3));
+
+  // Delete a character.
+  edits = EditedText(Text("asd", 3, 3), Text("asdf", 4, 4)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::DELETE_TEXT,
+                                     base::UTF8ToUTF16(""), -1));
+
+  // Add characters while the cursor is not at the end.
+  edits = EditedText(Text("asqwed", 5, 5), Text("asd", 2, 2)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("qwe"), 3));
+}
+
+TEST_F(TextInputInfoTest, CommitDiffWithSelection) {
+  // There was a selection and the new text is shorter than the selection text.
+  auto edits = EditedText(Text("This a text", 6, 6), Text("This is text", 5, 7))
+                   .GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("a"), 1));
+
+  // There was a selection and the new text is longer than the selection text.
+  // This could happen when the user clicks on a keyboard suggestion.
+  edits =
+      EditedText(Text("This was the text", 12, 12), Text("This is text", 5, 7))
+          .GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("was the"), 7));
+  // There was a selection and the new text is of the same length as the
+  // selection.
+  edits = EditedText(Text("This ha text", 7, 7), Text("This is text", 5, 7))
+              .GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("ha"), 2));
+}
+
+TEST_F(TextInputInfoTest, CompositionDiff) {
+  // Start composition
+  auto edits = EditedText(Text("a", 1, 1, 0, 1), Text("", 0, 0)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
+                                     base::UTF8ToUTF16("a"), 1));
+
+  // Add more characters.
+  edits = EditedText(Text("asdf", 4, 4, 0, 4), Text("a", 1, 1, 0, 1)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
+                                     base::UTF8ToUTF16("asdf"), 3));
+
+  // Delete a few characters.
+  edits =
+      EditedText(Text("as", 2, 2, 0, 2), Text("asdf", 4, 4, 0, 4)).GetDiff();
+  EXPECT_EQ(edits.size(), 1u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::SET_COMPOSING_TEXT,
+                                     base::UTF8ToUTF16("as"), -2));
+
+  // Finish composition.
+  edits =
+      EditedText(Text("as ", 3, 3, -1, -1), Text("as", 2, 2, 0, 2)).GetDiff();
+  EXPECT_EQ(edits.size(), 2u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
+  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("as "), 3));
+
+  // Finish composition, but the text is different. This could happen when the
+  // user hits a suggestion that's different from the current composition, but
+  // has the same length.
+  edits =
+      EditedText(Text("lk", 2, 2, -1, -1), Text("as", 2, 2, 0, 2)).GetDiff();
+  EXPECT_EQ(edits.size(), 2u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
+  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("lk"), 2));
+
+  // Finish composition, but the new text is shorter than the previous
+  // composition. This could happen when the user hits a suggestion that's
+  // shorter than the text they were composing.
+  edits =
+      EditedText(Text("hi hello", 2, 2, -1, -1), Text("hii hello", 3, 3, 0, 3))
+          .GetDiff();
+  EXPECT_EQ(edits.size(), 2u);
+  EXPECT_EQ(edits[0], TextEditAction(TextEditActionType::CLEAR_COMPOSING_TEXT));
+  EXPECT_EQ(edits[1], TextEditAction(TextEditActionType::COMMIT_TEXT,
+                                     base::UTF8ToUTF16("hi"), 2));
+}
+
+}  // namespace vr