Revert 131019 - This caused memory tests to fail on all the perf bots.

Move most TemplateURL data members to a new struct, TemplateURLData.  This allows us to eliminate the TemplateURL NULL constructor, most public non-const TemplateURL functions, and most TemplateURL friend declarations.

This is also a necessary precursor to changing TemplateURLService's APIs to convert most "const TemplateURL*" cases to "TemplateURL*", which I'll explain once I actually make the change.

There is some awkwardness here around keywords, as keyword autogeneration requires a TemplateURL but the state bits are kept on TemplateURLData.  This will go away in the future when I remove keyword autogeneration from TemplateURL entirely.

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

TBR=pkasting@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9949059

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

6 files changed, 50 insertions, 45 deletions

diff --git a/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm b/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm
index 987679d..8cf6050 100644
--- a/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm
@@ -206,13 +206,12 @@ TEST_F(EditSearchEngineControllerTest, ValidateFields) {
 
 // Tests editing an existing TemplateURL.
 TEST_F(EditSearchEngineControllerTest, EditTemplateURL) {
-  TemplateURLData data;
-  data.short_name = ASCIIToUTF16("Foobar");
-  data.SetKeyword(ASCIIToUTF16("keyword"));
+  TemplateURL url;
+  url.set_short_name(ASCIIToUTF16("Foobar"));
+  url.set_keyword(ASCIIToUTF16("keyword"));
   std::string urlString = TemplateURLRef::DisplayURLToURLRef(
       ASCIIToUTF16("http://foo-bar.com"));
-  data.SetURL(urlString);
-  TemplateURL url(data);
+  url.SetURL(urlString);
   FakeEditSearchEngineController *controller =
       [[FakeEditSearchEngineController alloc] initWithProfile:profile()
                                                      delegate:nil
diff --git a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
index 596bfe2..3a1cf30 100644
--- a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
+++ b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
@@ -285,16 +285,18 @@ class OmniboxViewTest : public InProcessBrowserTest,
          i = builtins.begin(); i != builtins.end(); ++i)
       model->Remove(*i);
 
-    TemplateURLData data;
-    data.short_name = ASCIIToUTF16(kSearchShortName);
-    data.SetKeyword(ASCIIToUTF16(kSearchKeyword));
-    data.SetURL(kSearchURL);
-    TemplateURL* template_url = new TemplateURL(data);
+    TemplateURL* template_url = new TemplateURL();
+    template_url->SetURL(kSearchURL);
+    template_url->set_keyword(ASCIIToUTF16(kSearchKeyword));
+    template_url->set_short_name(ASCIIToUTF16(kSearchShortName));
     model->Add(template_url);
     model->SetDefaultSearchProvider(template_url);
 
-    data.SetKeyword(ASCIIToUTF16(kSearchKeyword2));
-    model->Add(new TemplateURL(data));
+    TemplateURL* second_url = new TemplateURL();
+    second_url->SetURL(kSearchURL);
+    second_url->set_keyword(ASCIIToUTF16(kSearchKeyword2));
+    second_url->set_short_name(ASCIIToUTF16(kSearchShortName));
+    model->Add(second_url);
   }
 
   void AddHistoryEntry(const TestHistoryEntry& entry, const Time& time) {
@@ -943,11 +945,10 @@ class OmniboxViewTest : public InProcessBrowserTest,
         TemplateURLServiceFactory::GetForProfile(browser()->profile());
 
     // Add a non-default substituting keyword.
-    TemplateURLData data;
-    data.short_name = ASCIIToUTF16("Search abc");
-    data.SetKeyword(ASCIIToUTF16(kSearchText));
-    data.SetURL("http://abc.com/{searchTerms}");
-    TemplateURL* template_url = new TemplateURL(data);
+    TemplateURL* template_url = new TemplateURL();
+    template_url->SetURL("http://abc.com/{searchTerms}");
+    template_url->set_keyword(UTF8ToUTF16(kSearchText));
+    template_url->set_short_name(UTF8ToUTF16("Search abc"));
     template_url_service->Add(template_url);
 
     omnibox_view->SetUserText(string16());
@@ -969,9 +970,11 @@ class OmniboxViewTest : public InProcessBrowserTest,
 
     // Try a non-substituting keyword.
     template_url_service->Remove(template_url);
-    data.short_name = ASCIIToUTF16("abc");
-    data.SetURL("http://abc.com/");
-    template_url_service->Add(new TemplateURL(data));
+    template_url = new TemplateURL();
+    template_url->SetURL("http://abc.com/");
+    template_url->set_keyword(UTF8ToUTF16(kSearchText));
+    template_url->set_short_name(UTF8ToUTF16("abc"));
+    template_url_service->Add(template_url);
 
     // We always allow exact matches for non-substituting keywords.
     ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchTextKeys));
diff --git a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc
index a5ce1c4..db46f3e 100644
--- a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc
+++ b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc
@@ -41,9 +41,8 @@ bool EditSearchEngineController::IsURLValid(
   // contains replacement strings.  We do this by constructing a dummy
   // TemplateURL owner because |template_url_| might be NULL and we can't call
   // TemplateURLRef::IsValid() when its owner is NULL.
-  TemplateURLData data;
-  data.SetURL(url);
-  TemplateURL t_url(data);
+  TemplateURL t_url;
+  t_url.SetURL(url);
   const TemplateURLRef& template_ref = t_url.url_ref();
   if (!template_ref.IsValid())
     return false;
@@ -98,9 +97,15 @@ void EditSearchEngineController::AcceptAddOrEdit(
     // Confiming an entry we got from JS. We have a template_url_, but it
     // hasn't yet been added to the model.
     DCHECK(template_url_);
+    // const_cast is ugly, but this is the same thing the TemplateURLService
+    // does in a similar situation (updating an existing TemplateURL with
+    // data from a new one).
+    TemplateURL* modifiable_url = const_cast<TemplateURL*>(template_url_);
+    modifiable_url->set_short_name(title_input);
+    modifiable_url->set_keyword(keyword_input);
+    modifiable_url->SetURL(url_string);
     // TemplateURLService takes ownership of template_url_.
-    template_url_service->AddWithOverrides(template_url_, title_input,
-                                           keyword_input, url_string);
+    template_url_service->Add(modifiable_url);
     content::RecordAction(UserMetricsAction("KeywordEditor_AddKeywordJS"));
   } else {
     // Adding or modifying an entry via the Delegate.
@@ -129,9 +134,8 @@ std::string EditSearchEngineController::GetFixedUpURL(
   // Parse the string as a URL to determine the scheme. If we need to, add the
   // scheme. As the scheme may be expanded (as happens with {google:baseURL})
   // we need to replace the search terms before testing for the scheme.
-  TemplateURLData data;
-  data.SetURL(url);
-  TemplateURL t_url(data);
+  TemplateURL t_url;
+  t_url.SetURL(url);
   std::string expanded_url(t_url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"),
       TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16()));
   url_parse::Parsed parts;
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..90c7af8 100644
--- a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
+++ b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
@@ -245,10 +245,9 @@ TEST_F(KeywordEditorControllerTest, MakeDefaultNoWebData) {
 // Mutates the TemplateURLService and make sure table model is updating
 // appropriately.
 TEST_F(KeywordEditorControllerTest, MutateTemplateURLService) {
-  TemplateURLData data;
-  data.short_name = ASCIIToUTF16("b");
-  data.SetKeyword(ASCIIToUTF16("a"));
-  TemplateURL* turl = new TemplateURL(data);
+  TemplateURL* turl = new TemplateURL();
+  turl->set_keyword(ASCIIToUTF16("a"));
+  turl->set_short_name(ASCIIToUTF16("b"));
   model_->Add(turl);
 
   // Table model should have updated.
diff --git a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
index 3065749..6c8d0cf 100644
--- a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
+++ b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
@@ -171,10 +171,11 @@ void SearchEngineTabHelper::GenerateKeywordIfNecessary(
     url_service->Remove(current_url);
   }
 
-  TemplateURLData data;
-  data.short_name = keyword;
-  data.SetKeyword(keyword);
-  data.SetURL(url.spec());
+  TemplateURL* new_url = new TemplateURL();
+  new_url->set_short_name(keyword);
+  new_url->set_keyword(keyword);
+  new_url->set_safe_for_autoreplace(true);
+  new_url->add_input_encoding(params.searchable_form_encoding);
   DCHECK(controller.GetLastCommittedEntry());
   const GURL& current_favicon =
       controller.GetLastCommittedEntry()->GetFavicon().url;
@@ -183,9 +184,9 @@ void SearchEngineTabHelper::GenerateKeywordIfNecessary(
   // latter.
   // TODO(sky): Need a way to set the favicon that doesn't involve generating
   // its url.
-  data.favicon_url = current_favicon.is_valid() ?
+  const GURL& favicon_url = current_favicon.is_valid() ?
       current_favicon : TemplateURL::GenerateFaviconURL(params.referrer.url);
-  data.safe_for_autoreplace = true;
-  data.input_encodings.push_back(params.searchable_form_encoding);
-  url_service->Add(new TemplateURL(data));
+  new_url->SetURL(url.spec());
+  new_url->set_favicon_url(favicon_url);
+  url_service->Add(new_url);
 }
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..c523ca5 100644
--- a/chrome/browser/ui/search_engines/template_url_table_model.cc
+++ b/chrome/browser/ui/search_engines/template_url_table_model.cc
@@ -254,11 +254,10 @@ void TemplateURLTableModel::Add(int index,
                                 const std::string& url) {
   DCHECK(index >= 0 && index <= RowCount());
   template_url_service_->RemoveObserver(this);
-  TemplateURLData data;
-  data.short_name = short_name;
-  data.SetKeyword(keyword);
-  data.SetURL(url);
-  TemplateURL* turl = new TemplateURL(data);
+  TemplateURL* turl = new TemplateURL();
+  turl->set_short_name(short_name);
+  turl->set_keyword(keyword);
+  turl->SetURL(url);
   template_url_service_->Add(turl);
   ModelEntry* entry = new ModelEntry(this, turl);
   template_url_service_->AddObserver(this);