Re-enable disabled tests in chrome/browser/ui/omnibox/.

These passed 50 times in a row on Windows.

This converts from watching NOTIFICATION_NAV_ENTRY_COMMIT to using a navigation
observer in hopes of fixing observed flakiness in EnterToSearch.

Bug: 69433, 133341, 133344, 133347, 133355, 133360, 146953, 408634, 431575, 751031, 751543, 823532, 841195
Change-Id: Ia8ce63c8a9aca1ea415fb99756b08409cbc8369c
Reviewed-on: https://chromium-review.googlesource.com/c/1332257
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607713}

chrome/browser/ui/omnibox/omnibox_view_browsertest.cc

@@ -49,6 +49,7 @@
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "net/dns/mock_host_resolver.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
@@ -204,19 +205,22 @@ class OmniboxViewTest : public InProcessBrowserTest,
       ASSERT_NO_FATAL_FAILURE(SendKey(*keys, 0));
   }
 
-  bool SendKeyAndWait(const Browser* browser,
-                      ui::KeyboardCode key,
-                      int modifiers,
-                      int type,
-                      const content::NotificationSource& source)
-                          WARN_UNUSED_RESULT {
+  bool ExpectBrowserClosed(const Browser* browser,
+                           ui::KeyboardCode key,
+                           int modifiers) WARN_UNUSED_RESULT {
     return ui_test_utils::SendKeyPressAndWait(
-        browser, key,
-        (modifiers & ui::EF_CONTROL_DOWN) != 0,
+        browser, key, (modifiers & ui::EF_CONTROL_DOWN) != 0,
         (modifiers & ui::EF_SHIFT_DOWN) != 0,
         (modifiers & ui::EF_ALT_DOWN) != 0,
         (modifiers & ui::EF_COMMAND_DOWN) != 0,
-        type, source);
+        chrome::NOTIFICATION_BROWSER_CLOSED, content::Source<Browser>(browser));
+  }
+
+  void NavigateExpectUrl(const GURL& url, int modifiers = 0) {
+    content::TestNavigationObserver observer(url);
+    observer.WatchExistingWebContents();
+    ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_RETURN, modifiers));
+    observer.WaitForNavigationFinished();
   }
 
   void WaitForTabOpenOrCloseForBrowser(const Browser* browser,
@@ -401,10 +405,7 @@ class OmniboxViewTest : public InProcessBrowserTest,
 };
 
 // Test if ctrl-* accelerators are workable in omnibox.
-// See http://crbug.com/19193: omnibox blocks ctrl-* commands
-//
-// Flaky on interactive tests (dbg), http://crbug.com/69433
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_BrowserAccelerators) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, BrowserAccelerators) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -457,21 +458,11 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_BrowserAccelerators) {
 
 #if !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
   // Try alt-f4 to close the browser.
-  ASSERT_TRUE(SendKeyAndWait(
-      browser(), ui::VKEY_F4, ui::EF_ALT_DOWN,
-      chrome::NOTIFICATION_BROWSER_CLOSED,
-      content::Source<Browser>(browser())));
+  ASSERT_TRUE(ExpectBrowserClosed(browser(), ui::VKEY_F4, ui::EF_ALT_DOWN));
 #endif
 }
 
-// Fails on Linux.  http://crbug.com/408634
-#if defined(OS_LINUX)
-#define MAYBE_PopupAccelerators DISABLED_PopupAccelerators
-#else
-#define MAYBE_PopupAccelerators PopupAccelerators
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PopupAccelerators) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, PopupAccelerators) {
   // Create a popup.
   Browser* popup = CreateBrowserForPopup(browser()->profile());
   ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(popup));
@@ -486,9 +477,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PopupAccelerators) {
   // This piece of code doesn't work on Mac, because the Browser object won't
   // be destroyed before finishing the current message loop iteration, thus
   // No BROWSER_CLOSED notification will be sent.
-  ASSERT_TRUE(SendKeyAndWait(
-      popup, ui::VKEY_W, ui::EF_CONTROL_DOWN,
-      chrome::NOTIFICATION_BROWSER_CLOSED, content::Source<Browser>(popup)));
+  ASSERT_TRUE(ExpectBrowserClosed(popup, ui::VKEY_W, ui::EF_CONTROL_DOWN));
 
   // Create another popup.
   popup = CreateBrowserForPopup(browser()->profile());
@@ -514,20 +503,11 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PopupAccelerators) {
 
 #if !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
   // Try alt-f4 to close the popup.
-  ASSERT_TRUE(SendKeyAndWait(
-      popup, ui::VKEY_F4, ui::EF_ALT_DOWN,
-      chrome::NOTIFICATION_BROWSER_CLOSED, content::Source<Browser>(popup)));
+  ASSERT_TRUE(ExpectBrowserClosed(popup, ui::VKEY_F4, ui::EF_ALT_DOWN));
 #endif
 }
 
-// http://crbug.com/133341
-#if defined(OS_LINUX)
-#define MAYBE_BackspaceInKeywordMode DISABLED_BackspaceInKeywordMode
-#else
-#define MAYBE_BackspaceInKeywordMode BackspaceInKeywordMode
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_BackspaceInKeywordMode) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, BackspaceInKeywordMode) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -581,13 +561,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_BackspaceInKeywordMode) {
             UTF16ToUTF8(omnibox_view->GetText()));
 }
 
-// Flaky on Windows and Linux. http://crbug.com/751543
-#if defined(OS_WIN) || defined(OS_LINUX)
-#define MAYBE_Escape DISABLED_Escape
-#else
-#define MAYBE_Escape Escape
-#endif
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_Escape) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, Escape) {
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIHistoryURL));
   chrome::FocusLocationBar(browser());
 
@@ -608,13 +582,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_Escape) {
   EXPECT_TRUE(omnibox_view->IsSelectAll());
 }
 
-#if defined(OS_LINUX)
-#define MAYBE_DesiredTLD DISABLED_DesiredTLD
-#else
-#define MAYBE_DesiredTLD DesiredTLD
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DesiredTLD) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DesiredTLD) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   OmniboxPopupModel* popup_model = omnibox_view->model()->popup_model();
@@ -627,26 +595,14 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DesiredTLD) {
   ASSERT_NO_FATAL_FAILURE(SendKeySequence(kKeys));
   ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
   ASSERT_TRUE(popup_model->IsOpen());
+
   // ctrl-Enter triggers desired_tld feature, thus www.bar.com shall be
   // opened.
-  ASSERT_TRUE(SendKeyAndWait(browser(), ui::VKEY_RETURN, ui::EF_CONTROL_DOWN,
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<content::NavigationController>(
-          &browser()->tab_strip_model()->GetActiveWebContents()->
-              GetController())));
-
-  GURL url = browser()->tab_strip_model()->GetActiveWebContents()->GetURL();
-  EXPECT_EQ("www.bar.com", url.host());
-  EXPECT_EQ("/", url.path());
+  ASSERT_NO_FATAL_FAILURE(
+      NavigateExpectUrl(GURL("http://www.bar.com/"), ui::EF_CONTROL_DOWN));
 }
 
-#if defined(OS_LINUX)
-#define MAYBE_DesiredTLDWithTemporaryText DISABLED_DesiredTLDWithTemporaryText
-#else
-#define MAYBE_DesiredTLDWithTemporaryText DesiredTLDWithTemporaryText
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DesiredTLDWithTemporaryText) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DesiredTLDWithTemporaryText) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   OmniboxPopupModel* popup_model = omnibox_view->model()->popup_model();
@@ -684,29 +640,11 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DesiredTLDWithTemporaryText) {
 
   // Hitting ctrl-enter should navigate based on the current text rather than
   // the original input, i.e. to www.abc.com instead of www.ab.com.
-  ASSERT_TRUE(SendKeyAndWait(
-      browser(), ui::VKEY_RETURN, ui::EF_CONTROL_DOWN,
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<content::NavigationController>(
-          &browser()->tab_strip_model()->GetActiveWebContents()->
-              GetController())));
-
-  GURL url(browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
-  EXPECT_EQ("www.abc.com", url.host());
-  EXPECT_EQ("/", url.path());
+  ASSERT_NO_FATAL_FAILURE(
+      NavigateExpectUrl(GURL("http://www.abc.com/"), ui::EF_CONTROL_DOWN));
 }
 
-// See http://crbug.com/431575.
-// Flaky on Mac (crbug.com/841195).
-#if defined(OS_MACOSX)
-#define MAYBE_ClearUserTextAfterBackgroundCommit \
-  DISABLED_ClearUserTextAfterBackgroundCommit
-#else
-#define MAYBE_ClearUserTextAfterBackgroundCommit \
-  ClearUserTextAfterBackgroundCommit
-#endif
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest,
-                       MAYBE_ClearUserTextAfterBackgroundCommit) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, ClearUserTextAfterBackgroundCommit) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -746,8 +684,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, AltEnter) {
   ASSERT_NO_FATAL_FAILURE(WaitForTabOpenOrClose(tab_count + 1));
 }
 
-// http://crbug.com/133354, http://crbug.com/146953
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_EnterToSearch) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, EnterToSearch) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   OmniboxPopupModel* popup_model = omnibox_view->model()->popup_model();
@@ -757,43 +694,20 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_EnterToSearch) {
   ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchTextKeys));
   ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
   ASSERT_TRUE(popup_model->IsOpen());
-
-  // Check if the default match result is Search Primary Provider.
   ASSERT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
             popup_model->result().default_match()->type);
-
-  // Open the default match.
-  ASSERT_TRUE(SendKeyAndWait(browser(), ui::VKEY_RETURN, 0,
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<content::NavigationController>(
-          &browser()->tab_strip_model()->GetActiveWebContents()->
-              GetController())));
-  GURL url = browser()->tab_strip_model()->GetActiveWebContents()->GetURL();
-  EXPECT_EQ(kSearchTextURL, url.spec());
+  ASSERT_NO_FATAL_FAILURE(NavigateExpectUrl(GURL(kSearchTextURL)));
 
   // Test that entering a single character then Enter performs a search.
-  const ui::KeyboardCode kSearchSingleCharKeys[] = {
-    ui::VKEY_Z, ui::VKEY_UNKNOWN
-  };
   chrome::FocusLocationBar(browser());
   EXPECT_TRUE(omnibox_view->IsSelectAll());
-  ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchSingleCharKeys));
+  ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_Z, 0));
   ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
   ASSERT_TRUE(popup_model->IsOpen());
-  EXPECT_EQ("z", UTF16ToUTF8(omnibox_view->GetText()));
-
-  // Check if the default match result is Search Primary Provider.
   ASSERT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
             popup_model->result().default_match()->type);
-
-  // Open the default match.
-  ASSERT_TRUE(SendKeyAndWait(browser(), ui::VKEY_RETURN, 0,
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<content::NavigationController>(
-          &browser()->tab_strip_model()->GetActiveWebContents()->
-              GetController())));
-  url = browser()->tab_strip_model()->GetActiveWebContents()->GetURL();
-  EXPECT_EQ("http://www.foo.com/search?q=z", url.spec());
+  ASSERT_NO_FATAL_FAILURE(
+      NavigateExpectUrl(GURL("http://www.foo.com/search?q=z")));
 }
 
 IN_PROC_BROWSER_TEST_F(OmniboxViewTest, EscapeToDefaultMatch) {
@@ -983,13 +897,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, SearchDisabledDontCrashOnQuestionMark) {
   ASSERT_EQ(ASCIIToUTF16("?"), omnibox_view->GetText());
 }
 
-// Flaky on Windows and Linux. http://crbug.com/751543
-#if defined(OS_WIN) || defined(OS_LINUX)
-#define MAYBE_AcceptKeywordBySpace DISABLED_AcceptKeywordBySpace
-#else
-#define MAYBE_AcceptKeywordBySpace AcceptKeywordBySpace
-#endif
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_AcceptKeywordBySpace) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, AcceptKeywordBySpace) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -1247,13 +1155,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, NonSubstitutingKeywordTest) {
             popup_model->result().default_match()->destination_url.spec());
 }
 
-// crbug.com/751031
-#if defined(OS_MACOSX)
-#define MAYBE_DeleteItem DISABLED_DeleteItem
-#else
-#define MAYBE_DeleteItem DeleteItem
-#endif
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DeleteItem) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DeleteItem) {
   // Disable the search provider, to make sure the popup contains only history
   // items.
   TemplateURLService* model =
@@ -1352,14 +1254,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_DeleteItem) {
   ASSERT_TRUE(omnibox_view->IsSelectAll());
 }
 
-// http://crbug.com/133344
-#if defined(OS_LINUX)
-#define MAYBE_TabAcceptKeyword DISABLED_TabAcceptKeyword
-#else
-#define MAYBE_TabAcceptKeyword TabAcceptKeyword
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_TabAcceptKeyword) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, TabAcceptKeyword) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -1402,15 +1297,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_TabAcceptKeyword) {
 
 #if !defined(OS_MACOSX)
 // Mac intentionally does not support this behavior.
-
-// http://crbug.com/133360
-#if defined(OS_LINUX)
-#define MAYBE_TabTraverseResultsTest DISABLED_TabTraverseResultsTest
-#else
-#define MAYBE_TabTraverseResultsTest TabTraverseResultsTest
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_TabTraverseResultsTest) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, TabTraverseResultsTest) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   OmniboxPopupModel* popup_model = omnibox_view->model()->popup_model();
@@ -1494,20 +1381,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_TabTraverseResultsTest) {
 }
 #endif
 
-
-// http://crbug.com/133347
-#if defined(OS_LINUX)
-#define MAYBE_PersistKeywordModeOnTabSwitch \
-    DISABLED_PersistKeywordModeOnTabSwitch
-#elif defined(OS_MACOSX)
-// Getting text from textfields doesn't always work: https://crbug.com/823532
-#define MAYBE_PersistKeywordModeOnTabSwitch \
-    DISABLED_PersistKeywordModeOnTabSwitch
-#else
-#define MAYBE_PersistKeywordModeOnTabSwitch PersistKeywordModeOnTabSwitch
-#endif
-
-IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PersistKeywordModeOnTabSwitch) {
+IN_PROC_BROWSER_TEST_F(OmniboxViewTest, PersistKeywordModeOnTabSwitch) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
 
@@ -1545,17 +1419,8 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PersistKeywordModeOnTabSwitch) {
   ASSERT_EQ(omnibox_view->GetText(), base::ASCIIToUTF16(kSearchText));
 }
 
-// http://crbug.com/133355
-#if defined(OS_LINUX)
-#define MAYBE_CtrlKeyPressedWithInlineAutocompleteTest \
-    DISABLED_CtrlKeyPressedWithInlineAutocompleteTest
-#else
-#define MAYBE_CtrlKeyPressedWithInlineAutocompleteTest \
-    CtrlKeyPressedWithInlineAutocompleteTest
-#endif
-
 IN_PROC_BROWSER_TEST_F(OmniboxViewTest,
-                       MAYBE_CtrlKeyPressedWithInlineAutocompleteTest) {
+                       CtrlKeyPressedWithInlineAutocompleteTest) {
   OmniboxView* omnibox_view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view));
   OmniboxPopupModel* popup_model = omnibox_view->model()->popup_model();