Enforce that TemplateURLs' URLs cannot be empty.

This should already have been true in non-test code: the keyword editor, the importer, the extension system, and the search engine autodetection code all tried to ensure that the URLs are non-empty.  However, because we've seen problems occur, this change tries to sanitize the pref, database, and sync inputs to expunge any bogus entries.

We also fix tests, add some more DCHECKs to make this assertion clearer in some of the TemplateURL-adding paths, and then remove conditionals that checked for empty URLs.

This also cleans up a silly design where TemplateURLService listened for changes to the default search provider prefs -- meaning that anyone who tried to write them wound up calling back to the observer as every single pref was written.  This led to reading back bad state.  Eliminating this ought to allow for some other cleanups in TemplateURLService, but I haven't tried to make them here.

BUG=none
TEST=none
Review URL: https://chromiumcodereview.appspot.com/10014033

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

2 files changed, 16 insertions, 19 deletions

diff --git a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
index d78f33e..68ef24f 100644
--- a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
+++ b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
@@ -15,8 +15,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
 #include "chrome/test/base/testing_profile.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/models/table_model_observer.h"
 
@@ -70,24 +69,20 @@ class KeywordEditorControllerTest : public testing::Test,
   void SimulateDefaultSearchIsManaged(const std::string& url) {
     ASSERT_FALSE(url.empty());
     TestingPrefService* service = profile_->GetTestingPrefService();
-    service->SetManagedPref(
-        prefs::kDefaultSearchProviderEnabled,
-        Value::CreateBooleanValue(true));
-    service->SetManagedPref(
-        prefs::kDefaultSearchProviderSearchURL,
-        Value::CreateStringValue(url));
-    service->SetManagedPref(
-        prefs::kDefaultSearchProviderName,
-        Value::CreateStringValue("managed"));
+    service->SetManagedPref(prefs::kDefaultSearchProviderEnabled,
+                            Value::CreateBooleanValue(true));
+    service->SetManagedPref(prefs::kDefaultSearchProviderSearchURL,
+                            Value::CreateStringValue(url));
+    service->SetManagedPref(prefs::kDefaultSearchProviderName,
+                            Value::CreateStringValue("managed"));
     // Clear the IDs that are not specified via policy.
-    service->SetManagedPref(
-        prefs::kDefaultSearchProviderID, new StringValue(""));
-    service->SetManagedPref(
-        prefs::kDefaultSearchProviderPrepopulateID, new StringValue(""));
-    model_->Observe(
-        chrome::NOTIFICATION_PREF_CHANGED,
-        content::Source<PrefService>(profile_->GetTestingPrefService()),
-        content::Details<std::string>(NULL));
+    service->SetManagedPref(prefs::kDefaultSearchProviderID,
+                            new StringValue(std::string()));
+    service->SetManagedPref(prefs::kDefaultSearchProviderPrepopulateID,
+                            new StringValue(std::string()));
+    model_->Observe(chrome::NOTIFICATION_DEFAULT_SEARCH_POLICY_CHANGED,
+                    content::NotificationService::AllSources(),
+                    content::NotificationService::NoDetails());
   }
 
   TemplateURLTableModel* table_model() const {
diff --git a/chrome/browser/ui/search_engines/template_url_table_model.cc b/chrome/browser/ui/search_engines/template_url_table_model.cc
index 83b566a..cd238ad 100644
--- a/chrome/browser/ui/search_engines/template_url_table_model.cc
+++ b/chrome/browser/ui/search_engines/template_url_table_model.cc
@@ -253,6 +253,7 @@ void TemplateURLTableModel::Add(int index,
                                 const string16& keyword,
                                 const std::string& url) {
   DCHECK(index >= 0 && index <= RowCount());
+  DCHECK(!url.empty());
   template_url_service_->RemoveObserver(this);
   TemplateURLData data;
   data.short_name = short_name;
@@ -272,6 +273,7 @@ void TemplateURLTableModel::ModifyTemplateURL(int index,
                                               const string16& keyword,
                                               const std::string& url) {
   DCHECK(index >= 0 && index <= RowCount());
+  DCHECK(!url.empty());
   const TemplateURL* template_url = GetTemplateURL(index);
   template_url_service_->RemoveObserver(this);
   template_url_service_->ResetTemplateURL(template_url, title, keyword, url);