Remove the "autogenerate keyword" bit on TemplateURL.

chrome/browser/search_engines/template_url_service.h

Index: chrome/browser/search_engines/template_url_service.h
===================================================================
--- chrome/browser/search_engines/template_url_service.h	(revision 135424)
+++ chrome/browser/search_engines/template_url_service.h	(working copy)
@@ -83,12 +83,10 @@
   TemplateURLService(const Initializer* initializers, const int count);
   virtual ~TemplateURLService();
 
-  // Generates a suitable keyword for the specified url.  Returns an empty
-  // string if a keyword couldn't be generated.  If |autodetected| is true, we
-  // don't generate keywords for a variety of situations where we would probably
-  // not want to auto-add keywords, such as keywords for searches on pages that
-  // themselves come from form submissions.
-  static string16 GenerateKeyword(const GURL& url, bool autodetected);
+  // Generates a suitable keyword for the specified url, which must be valid.
+  // This is guaranteed not to return an empty string, since TemplateURLs should
+  // never have an empty keyword.
+  static string16 GenerateKeyword(const GURL& url);

[sky, 2012/05/04 23:20:25]

Can you add a comment to this and GenerateSearchURLUsingTermsData that they are
invoked from non-ui threads.

[Peter Kasting, 2012/05/07 17:50:24]

Is there a reason to call that out for this function?  It's so simple now that
it doesn't matter whether it's restricted to the UI thread or not.

I added a comment to GenerateSearchURLUsingTermsData().

 
   // Removes any unnecessary characters from a user input keyword.
   // This removes the leading scheme, "www." and any trailing slash.
@@ -140,7 +138,8 @@
   // or NULL if there are no such TemplateURLs
   TemplateURL* GetTemplateURLForHost(const std::string& host);
 
-  // Takes ownership of |template_url| and adds it to this model.
+  // Takes ownership of |template_url| and adds it to this model.  For obvious
+  // reasons, it is illegal to Add() the same |template_url| pointer twice.
   void Add(TemplateURL* template_url);
 
   // Like Add(), but overwrites the |template_url|'s values with the provided
@@ -413,6 +412,11 @@
   // in the default list and is marked as safe_for_autoreplace.
   bool CanReplace(const TemplateURL* t_url);
 
+  // It's possible for an extension TemplateURL to mask a replaceable
+  // non-extension TemplateURL with the same keyword (see comments in
+  // AddToMaps()).  Find the masked TemplateURL, if any.

[SteveT, 2012/05/07 18:17:47]

This returns NULL if no such masked TemplateURL exists?

[Peter Kasting, 2012/05/07 19:39:14]

I rewrote this comment entirely in more recent patches.

+  TemplateURL* FindNonExtensionTemplateURLForKeyword(const string16& keyword);
+
   // Updates the information in |existing_turl| using the information from
   // |new_values|, but the ID for |existing_turl| is retained.
   // Notifying observers is the responsibility of the caller.
@@ -459,8 +463,12 @@
   // If |newly_adding| is false, we assume that this TemplateURL was already
   // part of the model in the past, and therefore we don't need to do things
   // like assign it an ID or notify sync.
-  // Caller is responsible for notifying observers.
-  void AddNoNotify(TemplateURL* template_url, bool newly_adding);
+  // This function guarantees that on return the model will not have two
+  // non-extension TemplateURLs with the same keyword.  If that means that it
+  // cannot add the provided argument, it will delete it and return false.
+  // Caller is responsible for notifying observers if this function returns
+  // true.
+  bool AddNoNotify(TemplateURL* template_url, bool newly_adding);
 
   // Removes the keyword from the model. This deletes the supplied TemplateURL.
   // This fails if the supplied template_url is the default search provider.
@@ -491,15 +499,14 @@
   // it is unique to the Service.
   string16 UniquifyKeyword(const TemplateURL& turl);
 
-  // Given a TemplateURL from Sync (cloud), resolves any keyword conflicts by
-  // checking the local keywords and uniquifying either the cloud keyword or a
-  // conflicting local keyword (whichever is older). If the cloud TURL is
-  // changed, then an appropriate SyncChange is appended to |change_list|. If
-  // a local TURL is changed, the service is updated with the new keyword. If
-  // there was no conflict to begin with, this does nothing. In the case of tied
-  // last_modified dates, |sync_turl| wins. Returns true iff there was a
-  // conflict.
-  bool ResolveSyncKeywordConflict(TemplateURL* sync_turl,
+  // Given a TemplateURL from Sync (cloud) and a local TemplateURL with the same
+  // keyword, resolves the conflict by uniquifying either the cloud keyword or
+  // the local keyword (whichever is older). If the cloud TURL is changed, then
+  // an appropriate SyncChange is appended to |change_list|. If a local TURL is
+  // changed, the service is updated with the new keyword. In the case of tied
+  // last_modified dates, |sync_turl| wins.
+  void ResolveSyncKeywordConflict(TemplateURL* sync_turl,
+                                  TemplateURL* local_turl,
                                   SyncChangeList* change_list);
 
   // Returns a TemplateURL from the service that has the same keyword and search