Make TemplateURLService functions pickier about preconditions.

The goal here is to crash early if existing preconditions are violated,
so we have a better shot at understanding how clients are in a bad
state.

Bug: 1031506
Change-Id: Iec8360798a8381579b6e5dd536b1c503660191a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989037
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729175}

components/search_engines/template_url_service.cc

@@ -1444,19 +1444,13 @@ void TemplateURLService::Init(const Initializer* initializers,
 
 TemplateURL* TemplateURLService::BestEngineForKeyword(TemplateURL* engine1,
                                                       TemplateURL* engine2) {
-  DCHECK(engine1);
-  DCHECK(engine2);
-  DCHECK_EQ(engine1->keyword(), engine2->keyword());
+  CHECK(engine1);
+  CHECK(engine2);
+  CHECK_EQ(engine1->keyword(), engine2->keyword());
 
   // We should only have overlapping keywords when at least one comes from
   // an extension.
-  DCHECK(IsCreatedByExtension(engine1) || IsCreatedByExtension(engine2));
-
-  // TODO(a-v-y) Remove following code for non extension engines when reasons
-  // for crash https://bugs.chromium.org/p/chromium/issues/detail?id=697745
-  // become clear.
-  if (!IsCreatedByExtension(engine1) && !IsCreatedByExtension(engine2))
-    return CanReplace(engine1) ? engine2 : engine1;
+  CHECK(IsCreatedByExtension(engine1) || IsCreatedByExtension(engine2));
 
   if (engine2->type() == engine1->type()) {
     return engine1->extension_info_->install_time >
@@ -1474,8 +1468,11 @@ TemplateURL* TemplateURLService::BestEngineForKeyword(TemplateURL* engine1,
 
 void TemplateURLService::RemoveFromMaps(const TemplateURL* template_url) {
   const base::string16& keyword = template_url->keyword();
-  DCHECK_NE(0U, keyword_to_turl_and_length_.count(keyword));
-  if (keyword_to_turl_and_length_[keyword].first == template_url) {
+  auto iter = keyword_to_turl_and_length_.find(keyword);
+  CHECK(iter != keyword_to_turl_and_length_.end());
+  // The entry at |iter| may not be |template_url| if it's an extension-created
+  // entry with the same keyword.
+  if (iter->second.first == template_url) {
     // We need to check whether the keyword can now be provided by another
     // TemplateURL. See the comments for BestEngineForKeyword() for more
     // information on extension keywords and how they can coexist with
@@ -1494,7 +1491,7 @@ void TemplateURLService::RemoveFromMaps(const TemplateURL* template_url) {
       AddToMap(best_fallback);
       AddToDomainMap(best_fallback);
     } else {
-      keyword_to_turl_and_length_.erase(keyword);
+      keyword_to_turl_and_length_.erase(iter);
     }
   }
 
@@ -1520,7 +1517,7 @@ void TemplateURLService::AddToMaps(TemplateURL* template_url) {
     AddToDomainMap(template_url);
   } else {
     TemplateURL* existing_url = i->second.first;
-    DCHECK_NE(existing_url, template_url);
+    CHECK_NE(existing_url, template_url);
     if (BestEngineForKeyword(existing_url, template_url) != existing_url) {
       RemoveFromDomainMap(existing_url);
       AddToMap(template_url);
@@ -1690,8 +1687,10 @@ bool TemplateURLService::Update(TemplateURL* existing_turl,
   // calling AddToMaps() below).
   existing_turl->CopyFrom(new_values);
   existing_turl->data_.id = previous_id;
-  if (keep_old_keyword)
+  if (keep_old_keyword) {
+    CHECK_NE(old_keyword, new_values.keyword());
     existing_turl->data_.SetKeyword(old_keyword);
+  }
 
   AddToMaps(existing_turl);
 
@@ -1709,7 +1708,7 @@ bool TemplateURLService::Update(TemplateURL* existing_turl,
   if (default_search_provider_source_ != DefaultSearchManager::FROM_FALLBACK)
     MaybeUpdateDSEViaPrefs(existing_turl);
 
-  DCHECK(!HasDuplicateKeywords());
+  CHECK(!HasDuplicateKeywords());
   return true;
 }
 
@@ -1988,7 +1987,7 @@ TemplateURL* TemplateURLService::Add(std::unique_ptr<TemplateURL> template_url,
   if (template_url_ptr)
     model_mutated_notification_pending_ = true;
 
-  DCHECK(!HasDuplicateKeywords());
+  CHECK(!HasDuplicateKeywords());
   return template_url_ptr;
 }