[omnibox] Break out SetCaretPos() method and enhance browser test

chrome/browser/ui/omnibox/omnibox_view_browsertest.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <stdio.h>
 
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 843 matching lines @@
   EXPECT_EQ(old_text.size(), end);
 
   // Delete the content
   ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_DELETE, 0));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
   omnibox_view->GetSelectionBounds(&start, &end);
   EXPECT_EQ(0U, start);
   EXPECT_EQ(0U, end);
   EXPECT_TRUE(omnibox_view->GetText().empty());
 
+  // Add a small amount of text.

[Peter Kasting, 2017/05/18 18:38:34]

Nit: "...to move the cursor to a nonzero value."  Then update the next comment
to "Check that RevertAll() resets the text and preserves the cursor position."

[Kevin Bailey, 2017/05/18 19:48:16]

Done.

+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_A, 0));
+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_B, 0));
+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_C, 0));
+
   // Check if RevertAll() can set text and cursor correctly.
   omnibox_view->RevertAll();
   EXPECT_FALSE(omnibox_view->IsSelectAll());
   EXPECT_EQ(old_text, omnibox_view->GetText());
   omnibox_view->GetSelectionBounds(&start, &end);
-  EXPECT_EQ(0U, start);
-  EXPECT_EQ(0U, end);
+  EXPECT_EQ(3U, start);
+  EXPECT_EQ(3U, end);
+
+  // Check if cursor gets reverted safely.

[Peter Kasting, 2017/05/18 18:38:34]

Nit: Maybe "Check that reverting clamps the cursor to the bounds of the new
text."

[Kevin Bailey, 2017/05/18 19:48:17]

Done.

+  // Move the cursor to the end.
+#if defined(OS_MACOSX)
+  // End doesn't work on Mac trybot.
+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_E, ui::EF_CONTROL_DOWN));
+#else
+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_END, 0));
+#endif
+  // Add a small amount of text.

[Peter Kasting, 2017/05/18 18:38:34]

Nit: "...to push the cursor past where the text end will be once we revert."

[Kevin Bailey, 2017/05/18 19:48:16]

Done.

+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_A, 0));
+  omnibox_view->RevertAll();
+  // Cursor should be no further than original text.
+  omnibox_view->GetSelectionBounds(&start, &end);
+  EXPECT_EQ(11U, start);
+  EXPECT_EQ(11U, end);
 }
 
 // Make sure the cursor position doesn't get set past the last character of
 // user input text when the URL is longer than the keyword.
 // (http://crbug.com/656209)
 IN_PROC_BROWSER_TEST_F(OmniboxViewTest, FocusSearchLongUrl) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
   ASSERT_GT(strlen(url::kAboutBlankURL), strlen(kSearchKeyword));
@@ skipping 1118 matching lines @@
 
   // Now Shift+Right should do nothing, and Shift+Left should reduce.
   // At the end, so Shift+Right should do nothing.
   ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_RIGHT, ui::EF_SHIFT_DOWN));
   EXPECT_EQ(2u, GetSelectionSize(omnibox_view));
 
   // And Left should reduce by one character.
   ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_LEFT, ui::EF_SHIFT_DOWN));
   EXPECT_EQ(1u, GetSelectionSize(omnibox_view));
 }