Fix a crash where we send an invalid notification to TemplateURLTableModel's observers.

BUG=52251
TEST=included, also try: chmod a-r /path/to/profile/Default/Web\ Data, start Chromium and try to set the default search provider.
Review URL: http://codereview.chromium.org/3189006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56652 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 35 insertions, 9 deletions

diff --git a/chrome/browser/search_engines/keyword_editor_controller_unittest.cc b/chrome/browser/search_engines/keyword_editor_controller_unittest.cc
index 499f5aa..3670b05 100644
--- a/chrome/browser/search_engines/keyword_editor_controller_unittest.cc
+++ b/chrome/browser/search_engines/keyword_editor_controller_unittest.cc
@@ -95,7 +95,7 @@ TEST_F(KeywordEditorControllerTest, Add) {
   EXPECT_EQ(L"a", turl->short_name());
   EXPECT_EQ(L"b", turl->keyword());
   EXPECT_TRUE(turl->url() != NULL);
-  EXPECT_TRUE(turl->url()->url() == "http://c");
+  EXPECT_EQ("http://c", turl->url()->url());
 }
 
 // Tests modifying a TemplateURL.
@@ -112,7 +112,7 @@ TEST_F(KeywordEditorControllerTest, Modify) {
   EXPECT_EQ(L"a1", turl->short_name());
   EXPECT_EQ(L"b1", turl->keyword());
   EXPECT_TRUE(turl->url() != NULL);
-  EXPECT_TRUE(turl->url()->url() == "http://c1");
+  EXPECT_EQ("http://c1", turl->url()->url());
 }
 
 // Tests making a TemplateURL the default search provider.
@@ -121,12 +121,29 @@ TEST_F(KeywordEditorControllerTest, MakeDefault) {
   ClearChangeCount();
 
   const TemplateURL* turl = model_->GetTemplateURLs()[0];
-  controller_->MakeDefaultTemplateURL(0);
+  int new_default = controller_->MakeDefaultTemplateURL(0);
+  EXPECT_EQ(0, new_default);
   // Making an item the default sends a handful of changes. Which are sent isn't
   // important, what is important is 'something' is sent.
   ASSERT_TRUE(items_changed_count_ > 0 || added_count_ > 0 ||
               removed_count_ > 0);
   ASSERT_TRUE(model_->GetDefaultSearchProvider() == turl);
+
+  // Making it default a second time should fail.
+  new_default = controller_->MakeDefaultTemplateURL(0);
+  EXPECT_EQ(-1, new_default);
+}
+
+TEST_F(KeywordEditorControllerTest, MakeDefaultNoWebData) {
+  // Simulate a failure to load Web Data.
+  model_->OnWebDataServiceRequestDone(NULL, NULL);
+
+  controller_->AddTemplateURL(L"a", L"b", "http://c{searchTerms}");
+  ClearChangeCount();
+
+  // This should not result in a crash.
+  int new_default = controller_->MakeDefaultTemplateURL(0);
+  EXPECT_EQ(0, new_default);
 }
 
 // Mutates the TemplateURLModel and make sure table model is updating
diff --git a/chrome/browser/search_engines/template_url_table_model.cc b/chrome/browser/search_engines/template_url_table_model.cc
index 2749f33..0167556 100644
--- a/chrome/browser/search_engines/template_url_table_model.cc
+++ b/chrome/browser/search_engines/template_url_table_model.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/search_engines/template_url_table_model.h"
 
+#include <string>
 #include <vector>
 
 #include "app/l10n_util.h"
@@ -161,8 +162,9 @@ void TemplateURLTableModel::Reload() {
     // NOTE: we don't use ShowInDefaultList here to avoid things bouncing
     // the lists while editing.
     if (!template_url->show_in_default_list() &&
-        !template_url->IsExtensionKeyword())
+        !template_url->IsExtensionKeyword()) {
       entries_.push_back(new ModelEntry(this, *template_url));
+    }
   }
 
   if (observer_)
@@ -245,8 +247,8 @@ void TemplateURLTableModel::Remove(int index) {
   template_url_model_->RemoveObserver(this);
   const TemplateURL* template_url = &GetTemplateURL(index);
 
-  scoped_ptr<ModelEntry> entry(entries_[static_cast<int>(index)]);
-  entries_.erase(entries_.begin() + static_cast<int>(index));
+  scoped_ptr<ModelEntry> entry(entries_[index]);
+  entries_.erase(entries_.begin() + index);
   if (index < last_search_engine_index_)
     last_search_engine_index_--;
   if (observer_)
@@ -345,8 +347,13 @@ int TemplateURLTableModel::MakeDefaultTemplateURL(int index) {
 
   // The formatting of the default engine is different; notify the table that
   // both old and new entries have changed.
-  if (current_default != NULL)
-    NotifyChanged(IndexOfTemplateURL(current_default));
+  if (current_default != NULL) {
+    int old_index = IndexOfTemplateURL(current_default);
+    // current_default may not be in the list of TemplateURLs if the database is
+    // corrupt and the default TemplateURL is used from preferences
+    if (old_index >= 0)
+      NotifyChanged(old_index);
+  }
   const int new_index = IndexOfTemplateURL(keyword);
   NotifyChanged(new_index);
 
@@ -355,8 +362,10 @@ int TemplateURLTableModel::MakeDefaultTemplateURL(int index) {
 }
 
 void TemplateURLTableModel::NotifyChanged(int index) {
-  if (observer_)
+  if (observer_) {
+    DCHECK_GE(index, 0);
     observer_->OnItemsChanged(index, 1);
+  }
 }
 
 void TemplateURLTableModel::FavIconAvailable(ModelEntry* entry) {