Deactivate omnibox touch editing on command execution

A Views textfield hides touch text selection handles on command
execution. Omnibox overrides command execution for some commands without
calling textfield's implementation; so, it needs to hide the handles for
those commands.
Also, introduced TextfieldTestApi class for accessing private members of
Textfield class in tests.

BUG=373532

Review URL: https://codereview.chromium.org/297733002

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

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

@@ -335,6 +335,10 @@ void OmniboxViewViews::OnNativeThemeChanged(const ui::NativeTheme* theme) {
 }
 
 void OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) {
+  // In the base class, touch text selection is deactivated when a command is
+  // executed. Since we are not always calling the base class implementation
+  // here, we need to deactivate touch text selection here, too.
+  DestroyTouchSelection();
   switch (command_id) {
     // These commands don't invoke the popup via OnBefore/AfterPossibleChange().
     case IDS_PASTE_AND_GO:

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

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 
+#include "base/command_line.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -21,7 +22,9 @@
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/test/ui_controls.h"
+#include "ui/base/ui_base_switches.h"
 #include "ui/events/event_processor.h"
+#include "ui/views/controls/textfield/textfield_test_api.h"
 
 class OmniboxViewViewsTest : public InProcessBrowserTest {
  protected:
@@ -289,3 +292,35 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, BackgroundIsOpaque) {
   ASSERT_TRUE(view);
   EXPECT_FALSE(view->GetRenderText()->background_is_transparent());
 }
+
+// Tests if executing a command hides touch editing handles.
+IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest,
+                       DeactivateTouchEditingOnExecuteCommand) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnableTouchEditing);
+
+  OmniboxView* view = NULL;
+  ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &view));
+  OmniboxViewViews* omnibox_view_views = static_cast<OmniboxViewViews*>(view);
+  views::TextfieldTestApi textfield_test_api(omnibox_view_views);
+
+  // Put a URL on the clipboard. It is written to the clipboard upon destruction
+  // of the writer.
+  {
+    ui::ScopedClipboardWriter clipboard_writer(
+        ui::Clipboard::GetForCurrentThread(),
+        ui::CLIPBOARD_TYPE_COPY_PASTE);
+    clipboard_writer.WriteURL(base::ASCIIToUTF16("http://www.example.com/"));
+  }
+
+  // Tap to activate touch editing.
+  gfx::Point omnibox_center =
+      omnibox_view_views->GetBoundsInScreen().CenterPoint();
+  Tap(omnibox_center, omnibox_center);
+  EXPECT_TRUE(textfield_test_api.touch_selection_controller());
+
+  // Execute a command and check if it deactivate touch editing. Paste & Go is
+  // chosen since it is specific to Omnibox and its execution wouldn't be
+  // delgated to the base Textfield class.
+  omnibox_view_views->ExecuteCommand(IDS_PASTE_AND_GO, ui::EF_NONE);
+  EXPECT_FALSE(textfield_test_api.touch_selection_controller());
+}

ui/views/controls/textfield/textfield.h

@@ -30,10 +30,6 @@
 
 namespace views {
 
-namespace test {
-class WidgetTestInteractive;
-}
-
 class MenuRunner;
 class Painter;
 class TextfieldController;
@@ -297,9 +293,7 @@ class VIEWS_EXPORT Textfield : public View,
   virtual base::string16 GetSelectionClipboardText() const;
 
  private:
-  friend class TextfieldTest;
-  friend class TouchSelectionControllerImplTest;
-  friend class test::WidgetTestInteractive;
+  friend class TextfieldTestApi;
 
   // Handles a request to change the value of this text field from software
   // using an accessibility API (typically automation software, screen readers

ui/views/controls/textfield/textfield_test_api.cc

+// Copyright 2014 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 "ui/views/controls/textfield/textfield_test_api.h"
+
+namespace views {
+
+TextfieldTestApi::TextfieldTestApi(Textfield* textfield)
+    : textfield_(textfield) {
+  DCHECK(textfield);
+}
+
+void TextfieldTestApi::UpdateContextMenu() {
+  textfield_->UpdateContextMenu();
+}
+
+gfx::RenderText* TextfieldTestApi::GetRenderText() const {
+  return textfield_->GetRenderText();
+}
+
+void TextfieldTestApi::CreateTouchSelectionControllerAndNotifyIt() {
+  textfield_->CreateTouchSelectionControllerAndNotifyIt();
+}
+
+void TextfieldTestApi::ResetTouchSelectionController() {
+  textfield_->touch_selection_controller_.reset();
+}
+
+}  // namespace views

ui/views/controls/textfield/textfield_test_api.h

+// Copyright 2014 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.
+
+#ifndef UI_VIEWS_CONTROLS_TEXTFIELD_TEXTFIELD_TEST_API_H_
+#define UI_VIEWS_CONTROLS_TEXTFIELD_TEXTFIELD_TEST_API_H_
+
+#include "ui/views/controls/textfield/textfield.h"
+
+namespace views {
+
+// Helper class to access internal state of Textfield in tests.
+class TextfieldTestApi {
+ public:
+  explicit TextfieldTestApi(Textfield* textfield);
+
+  void UpdateContextMenu();
+
+  gfx::RenderText* GetRenderText() const;
+
+  void CreateTouchSelectionControllerAndNotifyIt();
+
+  void ResetTouchSelectionController();
+
+  TextfieldModel* model() const { return textfield_->model_.get(); }
+
+  ui::MenuModel* context_menu_contents() const {
+    return textfield_->context_menu_contents_.get();
+  }
+
+  ui::TouchSelectionController* touch_selection_controller() const {
+    return textfield_->touch_selection_controller_.get();
+  }
+
+ private:
+  Textfield* textfield_;
+
+  DISALLOW_COPY_AND_ASSIGN(TextfieldTestApi);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_CONTROLS_TEXTFIELD_TEXTFIELD_TEST_API_H_

ui/views/controls/textfield/textfield_unittest.cc

@@ -31,6 +31,7 @@
 #include "ui/gfx/render_text.h"
 #include "ui/views/controls/textfield/textfield_controller.h"
 #include "ui/views/controls/textfield/textfield_model.h"
+#include "ui/views/controls/textfield/textfield_test_api.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/ime/mock_input_method.h"
 #include "ui/views/test/test_views_delegate.h"
@@ -197,6 +198,7 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController {
     container->AddChildView(textfield_);
     textfield_->SetBoundsRect(params.bounds);
     textfield_->set_id(1);
+    test_api_.reset(new TextfieldTestApi(textfield_));
 
     for (int i = 1; i < count; i++) {
       Textfield* textfield = new Textfield();
@@ -204,7 +206,7 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController {
       textfield->set_id(i + 1);
     }
 
-    model_ = textfield_->model_.get();
+    model_ = test_api_->model();
     model_->ClearEditHistory();
 
     input_method_ = new MockInputMethod();
@@ -216,12 +218,8 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController {
   }
 
   ui::MenuModel* GetContextMenuModel() {
-    textfield_->UpdateContextMenu();
-    return textfield_->context_menu_contents_.get();
-  }
-
-  ui::TouchSelectionController* GetTouchSelectionController() {
-    return textfield_->touch_selection_controller_.get();
+    test_api_->UpdateContextMenu();
+    return test_api_->context_menu_contents();
   }
 
  protected:
@@ -264,22 +262,22 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController {
   }
 
   int GetCursorPositionX(int cursor_pos) {
-    return textfield_->GetRenderText()->GetCursorBounds(
+    return test_api_->GetRenderText()->GetCursorBounds(
         gfx::SelectionModel(cursor_pos, gfx::CURSOR_FORWARD), false).x();
   }
 
   // Get the current cursor bounds.
   gfx::Rect GetCursorBounds() {
-    return textfield_->GetRenderText()->GetUpdatedCursorBounds();
+    return test_api_->GetRenderText()->GetUpdatedCursorBounds();
   }
 
   // Get the cursor bounds of |sel|.
   gfx::Rect GetCursorBounds(const gfx::SelectionModel& sel) {
-    return textfield_->GetRenderText()->GetCursorBounds(sel, true);
+    return test_api_->GetRenderText()->GetCursorBounds(sel, true);
   }
 
   gfx::Rect GetDisplayRect() {
-    return textfield_->GetRenderText()->display_rect();
+    return test_api_->GetRenderText()->display_rect();
   }
 
   // Mouse click on the point whose x-axis is |bound|'s x plus |x_offset| and
@@ -324,6 +322,7 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController {
   Widget* widget_;
 
   TestTextfield* textfield_;
+  scoped_ptr<TextfieldTestApi> test_api_;
   TextfieldModel* model_;
 
   // The string from Controller::ContentsChanged callback.
@@ -1903,7 +1902,7 @@ TEST_F(TextfieldTest, SelectionClipboard) {
 TEST_F(TextfieldTest, TouchSelectionAndDraggingTest) {
   InitTextfield();
   textfield_->SetText(ASCIIToUTF16("hello world"));
-  EXPECT_FALSE(GetTouchSelectionController());
+  EXPECT_FALSE(test_api_->touch_selection_controller());
   const int x = GetCursorPositionX(2);
   GestureEventForTest tap(ui::ET_GESTURE_TAP, x, 0, 1.0f, 0.0f);
   GestureEventForTest tap_down(ui::ET_GESTURE_TAP_DOWN, x, 0, 0.0f, 0.0f);
@@ -1912,18 +1911,18 @@ TEST_F(TextfieldTest, TouchSelectionAndDraggingTest) {
 
   // Tapping on the textfield should turn on the TouchSelectionController.
   textfield_->OnGestureEvent(&tap);
-  EXPECT_TRUE(GetTouchSelectionController());
+  EXPECT_TRUE(test_api_->touch_selection_controller());
 
   // Un-focusing the textfield should reset the TouchSelectionController
   textfield_->GetFocusManager()->ClearFocus();
-  EXPECT_FALSE(GetTouchSelectionController());
+  EXPECT_FALSE(test_api_->touch_selection_controller());
 
   // With touch editing enabled, long press should not show context menu.
   // Instead, select word and invoke TouchSelectionController.
   textfield_->OnGestureEvent(&tap_down);
   textfield_->OnGestureEvent(&long_press);
   EXPECT_STR_EQ("hello", textfield_->GetSelectedText());
-  EXPECT_TRUE(GetTouchSelectionController());
+  EXPECT_TRUE(test_api_->touch_selection_controller());
 
   // With touch drag drop enabled, long pressing in the selected region should
   // start a drag and remove TouchSelectionController.
@@ -1931,7 +1930,7 @@ TEST_F(TextfieldTest, TouchSelectionAndDraggingTest) {
   textfield_->OnGestureEvent(&tap_down);
   textfield_->OnGestureEvent(&long_press);
   EXPECT_STR_EQ("hello", textfield_->GetSelectedText());
-  EXPECT_FALSE(GetTouchSelectionController());
+  EXPECT_FALSE(test_api_->touch_selection_controller());
 
   // After disabling touch drag drop, long pressing again in the selection
   // region should not do anything.
@@ -1941,14 +1940,14 @@ TEST_F(TextfieldTest, TouchSelectionAndDraggingTest) {
   textfield_->OnGestureEvent(&tap_down);
   textfield_->OnGestureEvent(&long_press);
   EXPECT_STR_EQ("hello", textfield_->GetSelectedText());
-  EXPECT_TRUE(GetTouchSelectionController());
+  EXPECT_TRUE(test_api_->touch_selection_controller());
   EXPECT_TRUE(long_press.handled());
 }
 
 TEST_F(TextfieldTest, TouchScrubbingSelection) {
   InitTextfield();
   textfield_->SetText(ASCIIToUTF16("hello world"));
-  EXPECT_FALSE(GetTouchSelectionController());
+  EXPECT_FALSE(test_api_->touch_selection_controller());
 
   CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnableTouchEditing);
 
@@ -1982,7 +1981,7 @@ TEST_F(TextfieldTest, TouchScrubbingSelection) {
   // In the end, part of text should have been selected and handles should have
   // appeared.
   EXPECT_STR_EQ("ello ", textfield_->GetSelectedText());
-  EXPECT_TRUE(GetTouchSelectionController());
+  EXPECT_TRUE(test_api_->touch_selection_controller());
 }
 #endif
 

ui/views/touchui/touch_selection_controller_impl_unittest.cc

@@ -16,6 +16,7 @@
 #include "ui/gfx/rect.h"
 #include "ui/gfx/render_text.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/controls/textfield/textfield_test_api.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/touchui/touch_selection_controller_impl.h"
 #include "ui/views/views_touch_selection_controller_factory.h"
@@ -97,6 +98,8 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
     textfield_widget_->Show();
 
     textfield_->RequestFocus();
+
+    textfield_test_api_.reset(new TextfieldTestApi(textfield_));
   }
 
   void CreateWidget() {
@@ -116,7 +119,7 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
   }
 
   gfx::Rect GetCursorRect(const gfx::SelectionModel& sel) {
-    return textfield_->GetRenderText()->GetCursorBounds(sel, true);
+    return textfield_test_api_->GetRenderText()->GetCursorBounds(sel, true);
   }
 
   gfx::Point GetCursorPosition(const gfx::SelectionModel& sel) {
@@ -126,15 +129,15 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
 
   TouchSelectionControllerImpl* GetSelectionController() {
     return static_cast<TouchSelectionControllerImpl*>(
-        textfield_->touch_selection_controller_.get());
+        textfield_test_api_->touch_selection_controller());
   }
 
   void StartTouchEditing() {
-    textfield_->CreateTouchSelectionControllerAndNotifyIt();
+    textfield_test_api_->CreateTouchSelectionControllerAndNotifyIt();
   }
 
   void EndTouchEditing() {
-    textfield_->touch_selection_controller_.reset();
+    textfield_test_api_->ResetTouchSelectionController();
   }
 
   void SimulateSelectionHandleDrag(gfx::Point p, int selection_handle) {
@@ -183,7 +186,7 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
   }
 
   gfx::RenderText* GetRenderText() {
-    return textfield_->GetRenderText();
+    return textfield_test_api_->GetRenderText();
   }
 
   gfx::Point GetCursorHandleDragPoint() {
@@ -199,6 +202,7 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
   Widget* widget_;
 
   Textfield* textfield_;
+  scoped_ptr<TextfieldTestApi> textfield_test_api_;
   scoped_ptr<ViewsTouchSelectionControllerFactory> views_tsc_factory_;
 
  private:

ui/views/views.gyp

@@ -555,6 +555,8 @@
         '..',
       ],
       'sources': [
+        'controls/textfield/textfield_test_api.cc',
+        'controls/textfield/textfield_test_api.h',
         'corewm/tooltip_controller_test_helper.cc',
         'corewm/tooltip_controller_test_helper.h',
         'test/capture_tracking_view.cc',

ui/views/widget/widget_interactive_uitest.cc

@@ -21,6 +21,7 @@
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gl/gl_surface.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/controls/textfield/textfield_test_api.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/touchui/touch_selection_controller_impl.h"
 #include "ui/views/widget/widget.h"
@@ -170,24 +171,17 @@ class WidgetTestInteractive : public WidgetTest {
   }
 
  protected:
-  void ShowTouchSelectionQuickMenuImmediately(Textfield* textfield) {
-    DCHECK(textfield);
-    DCHECK(textfield->touch_selection_controller_);
-    TouchSelectionControllerImpl* controller =
-        static_cast<TouchSelectionControllerImpl*>(
-            textfield->touch_selection_controller_.get());
+  static void ShowQuickMenuImmediately(
+      TouchSelectionControllerImpl* controller) {
+    DCHECK(controller);
     if (controller->context_menu_timer_.IsRunning()) {
       controller->context_menu_timer_.Stop();
       controller->ContextMenuTimerFired();
     }
   }
 
-  bool TouchSelectionQuickMenuIsVisible(Textfield* textfield) {
-    DCHECK(textfield);
-    DCHECK(textfield->touch_selection_controller_);
-    TouchSelectionControllerImpl* controller =
-        static_cast<TouchSelectionControllerImpl*>(
-            textfield->touch_selection_controller_.get());
+  static bool IsQuickMenuVisible(TouchSelectionControllerImpl* controller) {
+    DCHECK(controller);
     return controller->context_menu_ && controller->context_menu_->visible();
   }
 };
@@ -768,16 +762,19 @@ TEST_F(WidgetTestInteractive, TouchSelectionQuickMenuIsNotActivated) {
   widget.Show();
   textfield->RequestFocus();
   textfield->SelectAll(true);
+  TextfieldTestApi textfield_test_api(textfield);
 
   RunPendingMessages();
 
   aura::test::EventGenerator generator(widget.GetNativeView()->GetRootWindow());
   generator.GestureTapAt(gfx::Point(10, 10));
-  ShowTouchSelectionQuickMenuImmediately(textfield);
+  ShowQuickMenuImmediately(static_cast<TouchSelectionControllerImpl*>(
+      textfield_test_api.touch_selection_controller()));
 
   EXPECT_TRUE(textfield->HasFocus());
   EXPECT_TRUE(widget.IsActive());
-  EXPECT_TRUE(TouchSelectionQuickMenuIsVisible(textfield));
+  EXPECT_TRUE(IsQuickMenuVisible(static_cast<TouchSelectionControllerImpl*>(
+      textfield_test_api.touch_selection_controller())));
 }
 
 namespace {