Local omnibox treats navsuggest suggestions as queries.

chrome/browser/ui/search/instant_extended_interactive_uitest.cc

Index: chrome/browser/ui/search/instant_extended_interactive_uitest.cc
diff --git a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
index 95d5d138aaa3d1659ccde27314466576eb4e82c0..1234ce7eed36cc2fe0f520e41dfbb1cfbc5b9c30 100644
--- a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
+++ b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
@@ -7,6 +7,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
@@ -35,6 +36,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/theme_source.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/thumbnail_score.h"
 #include "chrome/common/url_constants.h"
@@ -820,6 +822,75 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, ProcessIsolation) {
       active_tab->GetRenderProcessHost()->GetID()));
 }
 
+// Test that a search query will not be displayed for navsuggest queries.
+IN_PROC_BROWSER_TEST_F(
+    InstantExtendedTest, SearchQueryNotDisplayedForNavsuggest) {

[sreeram, 2013/04/29 22:26:31]

Nit: Move "InstantExtendedTest" to the previous line.

[dougw, 2013/04/29 22:48:49]

Done.

+  // Use only the local overlay.
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableLocalOnlyInstantExtendedAPI);
+  ASSERT_TRUE(chrome::IsLocalOnlyInstantExtendedAPIEnabled());
+
+  ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
+
+  // The second argument indicates to use only the local overlay and NTP.
+  instant()->SetInstantEnabled(true, true);
+
+  // Focus omnibox and confirm overlay isn't shown.
+  FocusOmniboxAndWaitForInstantSupport();

[sreeram, 2013/04/29 22:26:31]

FocusOmniboxAndWaitForInstant***Extended***Support()?

[dougw, 2013/04/29 22:48:49]

This is intentional. The test hangs if we wait for:
chrome::NOTIFICATION_INSTANT_NTP_SUPPORT_DETERMINED.

The function names are misleading. I wrote a cl to fix this: crrev.com/14050016.


On 2013/04/29 22:26:31, sreeram wrote:

+
+  // Typing in the omnibox should show the overlay.
+  SetOmniboxText("face");
+
+  // Add a navsuggest suggestion.
+  instant()->SetSuggestions(
+      instant()->overlay()->contents(),
+      std::vector<InstantSuggestion>(
+          1,
+          InstantSuggestion(ASCIIToUTF16("http://facemash.com/"),
+                            INSTANT_COMPLETE_NOW,
+                            INSTANT_SUGGESTION_URL,
+                            ASCIIToUTF16("face"))));
+
+  while (!omnibox()->model()->autocomplete_controller()->done()) {
+    content::WindowedNotificationObserver autocomplete_observer(
+        chrome::NOTIFICATION_AUTOCOMPLETE_CONTROLLER_RESULT_READY,
+        content::NotificationService::AllSources());
+    autocomplete_observer.Wait();
+  }
+
+  EXPECT_TRUE(
+      ExecuteScript(
+          "var sortedNativeSuggestions = chrome.embeddedSearch.searchBox.native"
+          "Suggestions.sort(function (a,b) {return b.rankingData.relevance - a."
+          "rankingData.relevance});"));

[sreeram, 2013/04/29 22:26:31]

Yikes. This is really hard to read. Please don't split variables across multiple
lines. How about something like this:
  EXPECT_TRUE(ExecuteScript(
      "var sorted = chrome.embeddedSearch.searchBox.nativeSuggestions.sort("
          "function (a, b) {"
            "return b.rankingData.relevance - a.rankingData.relevance;"
          "});"));

[dougw, 2013/04/29 22:48:49]

Done.

+
+  // Create an event listener that opens the top suggestion in a new tab.

[sreeram, 2013/04/29 22:26:31]

This comment has nothing to do with the next line. Just delete it.

[dougw, 2013/04/29 22:48:49]

Done.

+  content::WebContents* overlay = instant()->GetOverlayContents();
+
+  int suggestions_count;

[sreeram, 2013/04/29 22:26:31]

Initialize this to an invalid value, such as -1.

[dougw, 2013/04/29 22:48:49]

Done.

+  EXPECT_TRUE(GetIntFromJS(
+      overlay, "sortedNativeSuggestions.length", &suggestions_count));
+  EXPECT_TRUE(suggestions_count > 0);

[sreeram, 2013/04/29 22:26:31]

EXPECT_GT(suggestions_count, 0);

[dougw, 2013/04/29 22:48:49]

Done.

+
+  std::string type;
+  EXPECT_TRUE(
+      GetStringFromJS(overlay, "sortedNativeSuggestions[0].type", &type));
+  ASSERT_TRUE(type == "navsuggest");

[sreeram, 2013/04/29 22:26:31]

EXPECT_EQ("navsuggest", type);

[dougw, 2013/04/29 22:48:49]

Done.

+
+  bool is_search_property_exists;

[sreeram, 2013/04/29 22:26:31]

Nit: Fix bad grammar: "is_search_property_defined" or some such.

What's the advantage of testing the definedness of this property? Why not just
ask whether it's true or false? Does anyone care about the difference between it
being undefined versus it being defined and false?

[dougw, 2013/04/29 22:48:49]

Done.

GetBoolFromJS will hang if it is called when is_search isn't defined.

On 2013/04/29 22:26:31, sreeram wrote:

+  EXPECT_TRUE(
+      GetBoolFromJS(
+          overlay, "sortedNativeSuggestions[0].hasOwnProperty('is_search')",
+          &is_search_property_exists));
+
+  if (is_search_property_exists) {
+    bool is_search;
+    EXPECT_TRUE(GetBoolFromJS(
+        overlay, "sortedNativeSuggestions[0].is_search", &is_search));
+    EXPECT_FALSE(is_search);
+  }
+}
+
 // Verification of fix for BUG=176365.  Ensure that each Instant WebContents in
 // a tab uses a new BrowsingInstance, to avoid conflicts in the
 // NavigationController.