[omnibox] Delete duplicate matches at BaseSearchProvider

There are some cases when deletable matches can be duplicates in
BaseSearchProvider, so after deleting we can still observe Remove
Suggest button. This CL adds checking whether there are deletable
duplicates with matching contents and type.

Bug: 1108674
Change-Id: Ic1a247957a3c8d0dde01134edb6205bd3006e6dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315859
Commit-Queue: Alexander Yashkin <a-v-y@yandex-team.ru>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792688}

components/omnibox/browser/base_search_provider.cc

@@ -32,6 +32,15 @@
 #include "url/gurl.h"
 #include "url/origin.h"
 
+namespace {
+
+bool MatchTypeAndContentsAreEqual(const AutocompleteMatch& lhs,
+                                  const AutocompleteMatch& rhs) {
+  return lhs.contents == rhs.contents && lhs.type == rhs.type;
+}
+
+}  // namespace
+
 // SuggestionDeletionHandler -------------------------------------------------
 
 // This class handles making requests to the server in order to delete
@@ -569,10 +578,24 @@ void BaseSearchProvider::DeleteMatchFromMatches(
     // may have reformulated that. Not that while checking for matching
     // contents works for personalized suggestions, if more match types gain
     // deletion support, this algorithm may need to be re-examined.
-    if (i->contents == match.contents && i->type == match.type) {
+
+    if (MatchTypeAndContentsAreEqual(match, *i)) {
       matches_.erase(i);
       break;
     }
+
+    // Handle the case where the deleted match is only found within the
+    // duplicate_matches sublist.
+    std::vector<AutocompleteMatch>& duplicates = i->duplicate_matches;
+    auto it =
+        std::remove_if(duplicates.begin(), duplicates.end(),
+                       [&match](const AutocompleteMatch& duplicate) {
+                         return MatchTypeAndContentsAreEqual(match, duplicate);
+                       });
+    if (it != duplicates.end()) {
+      duplicates.erase(it, duplicates.end());
+      break;
+    }
   }
 }
 

components/omnibox/browser/base_search_provider_unittest.cc

@@ -39,7 +39,6 @@ class TestBaseSearchProvider : public BaseSearchProvider {
       : BaseSearchProvider(type, client) {}
   TestBaseSearchProvider(const TestBaseSearchProvider&) = delete;
   TestBaseSearchProvider& operator=(const TestBaseSearchProvider&) = delete;
-  MOCK_METHOD1(DeleteMatch, void(const AutocompleteMatch& match));
   MOCK_CONST_METHOD1(AddProviderInfo, void(ProvidersInfo* provider_info));
   MOCK_CONST_METHOD1(GetTemplateURL, const TemplateURL*(bool is_keyword));
   MOCK_CONST_METHOD1(GetInput, const AutocompleteInput(bool is_keyword));
@@ -63,6 +62,10 @@ class TestBaseSearchProvider : public BaseSearchProvider {
                                       map);
   }
 
+  void AddMatch(const AutocompleteMatch& match) {
+    matches_.push_back(match);
+  }
+
  protected:
   ~TestBaseSearchProvider() override {}
 };
@@ -214,3 +217,46 @@ TEST_F(BaseSearchProviderTest, MatchTailSuggestionProperly) {
   text = entry.second.GetAdditionalInfo(kACMatchPropertySuggestionText);
   EXPECT_GE(text.length(), length);
 }
+
+TEST_F(BaseSearchProviderTest, DeleteDuplicateMatch) {
+  TemplateURLData data;
+  data.SetURL("http://foo.com/url?bar={searchTerms}");
+  auto template_url = std::make_unique<TemplateURL>(data);
+
+  TestBaseSearchProvider::MatchMap map;
+  base::string16 query = base::ASCIIToUTF16("site.com");
+
+  EXPECT_CALL(*provider_, GetInput(_))
+      .WillRepeatedly(Return(AutocompleteInput()));
+  EXPECT_CALL(*provider_, GetTemplateURL(_))
+      .WillRepeatedly(Return(template_url.get()));
+
+  SearchSuggestionParser::SuggestResult more_relevant(
+      query, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+      /*subtypes=*/{}, /*from_keyword_provider=*/false,
+      /*relevance=*/850, /*relevance_from_server=*/true,
+      /*input_text=*/query);
+  provider_->AddMatchToMap(more_relevant, std::string(),
+                           TemplateURLRef::NO_SUGGESTION_CHOSEN, false, false,
+                           &map);
+
+  SearchSuggestionParser::SuggestResult less_relevant(
+      query, AutocompleteMatchType::SEARCH_HISTORY,
+      /*subtypes=*/{}, /*from_keyword_provider=*/false,
+      /*relevance=*/735, /*relevance_from_server=*/true,
+      /*input_text=*/query);
+  provider_->AddMatchToMap(less_relevant, std::string(),
+                           TemplateURLRef::NO_SUGGESTION_CHOSEN, true, false,
+                           &map);
+
+  ASSERT_EQ(1U, map.size());
+  ASSERT_TRUE(provider_->matches().empty());
+
+  AutocompleteMatch match = map.begin()->second;
+  ASSERT_EQ(1U, match.duplicate_matches.size());
+  provider_->AddMatch(match);
+
+  provider_->DeleteMatch(match.duplicate_matches[0]);
+  ASSERT_EQ(1U, provider_->matches().size());
+  ASSERT_TRUE(provider_->matches()[0].duplicate_matches.empty());
+}