Merge 192453 "views: Do not open popup on Paste+Go."

> views: Do not open popup on Paste+Go.
> 
> BUG=214335
> TEST=Manually tested affected commands + browsertest.
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/13469011

TBR=jered@chromium.org
Review URL: https://codereview.chromium.org/13741023

git-svn-id: svn://svn.chromium.org/chrome/branches/1453/src@192881 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 77 insertions, 9 deletions

diff --git a/chrome/browser/ui/omnibox/omnibox_view.h b/chrome/browser/ui/omnibox/omnibox_view.h
index db3c828..531c328 100644
--- a/chrome/browser/ui/omnibox/omnibox_view.h
+++ b/chrome/browser/ui/omnibox/omnibox_view.h
@@ -178,9 +178,10 @@ class OmniboxView {
   // reset the user's original selection.
   virtual void OnRevertTemporaryText() = 0;
 
-  // Every piece of code that can change the edit should call these functions
-  // before and after the change.  These functions determine if anything
-  // meaningful changed, and do any necessary updating and notification.
+  // Checkpoints the current edit state before an operation that might trigger
+  // a new autocomplete run to open or modify the popup. Call this before
+  // user-initiated edit actions that trigger autocomplete, but *not* for
+  // automatic changes to the textfield that should not affect autocomplete.
   virtual void OnBeforePossibleChange() = 0;
   // OnAfterPossibleChange() returns true if there was a change that caused it
   // to call UpdatePopup().
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index ef77a34..596e0b9 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -754,16 +754,22 @@ bool OmniboxViewViews::HandlesCommand(int command_id) const {
 void OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) {
   switch (command_id) {
     case IDS_APP_PASTE:
+      OnBeforePossibleChange();
       OnPaste();
+      OnAfterPossibleChange();
       break;
     case IDS_PASTE_AND_GO:
+      // This method call is not wrapped by OnBeforePossibleChange() and
+      // OnAfterPossibleChange() to avoid opening the omnibox popup.
       model()->PasteAndGo(GetClipboardText());
       break;
     case IDC_COPY_URL:
       CopyURL();
       break;
     default:
+      OnBeforePossibleChange();
       command_updater()->ExecuteCommand(command_id);
+      OnAfterPossibleChange();
       break;
   }
 }
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
new file mode 100644
index 0000000..6fa4a4e
--- /dev/null
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
@@ -0,0 +1,51 @@
+// 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/app/chrome_command_ids.h"
+#include "chrome/browser/command_updater.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/omnibox/omnibox_views.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "grit/generated_resources.h"
+#include "ui/base/clipboard/clipboard.h"
+#include "ui/base/clipboard/scoped_clipboard_writer.h"
+#include "ui/views/controls/textfield/native_textfield_wrapper.h"
+
+typedef InProcessBrowserTest 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;
+  views::NativeTextfieldWrapper* native_textfield_wrapper =
+      static_cast<views::NativeTextfieldWrapper*>(
+          omnibox_view_views->GetNativeWrapperForTesting());
+  if (!native_textfield_wrapper)
+    return;
+
+  // Put an URL on the clipboard.
+  {
+    ui::ScopedClipboardWriter clipboard_writer(
+        ui::Clipboard::GetForCurrentThread(), ui::Clipboard::BUFFER_STANDARD);
+    clipboard_writer.WriteURL(ASCIIToUTF16("http://www.example.com/"));
+  }
+
+  // Paste and go.
+  native_textfield_wrapper->ExecuteTextCommand(IDS_PASTE_AND_GO);
+
+  // The popup should not be open.
+  EXPECT_FALSE(view->model()->popup_model()->IsOpen());
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index ce82e5b..532734b 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -242,6 +242,7 @@
         'browser/ui/views/menu_item_view_test.cc',
         'browser/ui/views/menu_model_adapter_test.cc',
         'browser/ui/views/message_center/web_notification_tray_win_browsertest.cc',
+        'browser/ui/views/omnibox/omnibox_view_views_browsertest.cc',
         'browser/ui/views/panels/panel_view_browsertest.cc',
         'browser/ui/views/ssl_client_certificate_selector_browsertest.cc',
         'browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc',
diff --git a/ui/views/controls/textfield/native_textfield_views.cc b/ui/views/controls/textfield/native_textfield_views.cc
index 838ef31..67264a7 100644
--- a/ui/views/controls/textfield/native_textfield_views.cc
+++ b/ui/views/controls/textfield/native_textfield_views.cc
@@ -751,37 +751,46 @@ void NativeTextfieldViews::ExecuteCommand(int command_id, int event_flags) {
   if (!IsCommandIdEnabled(command_id))
     return;
 
-  bool text_changed = false;
-  OnBeforeUserAction();
   TextfieldController* controller = textfield_->GetController();
   if (controller && controller->HandlesCommand(command_id)) {
     controller->ExecuteCommand(command_id, 0);
   } else {
+    bool text_changed = false;
     switch (command_id) {
       case IDS_APP_CUT:
+        OnBeforeUserAction();
         text_changed = Cut();
+        UpdateAfterChange(text_changed, text_changed);
+        OnAfterUserAction();
         break;
       case IDS_APP_COPY:
+        OnBeforeUserAction();
         Copy();
+        OnAfterUserAction();
         break;
       case IDS_APP_PASTE:
+        OnBeforeUserAction();
         text_changed = Paste();
+        UpdateAfterChange(text_changed, text_changed);
+        OnAfterUserAction();
         break;
       case IDS_APP_DELETE:
+        OnBeforeUserAction();
         text_changed = model_->Delete();
+        UpdateAfterChange(text_changed, text_changed);
+        OnAfterUserAction();
         break;
       case IDS_APP_SELECT_ALL:
+        OnBeforeUserAction();
         SelectAll(false);
+        UpdateAfterChange(false, true);
+        OnAfterUserAction();
         break;
       default:
         controller->ExecuteCommand(command_id, 0);
         break;
     }
   }
-
-  // The cursor must have changed if text changed during cut/paste/delete.
-  UpdateAfterChange(text_changed, text_changed);
-  OnAfterUserAction();
 }
 
 void NativeTextfieldViews::SetColor(SkColor value) {