Omnibox selects all text on tap.

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

 // Copyright (c) 2013 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/ui/views/omnibox/omnibox_view_views.h"
 
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_views.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "grit/generated_resources.h"
+#include "ui/aura/root_window.h"

[Peter Kasting, 2013/11/04 21:23:07]

Don't we need to guard Aura-specific includes and code with the appropriate
preprocessor directive?  After all, one can still build views without Aura.

[tdresser, 2013/11/05 15:28:00]

Done. Are you aware of any bots which build views without Aura?
I can't compile this configuration on linux, and it sounds like all windows bots
now use Aura.

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

We've switched the bots over to not doing this, but it's still possible for
engineers to build this way.  That will probably change over the next several
months.

+#include "ui/aura/root_window_host_delegate.h"
+#include "ui/aura/window.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/test/ui_controls.h"
 #include "ui/views/controls/textfield/native_textfield_wrapper.h"
 
 class OmniboxViewViewsTest : public InProcessBrowserTest {
  protected:
   OmniboxViewViewsTest() {}
 
   static void GetOmniboxViewForBrowser(const Browser* browser,
@@ skipping 10 matching lines @@
   void ClickBrowserWindowCenter() {
     ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(
         BrowserView::GetBrowserViewForBrowser(
             browser())->GetBoundsInScreen().CenterPoint()));
     ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(ui_controls::LEFT,
                                                    ui_controls::DOWN));
     ASSERT_TRUE(
         ui_test_utils::SendMouseEventsSync(ui_controls::LEFT, ui_controls::UP));
   }
 
+  // Tap the center of the browser window.
+  void TapBrowserWindowCenter() {
+    gfx::Point center = BrowserView::GetBrowserViewForBrowser(
+        browser())->GetBoundsInScreen().CenterPoint();
+    ui::TouchEvent press(ui::ET_TOUCH_PRESSED, center,
+                         5, base::TimeDelta::FromMilliseconds(0));
+    ui::TouchEvent release(ui::ET_TOUCH_RELEASED, center,
+                           5, base::TimeDelta::FromMilliseconds(50));
+
+    rwhd_->OnHostTouchEvent(&press);

[Peter Kasting, 2013/11/04 21:23:07]

Tiny nit: It may make slightly more sense to interleave:

    ui::TouchEvent press(...);
    rwhd_->OnHostTouchEvent(&press);

    ui::TouchEvent release(...);
    rwhd_->OnHostTouchEvent(&release);

(2 places)

[tdresser, 2013/11/05 15:28:00]

Done.

+    rwhd_->OnHostTouchEvent(&release);
+  }
+
   // 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
   // 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));
     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));
     ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(button, ui_controls::UP));
   }
 
+  void TapOmnibox(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;

[Peter Kasting, 2013/11/04 21:23:07]

Nit: I'd probably inline these two temps into the next two statements.

[tdresser, 2013/11/05 15:28:00]

Done.

+    gfx::Point release_point = omnibox_origin + release_offset;
+
+    ui::TouchEvent press(ui::ET_TOUCH_PRESSED, press_point,
+                         5, base::TimeDelta::FromMilliseconds(0));
+    ui::TouchEvent release(ui::ET_TOUCH_RELEASED, release_point,
+                           5, base::TimeDelta::FromMilliseconds(50));
+
+    rwhd_->OnHostTouchEvent(&press);
+    rwhd_->OnHostTouchEvent(&release);
+  }
+
  private:
   // InProcessBrowserTest:
   virtual void SetUpOnMainThread() OVERRIDE {
+    rwhd_ = browser()->window()->GetNativeWindow()->GetRootWindow()->
+        GetDispatcher()->AsRootWindowHostDelegate();
+
     ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
     chrome::FocusLocationBar(browser());
     ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   }
 
+  aura::RootWindowHostDelegate* rwhd_;

[Peter Kasting, 2013/11/04 21:23:07]

Nit: Does this need to be a member?  Can't we just calculate it in the two
functions that need it?

[tdresser, 2013/11/05 15:28:00]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(OmniboxViewViewsTest);
 };
 
 IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, PasteAndGoDoesNotLeavePopupOpen) {
   OmniboxView* view = browser()->window()->GetLocationBar()->GetLocationEntry();
   OmniboxViewViews* omnibox_view_views = GetOmniboxViewViews(view);
   // This test is only relevant when OmniboxViewViews is present and is using
   // the native textfield wrapper.
   if (!omnibox_view_views)
     return;
@@ skipping 57 matching lines @@
   ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, release));
   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));
   EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
 }
+
+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::Vector2d tap(40, 10);

[Peter Kasting, 2013/11/04 21:23:07]

Could this offset be outside the omnibox in a future omnibox update or on any
particular platforms?  Maybe instead of hardcoding 40,10 you should find the
omnibox center point or something.  Similar concerns apply to the other vectors
below.

Nit: |tap_offset| would be a better name than |tap|.

[tdresser, 2013/11/05 15:28:00]

Done. I changed this in the click test as well.

+
+  // 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(TapOmnibox(tap, tap));
+  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(TapOmnibox(tap, tap));
+  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);
+  const gfx::Vector2d tap_2(tap.x() + 10, tap.y());
+  ASSERT_NO_FATAL_FAILURE(TapOmnibox(tap_2, tap_2));
+  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());
+  const gfx::Vector2d release(tap.x() + 50, tap.y());
+  ASSERT_NO_FATAL_FAILURE(TapOmnibox(tap, release));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(omnibox_view->IsSelectAll());
+}