Tweak Views Omnibox/Textfield; re-enable OmniboxViewTest.SelectAllOnClick.

Update test expectations for OmniboxViewTest.SelectAllOnClick:
(CrOS used to show/retain unfocused (grey highlight) Omnibox selections)
(My http://crrev.com/183239 made it save/restore selections like Windows)
Don't expect unfocused selections; check that clicking anew selects all.

Move the base omnibox click offset over to the right a little.
(fixes the test for OmniboxViewWin's slightly different hit-test area)
Enable previously USE_AURA-limited code on all TOOLKIT_VIEWS.

Update NativeTextfieldViews::OnMouseDragged to only handle left-click.
(it was incorrectly and incompletely handling right- and middle-drag)
Make NativeTextfieldViews::HandleMousePressEvent focus on right-click.
(it was selecting all and showing the context menu without taking focus)

Use ui_test_utils::IsViewFocused() instead of GetFocusView()->HasFocus(); 
Inline the last remaining GetFocusView call; some refactoring.

BUG=128556,139069
TEST=None; OmniboxViewTest.SelectAllOnClick passes.
R=derat@chromium.org,sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/13896002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193404 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/omnibox/omnibox_view_browsertest.cc

@@ -48,7 +48,7 @@
 #include <gtk/gtk.h>
 #endif
 
-#if defined(USE_AURA)
+#if defined(TOOLKIT_VIEWS)
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #endif
 
@@ -1264,13 +1264,9 @@ class OmniboxViewTest : public InProcessBrowserTest,
     EXPECT_EQ(old_text, omnibox_view->GetText());
   }
 
-#if defined(USE_AURA)
+#if defined(TOOLKIT_VIEWS)
   const BrowserView* GetBrowserView() const {
-    return static_cast<BrowserView*>(browser()->window());
-  }
-
-  const views::View* GetFocusView() const {
-    return GetBrowserView()->GetViewByID(VIEW_ID_OMNIBOX);
+    return BrowserView::GetBrowserViewForBrowser(browser());
   }
 
   // Move the mouse to the center of the browser window and left-click.
@@ -1283,23 +1279,24 @@ class OmniboxViewTest : public InProcessBrowserTest,
                     ui_controls::LEFT, ui_controls::UP));
   }
 
-  // Press and release the mouse in the focus view at an offset from its origin.
+  // 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 ClickFocusViewOrigin(ui_controls::MouseButton button,
-                            const gfx::Vector2d& press_offset,
-                            const gfx::Vector2d& release_offset) {
-    gfx::Point focus_view_origin = GetFocusView()->GetBoundsInScreen().origin();
-    gfx::Point press_point = focus_view_origin + press_offset;
+  void ClickOmnibox(ui_controls::MouseButton button,
+                    const gfx::Vector2d& press_offset,
+                    const gfx::Vector2d& release_offset) {
+    const views::View* omnibox = GetBrowserView()->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 = focus_view_origin  + release_offset;
+    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));
   }
-#endif  // defined(USE_AURA)
+#endif  // defined(TOOLKIT_VIEWS)
 };
 
 // Test if ctrl-* accelerators are workable in omnibox.
@@ -1632,72 +1629,55 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PasteReplacingAll) {
 }
 #endif  // defined(TOOLKIT_GTK)
 
-// TODO(derat): Enable on Windows: http://crbug.com/128556
-#if defined(USE_AURA)
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_SelectAllOnClick) {
+#if defined(TOOLKIT_VIEWS)
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, SelectAllOnClick) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   omnibox_view->SetUserText(ASCIIToUTF16("http://www.google.com/"));
-  const gfx::Vector2d kClickOffset(2, 2);
+  const gfx::Vector2d click(40, 10);
 
   // Take the focus away from the omnibox.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
-  EXPECT_FALSE(GetFocusView()->HasFocus());
 
-  // Click in the omnibox.  All of its text should be selected.
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(ui_controls::LEFT, kClickOffset, kClickOffset));
+  // Clicking in the omnibox should take focus and select all text.
+  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, click));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
-  EXPECT_TRUE(GetFocusView()->HasFocus());
 
-  // Ensure that all of the text is selected and then take the focus away.  The
-  // selection should persist.
-  omnibox_view->SelectAll(false);
-  EXPECT_TRUE(omnibox_view->IsSelectAll());
+  // Clicking in another view should clear focus and the selection.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
-  EXPECT_TRUE(omnibox_view->IsSelectAll());
-  EXPECT_FALSE(GetFocusView()->HasFocus());
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(omnibox_view->IsSelectAll());
 
-  // Clicking in the omnibox while some of its text is already selected should
-  // have the effect of re-selecting the text.
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(ui_controls::LEFT, kClickOffset, kClickOffset));
-  // The following expect fails starting with one of the cls (148415-148428),
-  // most likely the WebKit roll @148419.
-  // http://crbug.com/139069
+  // Clicking in the omnibox again should take focus and select all text again.
+  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click, click));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
-  EXPECT_TRUE(GetFocusView()->HasFocus());
 
-  // Click in a different spot in the omnibox.  It should keep the focus but
-  // lose the selection.
+  // Clicking another omnibox spot should keep focus but clear the selection.
   omnibox_view->SelectAll(false);
-  const gfx::Vector2d kSecondClickOffset(kClickOffset.x() + 10,
-                                         kClickOffset.y());
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(
-          ui_controls::LEFT, kSecondClickOffset, kSecondClickOffset));
+  const gfx::Vector2d click_2(click.x() + 10, click.y());
+  ASSERT_NO_FATAL_FAILURE(ClickOmnibox(ui_controls::LEFT, click_2, click_2));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_FALSE(omnibox_view->IsSelectAll());
-  EXPECT_TRUE(GetFocusView()->HasFocus());
 
   // 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 kReleaseOffset(kClickOffset.x() + 10, kClickOffset.y());
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(ui_controls::LEFT, kClickOffset, kReleaseOffset));
+  const gfx::Vector2d release(click.x() + 10, click.y());
+  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());
-  EXPECT_TRUE(GetFocusView()->HasFocus());
 
-  // Middle-clicking shouldn't select all the text either.
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(ui_controls::LEFT, kClickOffset, kClickOffset));
+  // Middle-clicking should not be handled by the omnibox.
   ASSERT_NO_FATAL_FAILURE(ClickBrowserWindowCenter());
-  ASSERT_NO_FATAL_FAILURE(
-      ClickFocusViewOrigin(ui_controls::MIDDLE, kClickOffset, kClickOffset));
+  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());
 }
-#endif  // defined(USE_AURA)
+#endif  // defined(TOOLKIT_VIEWS)
 
 IN_PROC_BROWSER_TEST_F(OmniboxViewTest, CopyURLToClipboard) {
   OmniboxView* omnibox_view = NULL;

ui/views/controls/textfield/native_textfield_views.cc

@@ -121,8 +121,10 @@ bool NativeTextfieldViews::ExceededDragThresholdFromLastClickLocation(
 bool NativeTextfieldViews::OnMouseDragged(const ui::MouseEvent& event) {
   // Don't adjust the cursor on a potential drag and drop, or if the mouse
   // movement from the last mouse click does not exceed the drag threshold.
-  if (initiating_drag_ || !ExceededDragThresholdFromLastClickLocation(event))
+  if (initiating_drag_ || !ExceededDragThresholdFromLastClickLocation(event) ||
+      !event.IsOnlyLeftMouseButton()) {
     return true;
+  }
 
   OnBeforeUserAction();
   // TODO: Remove once NativeTextfield implementations are consolidated to
@@ -1353,33 +1355,35 @@ void NativeTextfieldViews::TrackMouseClicks(const ui::MouseEvent& event) {
 }
 
 void NativeTextfieldViews::HandleMousePressEvent(const ui::MouseEvent& event) {
-  if (event.IsOnlyLeftMouseButton()) {
+  if (event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton())
     textfield_->RequestFocus();
 
-    initiating_drag_ = false;
-    bool can_drag = true;
+  if (!event.IsOnlyLeftMouseButton())
+    return;
 
-    switch (aggregated_clicks_) {
-      case 0:
-        if (can_drag && GetRenderText()->IsPointInSelection(event.location()))
-          initiating_drag_ = true;
-        else
-          MoveCursorTo(event.location(), event.IsShiftDown());
-        break;
-      case 1:
-        MoveCursorTo(event.location(), false);
-        model_->SelectWord();
-        OnCaretBoundsChanged();
-        break;
-      case 2:
-        model_->SelectAll(false);
-        OnCaretBoundsChanged();
-        break;
-      default:
-        NOTREACHED();
-    }
-    SchedulePaint();
+  initiating_drag_ = false;
+  bool can_drag = true;
+
+  switch (aggregated_clicks_) {
+    case 0:
+      if (can_drag && GetRenderText()->IsPointInSelection(event.location()))
+        initiating_drag_ = true;
+      else
+        MoveCursorTo(event.location(), event.IsShiftDown());
+      break;
+    case 1:
+      MoveCursorTo(event.location(), false);
+      model_->SelectWord();
+      OnCaretBoundsChanged();
+      break;
+    case 2:
+      model_->SelectAll(false);
+      OnCaretBoundsChanged();
+      break;
+    default:
+      NOTREACHED();
   }
+  SchedulePaint();
 }
 
 bool NativeTextfieldViews::ImeEditingAllowed() const {