Omnibox selects all text on tap.

chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc

Index: chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
index 3ae833379671bfd6a86083e916a4d4174ae72df2..1797b4627ec66fbcd2f9d96b044ae8ca12310c9c 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
@@ -20,6 +20,12 @@
 #include "ui/base/test/ui_controls.h"
 #include "ui/views/controls/textfield/native_textfield_wrapper.h"
 
+#if defined(USE_AURA)
+#include "ui/aura/root_window.h"
+#include "ui/aura/root_window_host_delegate.h"
+#include "ui/aura/window.h"
+#endif // defined(USE_AURA)
+
 class OmniboxViewViewsTest : public InProcessBrowserTest {
  protected:
   OmniboxViewViewsTest() {}
@@ -45,25 +51,55 @@ class OmniboxViewViewsTest : public InProcessBrowserTest {
         ui_test_utils::SendMouseEventsSync(ui_controls::LEFT, ui_controls::UP));
   }
 
-  // Press and release the mouse in the omnibox at an offset from its origin.
-  // If |release_offset| differs from |press_offset|, the mouse will be moved
+  // Press and release the mouse at the specified locations.  If
+  // |release_offset| differs from |press_offset|, the mouse will be moved
   // between the press and release.
-  void ClickOmnibox(ui_controls::MouseButton button,
-                    const gfx::Vector2d& press_offset,
-                    const gfx::Vector2d& release_offset) {
-    const views::View* omnibox = BrowserView::GetBrowserViewForBrowser(
-        browser())->GetViewByID(VIEW_ID_OMNIBOX);
-    gfx::Point omnibox_origin = omnibox->GetBoundsInScreen().origin();
-    gfx::Point press_point = omnibox_origin + press_offset;
-    ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(press_point));
+  void Click(ui_controls::MouseButton button,
+             const gfx::Point& press_location,
+             const gfx::Point& release_location) {
+    ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(press_location));
     ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(button, ui_controls::DOWN));
 
-    gfx::Point release_point = omnibox_origin + release_offset;
-    if (release_point != press_point)
-      ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(release_point));
+    if (press_location != release_location)
+      ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(release_location));
     ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(button, ui_controls::UP));
   }
 
+#if defined(USE_AURA)
+  // Tap the center of the browser window.
+  void TapBrowserWindowCenter() {
+    aura::RootWindowHostDelegate* rwhd =
+        browser()->window()->GetNativeWindow()->GetRootWindow()->
+        GetDispatcher()->AsRootWindowHostDelegate();
+
+    gfx::Point center = BrowserView::GetBrowserViewForBrowser(
+        browser())->GetBoundsInScreen().CenterPoint();
+    ui::TouchEvent press(ui::ET_TOUCH_PRESSED, center,
+                         5, base::TimeDelta::FromMilliseconds(0));

[Peter Kasting, 2013/11/05 19:54:07]

Nit: You interleaved the press/release calls in Tap(), but not in this function.

[tdresser, 2013/11/05 20:18:20]

Done.

+    ui::TouchEvent release(ui::ET_TOUCH_RELEASED, center,
+                           5, base::TimeDelta::FromMilliseconds(50));
+
+    rwhd->OnHostTouchEvent(&press);
+    rwhd->OnHostTouchEvent(&release);
+  }
+
+  // Touch down and release at the specified locations.
+  void Tap(const gfx::Point& press_location,
+           const gfx::Point& release_location) {
+    aura::RootWindowHostDelegate* rwhd =
+        browser()->window()->GetNativeWindow()->GetRootWindow()->
+        GetDispatcher()->AsRootWindowHostDelegate();
+
+    ui::TouchEvent press(ui::ET_TOUCH_PRESSED, press_location,
+                         5, base::TimeDelta::FromMilliseconds(0));
+    rwhd->OnHostTouchEvent(&press);
+
+    ui::TouchEvent release(ui::ET_TOUCH_RELEASED, release_location,
+                           5, base::TimeDelta::FromMilliseconds(50));
+    rwhd->OnHostTouchEvent(&release);
+  }
+#endif // defined(USE_AURA)
+
  private:
   // InProcessBrowserTest:
   virtual void SetUpOnMainThread() OVERRIDE {
@@ -106,7 +142,11 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, SelectAllOnClick) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
   omnibox_view->SetUserText(ASCIIToUTF16("http://www.google.com/"));
-  const gfx::Vector2d click(40, 10);
+
+  const gfx::Rect omnibox_bounds = BrowserView::GetBrowserViewForBrowser(
+        browser())->GetViewByID(VIEW_ID_OMNIBOX)->GetBoundsInScreen();
+
+  const gfx::Point click_location = omnibox_bounds.CenterPoint();

[Peter Kasting, 2013/11/05 19:54:07]

Nit: Define this just above first use.

I'd probably move both |omnibox_bounds| and |click_location| down to just under
the "Clicking in the omnibox should take focus and select all text." comment,
and remove the blank lines around them.

[tdresser, 2013/11/05 20:18:20]

Done.

 
   // Take the focus away from the omnibox.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
@@ -114,7 +154,8 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, SelectAllOnClick) {
   EXPECT_FALSE(omnibox_view->IsSelectAll());
 
   // Clicking in the omnibox should take focus and select all text.
-  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, click));
+  ASSERT_NO_FATAL_FAILURE(Click(ui_controls::LEFT,
+                                click_location, click_location));
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
 
@@ -124,28 +165,82 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, SelectAllOnClick) {
   EXPECT_FALSE(omnibox_view->IsSelectAll());
 
   // Clicking in the omnibox again should take focus and select all text again.
-  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, click));
+  ASSERT_NO_FATAL_FAILURE(Click(ui_controls::LEFT,
+                                click_location, click_location));
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
 
   // Clicking another omnibox spot should keep focus but clear the selection.
   omnibox_view->SelectAll(false);
-  const gfx::Vector2d click_2(click.x() + 10, click.y());
-  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click_2, click_2));
+  const gfx::Point click2_location = omnibox_bounds.origin() +
+      gfx::Vector2d(omnibox_bounds.width() / 4, omnibox_bounds.height() / 4);
+  ASSERT_NO_FATAL_FAILURE(Click(ui_controls::LEFT,
+                                click2_location, click2_location));
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
 
   // Take the focus away and click in the omnibox again, but drag a bit before
   // releasing.  We should focus the omnibox but not select all of its text.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
-  const gfx::Vector2d release(click.x() + 10, click.y());
-  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, release));
+  ASSERT_NO_FATAL_FAILURE(Click(ui_controls::LEFT,
+                                click_location, click2_location));
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
 
   // Middle-clicking should not be handled by the omnibox.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
-  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::MIDDLE, click, click));
+  ASSERT_NO_FATAL_FAILURE(Click(ui_controls::MIDDLE,
+                                click_location, click_location));
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(omnibox_view->IsSelectAll());
+}
+
+#if defined(USE_AURA)
+IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, SelectAllOnTap) {
+  OmniboxView* omnibox_view = NULL;
+  ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
+  omnibox_view->SetUserText(ASCIIToUTF16("http://www.google.com/"));
+
+  const gfx::Rect omnibox_bounds = BrowserView::GetBrowserViewForBrowser(
+        browser())->GetViewByID(VIEW_ID_OMNIBOX)->GetBoundsInScreen();
+
+  const gfx::Point tap_location = omnibox_bounds.CenterPoint();

[Peter Kasting, 2013/11/05 19:54:07]

Nit: Same comments as above.

[tdresser, 2013/11/05 20:18:20]

Done.

+
+  // Take the focus away from the omnibox.
+  ASSERT_NO_FATAL_FAILURE(TapBrowserWindowCenter());
   EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
+
+  // Tapping in the omnibox should take focus and select all text.
+  ASSERT_NO_FATAL_FAILURE(Tap(tap_location, tap_location));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_TRUE(omnibox_view->IsSelectAll());
+
+  // Tapping in another view should clear focus and the selection.
+  ASSERT_NO_FATAL_FAILURE(TapBrowserWindowCenter());
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(omnibox_view->IsSelectAll());
+
+  // Tapping in the omnibox again should take focus and select all text again.
+  ASSERT_NO_FATAL_FAILURE(Tap(tap_location, tap_location));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_TRUE(omnibox_view->IsSelectAll());
+
+  // Tapping another omnibox spot should keep focus and selection.
+  omnibox_view->SelectAll(false);
+

[Peter Kasting, 2013/11/05 19:54:07]

Nit: This blank line and the one below aren't necessary (and you don't have them
in the similar location in the previous test)

[tdresser, 2013/11/05 20:18:20]

Done.

+  const gfx::Point tap2_location = omnibox_bounds.origin() +
+      gfx::Vector2d(omnibox_bounds.width() / 4, omnibox_bounds.height() / 4);
+
+  ASSERT_NO_FATAL_FAILURE(Tap(tap2_location, tap2_location));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_TRUE(omnibox_view->IsSelectAll());
+
+  // Take the focus away and tap in the omnibox again, but drag a bit before
+  // releasing.  We should focus the omnibox but not select all of its text.
+  ASSERT_NO_FATAL_FAILURE(TapBrowserWindowCenter());
+  ASSERT_NO_FATAL_FAILURE(Tap(tap_location, tap2_location));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(omnibox_view->IsSelectAll());
 }
+#endif // defined(USE_AURA)