Omnibox: Query in Omnibox - Fix CurrentTextIsURL and match generation

This CL makes CurrentTextIsURL and the match generation mechanism
gracefully handle Query in Omnibox.

It also adds some tests for that.

Bug: 874592
Change-Id: I84e2bda6b489173a0a56ba2d6f461a1d0207fb11
Reviewed-on: https://chromium-review.googlesource.com/c/1263421
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597615}

components/omnibox/browser/omnibox_edit_model.cc

@@ -1410,10 +1410,14 @@ void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match,
         (!popup_model() || !popup_model()->has_selected_match()))
       *alternate_nav_url = result().alternate_nav_url();
   } else {
+    base::string16 text_for_match_generation =
+        (user_input_in_progress() || GetQueryInOmniboxSearchTerms(nullptr))
+            ? view_->GetText()
+            : url_for_editing_;
+
     client_->GetAutocompleteClassifier()->Classify(
-        MaybePrependKeyword(user_input_in_progress_ ? view_->GetText()
-                                                    : url_for_editing_),
-        is_keyword_selected(), true, ClassifyPage(), match, alternate_nav_url);
+        MaybePrependKeyword(text_for_match_generation), is_keyword_selected(),
+        true, ClassifyPage(), match, alternate_nav_url);
   }
 }
 

components/omnibox/browser/omnibox_edit_model_unittest.cc

@@ -141,7 +141,7 @@ TEST_F(OmniboxEditModelTest, AdjustTextForCopy) {
     AutocompleteMatch match;
     match.type = AutocompleteMatchType::NAVSUGGEST;
     match.destination_url = GURL(input[i].match_destination_url);
-    model()->SetCurrentMatch(match);
+    model()->SetCurrentMatchForTest(match);
 
     base::string16 result = base::ASCIIToUTF16(input[i].input);
     GURL url;
@@ -254,15 +254,32 @@ TEST_F(OmniboxEditModelTest, AlternateNavHasHTTP) {
       client->alternate_nav_match().fill_into_edit));
 }
 
-TEST_F(OmniboxEditModelTest, GenerateMatchesFromFullFormattedUrl) {
+TEST_F(OmniboxEditModelTest, CurrentMatch) {
   toolbar_model()->set_formatted_full_url(
       base::ASCIIToUTF16("http://localhost/"));
   toolbar_model()->set_url_for_display(base::ASCIIToUTF16("localhost"));
   model()->ResetDisplayTexts();
 
-  // Bypass the test class's mock method to test the real behavior.
-  AutocompleteMatch match = model()->OmniboxEditModel::CurrentMatch(nullptr);
-  EXPECT_EQ(AutocompleteMatchType::URL_WHAT_YOU_TYPED, match.type);
+  // Tests that we use the formatted full URL instead of the elided URL to
+  // generate matches.
+  {
+    AutocompleteMatch match = model()->CurrentMatch(nullptr);
+    EXPECT_EQ(AutocompleteMatchType::URL_WHAT_YOU_TYPED, match.type);
+    EXPECT_TRUE(model()->CurrentTextIsURL());
+  }
+
+  // Tests that when there is a Query in Omnibox, generate matches from the
+  // query, instead of the full formatted URL.
+  TestOmniboxClient* client =
+      static_cast<TestOmniboxClient*>(model()->client());
+  client->SetFakeSearchTermsForQueryInOmnibox(base::ASCIIToUTF16("foobar"));
+  model()->ResetDisplayTexts();
+
+  {
+    AutocompleteMatch match = model()->CurrentMatch(nullptr);
+    EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, match.type);
+    EXPECT_FALSE(model()->CurrentTextIsURL());
+  }
 }
 
 TEST_F(OmniboxEditModelTest, DisplayText) {

components/omnibox/browser/omnibox_view_unittest.cc

@@ -164,7 +164,7 @@ TEST_F(OmniboxViewTest, GetIcon_BookmarkIcon) {
 
   AutocompleteMatch match;
   match.destination_url = kUrl;
-  model()->SetCurrentMatch(match);
+  model()->SetCurrentMatchForTest(match);
 
   bookmark_model()->AddURL(bookmark_model()->bookmark_bar_node(), 0,
                            base::ASCIIToUTF16("a bookmark"), kUrl);
@@ -190,7 +190,7 @@ TEST_F(OmniboxViewTest, GetIcon_Favicon) {
   AutocompleteMatch match;
   match.type = AutocompleteMatchType::URL_WHAT_YOU_TYPED;
   match.destination_url = kUrl;
-  model()->SetCurrentMatch(match);
+  model()->SetCurrentMatchForTest(match);
 
   view()->GetIcon(gfx::kFaviconSize, gfx::kPlaceholderColor, base::DoNothing());
 

components/omnibox/browser/test_omnibox_edit_model.cc

@@ -12,18 +12,25 @@ TestOmniboxEditModel::TestOmniboxEditModel(OmniboxView* view,
     : OmniboxEditModel(view, controller, std::make_unique<TestOmniboxClient>()),
       popup_is_open_(false) {}
 
+TestOmniboxEditModel::~TestOmniboxEditModel() {}
+
 bool TestOmniboxEditModel::PopupIsOpen() const {
   return popup_is_open_;
 }
 
-AutocompleteMatch TestOmniboxEditModel::CurrentMatch(GURL*) const {
-  return current_match_;
+AutocompleteMatch TestOmniboxEditModel::CurrentMatch(
+    GURL* alternate_nav_url) const {
+  if (override_current_match_)
+    return *override_current_match_;
+
+  return OmniboxEditModel::CurrentMatch(alternate_nav_url);
 }
 
 void TestOmniboxEditModel::SetPopupIsOpen(bool open) {
   popup_is_open_ = open;
 }
 
-void TestOmniboxEditModel::SetCurrentMatch(const AutocompleteMatch& match) {
-  current_match_ = match;
+void TestOmniboxEditModel::SetCurrentMatchForTest(
+    const AutocompleteMatch& match) {
+  override_current_match_ = std::make_unique<AutocompleteMatch>(match);
 }

components/omnibox/browser/test_omnibox_edit_model.h

@@ -5,23 +5,26 @@
 #ifndef COMPONENTS_OMNIBOX_BROWSER_TEST_OMNIBOX_EDIT_MODEL_H_
 #define COMPONENTS_OMNIBOX_BROWSER_TEST_OMNIBOX_EDIT_MODEL_H_
 
+#include <memory>
+
 #include "components/omnibox/browser/omnibox_edit_model.h"
 
 class TestOmniboxEditModel : public OmniboxEditModel {
  public:
   TestOmniboxEditModel(OmniboxView* view, OmniboxEditController* controller);
+  ~TestOmniboxEditModel() override;
 
   // OmniboxEditModel:
   bool PopupIsOpen() const override;
-  AutocompleteMatch CurrentMatch(GURL*) const override;
+  AutocompleteMatch CurrentMatch(GURL* alternate_nav_url) const override;
 
   void SetPopupIsOpen(bool open);
 
-  void SetCurrentMatch(const AutocompleteMatch& match);
+  void SetCurrentMatchForTest(const AutocompleteMatch& match);
 
  private:
   bool popup_is_open_;
-  AutocompleteMatch current_match_;
+  std::unique_ptr<AutocompleteMatch> override_current_match_;
 
   DISALLOW_COPY_AND_ASSIGN(TestOmniboxEditModel);
 };