Remove use of stl_util in search_engines.

components/search_engines/template_url_service.h

Index: components/search_engines/template_url_service.h
diff --git a/components/search_engines/template_url_service.h b/components/search_engines/template_url_service.h
index 06124c5d6c1e5c12edb6950c55fd4b07291fd812..170d887a9cb2f347762cfec7223b74b09bfdf1b6 100644
--- a/components/search_engines/template_url_service.h
+++ b/components/search_engines/template_url_service.h
@@ -78,6 +78,7 @@ class TemplateURLService : public WebDataServiceConsumer,
  public:
   using QueryTerms = std::map<std::string, std::string>;
   using TemplateURLVector = std::vector<TemplateURL*>;
+  using OwnedTemplateURLVector = std::vector<std::unique_ptr<TemplateURL>>;
   using SyncDataMap = std::map<std::string, syncer::SyncData>;
   using Subscription = base::CallbackList<void(void)>::Subscription;
 
@@ -168,18 +169,18 @@ class TemplateURLService : public WebDataServiceConsumer,
   // 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.
   // Returns true if the Add is successful.
-  bool Add(TemplateURL* template_url);
+  bool Add(std::unique_ptr<TemplateURL> template_url);

[Peter Kasting, 2016/08/31 04:12:57]

I have an idea for a signature change here that will simplify most of the other
changes in this CL.

What if we change this declaration to the following:

  // Adds |template_url| to this model.  Returns a raw pointer to |template_url|
  // if the addition succeeded, or null on failure.  (Many callers need still
  // need a raw pointer to the TemplateURL so they can access it later.)
  TemplateURL* Add(std::unique_ptr<TemplateURL> template_url);

Now code that does this:

    TemplateURL* template_url = new TemplateURL(data);
    template_url_service->Add(base::WrapUnique(template_url));
    ...use |template_url|...

...can become:

    TemplateURL* template_url =
        template_url_service->Add(base::MakeUnique<TemplateURL>(data));

And code that does this:

    std::unique_ptr<TemplateURL> turl_ptr = CreateTURL();
    TemplateURL* turl = turl.get();
    model->Add(std::move(turl_ptr));
    ...use |template_url|...

...can become:

    TemplateURL* turl = model->Add(CreateTURL());

One could argue this is not only shorter and more readable but also safer, since
most of the existing callers don't check the return value of Add() before using
the raw pointer, which would point to deleted memory if the Add() failed; with
the new pattern, the returned pointer value is null, so things would fail in a
more obvious way.

[Avi (use Gerrit), 2016/09/01 00:34:27]

Pretty clever. Let's give it a try.

 
   // Like Add(), but overwrites the |template_url|'s values with the provided
   // ones.
-  void AddWithOverrides(TemplateURL* template_url,
+  void AddWithOverrides(std::unique_ptr<TemplateURL> template_url,
                         const base::string16& short_name,
                         const base::string16& keyword,
                         const std::string& url);
 
   // Adds a search engine with the specified info.
   void AddExtensionControlledTURL(
-      TemplateURL* template_url,
+      std::unique_ptr<TemplateURL> template_url,
       std::unique_ptr<TemplateURL::AssociatedExtensionInfo> info);
 
   // Removes the keyword from the model. This deletes the supplied TemplateURL.
@@ -490,11 +491,7 @@ class TemplateURLService : public WebDataServiceConsumer,
 
   // Sets the keywords. This is used once the keywords have been loaded.
   // This does NOT notify the delegate or the database.
-  //
-  // This transfers ownership of the elements in |urls| to |this|, and may
-  // delete some elements, so it's not safe for callers to access any elements
-  // after calling; to reinforce this, this function clears |urls| on exit.
-  void SetTemplateURLs(TemplateURLVector* urls);
+  void SetTemplateURLs(std::unique_ptr<OwnedTemplateURLVector> urls);
 
   // Transitions to the loaded state.
   void ChangeToLoadedState();
@@ -576,7 +573,8 @@ class TemplateURLService : public WebDataServiceConsumer,
   // 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);
+  bool AddNoNotify(std::unique_ptr<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.
@@ -598,7 +596,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   // |default_from_prefs|. |default_from_prefs| may be NULL if there is no
   // policy-defined DSE in effect.
   void UpdateProvidersCreatedByPolicy(
-      TemplateURLVector* template_urls,
+      OwnedTemplateURLVector* template_urls,
       const TemplateURLData* default_from_prefs);
 
   // Resets the sync GUID of the specified TemplateURL and persists the change
@@ -662,18 +660,10 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Goes through a vector of TemplateURLs and ensure that both the in-memory
   // and database copies have valid sync_guids. This is to fix crbug.com/102038,
   // where old entries were being pushed to Sync without a sync_guid.
-  void PatchMissingSyncGUIDs(TemplateURLVector* template_urls);
+  void PatchMissingSyncGUIDs(OwnedTemplateURLVector* template_urls);
 
   void OnSyncedDefaultSearchProviderGUIDChanged();
 
-  // Adds |template_urls| to |template_urls_|.
-  //
-  // This transfers ownership of the elements in |template_urls| to |this|, and
-  // may delete some elements, so it's not safe for callers to access any
-  // elements after calling; to reinforce this, this function clears
-  // |template_urls| on exit.
-  void AddTemplateURLs(TemplateURLVector* template_urls);
-
   // Adds to |matches| all TemplateURLs stored in |keyword_to_turl_and_length|
   // whose keywords begin with |prefix|, sorted shortest-keyword-first.  If
   // |supports_replacement_only| is true, only TemplateURLs that support
@@ -742,7 +732,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Mapping from Sync GUIDs to the TemplateURL.
   GUIDToTURL guid_to_turl_;
 
-  TemplateURLVector template_urls_;
+  OwnedTemplateURLVector template_urls_;
 
   base::ObserverList<TemplateURLServiceObserver> model_observers_;