Remove use of stl_util in search_engines.

components/search_engines/template_url_service.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_
 #define COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_
 
 #include <stddef.h>
 
 #include <list>
@@ skipping 60 matching lines @@
 // TemplateURLService takes ownership of any TemplateURL passed to it. If there
 // is a KeywordWebDataService, deletion is handled by KeywordWebDataService,
 // otherwise TemplateURLService handles deletion.
 
 class TemplateURLService : public WebDataServiceConsumer,
                            public KeyedService,
                            public syncer::SyncableService {
  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;
 
   // We may want to treat the keyword in a TemplateURL as being a different
   // length than it actually is.  For example, for keywords that end in a
   // registry, e.g., '.com', we want to consider the registry characters as not
   // a meaningful part of the keyword and not penalize for the user not typing
   // those.)
   using TURLAndMeaningfulLength = std::pair<TemplateURL*, size_t>;
   using TURLsAndMeaningfulLengths = std::vector<TURLAndMeaningfulLength>;
@@ skipping 70 matching lines @@
   // retains ownership of it.
   TemplateURL* GetTemplateURLForGUID(const std::string& sync_guid);
 
   // Returns the first TemplateURL found with a URL using the specified |host|,
   // 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.  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.
   // This fails if the supplied template_url is the default search provider.
   void Remove(TemplateURL* template_url);
 
   // Removes any TemplateURL of the specified |type| associated with
   // |extension_id|. Unlike with Remove(), this can be called when the
   // TemplateURL in question is the current default search provider.
   void RemoveExtensionControlledTURL(const std::string& extension_id,
@@ skipping 290 matching lines @@
 
   // Helper fuction for adding an element to
   // |keyword_domain_to_turl_and_length_| if appropriate.
   void AddToDomainMap(TemplateURL* template_url);
 
   // Helper function for adding an element to |keyword_to_turl_and_length_|.
   void AddToMap(TemplateURL* template_url);
 
   // 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();
 
   // Called by DefaultSearchManager when the effective default search engine has
   // changed.
   void OnDefaultSearchChange(const TemplateURLData* new_dse_data,
                              DefaultSearchManager::Source source);
 
   // Applies a DSE change and reports metrics if appropriate.
@@ skipping 52 matching lines @@
 
   // Requests the Google URL tracker to check the server if necessary.
   void RequestGoogleURLTrackerServerCheckIfNecessary();
 
   // Invoked when the Google base URL has changed. Updates the mapping for all
   // TemplateURLs that have a replacement term of {google:baseURL} or
   // {google:baseSuggestURL}.
   void GoogleBaseURLChanged();
 
   // Adds a new TemplateURL to this model. TemplateURLService will own the
   // reference, and delete it when the TemplateURL is removed.

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

Nit: Second sentence here is now redundant.

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

Done.

   // 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.
   // 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);
+  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.
   // Caller is responsible for notifying observers.
   void RemoveNoNotify(TemplateURL* template_url);
 
   // Like ResetTemplateURL(), but instead of notifying observers, returns
   // whether anything has changed.
   bool ResetTemplateURLNoNotify(TemplateURL* url,
                                 const base::string16& title,
                                 const base::string16& keyword,
                                 const std::string& search_url);
 
   // Notify the observers that the model has changed.  This is done only if the
   // model is loaded.
   void NotifyObservers();
 
   // Updates |template_urls| so that the only "created by policy" entry is
   // |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
   // to the database. This does not notify observers.
   void ResetTemplateURLGUID(TemplateURL* url, const std::string& guid);
 
   // Attempts to generate a unique keyword for |turl| based on its original
   // keyword. If its keyword is already unique, that is returned. Otherwise, it
   // tries to return the autogenerated keyword if that is unique to the Service,
   // and finally it repeatedly appends special characters to the keyword until
@@ skipping 43 matching lines @@
   // This should only be called from MergeDataAndStartSyncing.
   void MergeInSyncTemplateURL(TemplateURL* sync_turl,
                               const SyncDataMap& sync_data,
                               syncer::SyncChangeList* change_list,
                               SyncDataMap* local_data,
                               syncer::SyncMergeResult* merge_result);
 
   // 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
   // replacement are returned.
   template <typename Container>
   void AddMatchingKeywordsHelper(
       const Container& keyword_to_turl_and_length,
       const base::string16& prefix,
       bool supports_replacement_only,
       TURLsAndMeaningfulLengths* matches);
@@ skipping 48 matching lines @@
   // template URL with an identical keyword, the template URL will not have an
   // entry in this map either.  This map will also not bother including entries
   // for keywords in which the keyword is the domain name, with no subdomain
   // before the domain name.  (The ordinary |keyword_to_turl_and_length|
   // suffices for that.)
   KeywordDomainToTURLAndMeaningfulLength keyword_domain_to_turl_and_length_;
 
   // Mapping from Sync GUIDs to the TemplateURL.
   GUIDToTURL guid_to_turl_;
 
-  TemplateURLVector template_urls_;
+  OwnedTemplateURLVector template_urls_;
 
   base::ObserverList<TemplateURLServiceObserver> model_observers_;
 
   // Maps from host to set of TemplateURLs whose search url host is host.
   // NOTE: This is always non-NULL; we use a std::unique_ptr<> to avoid circular
   // header dependencies.
   std::unique_ptr<SearchHostToURLsMap> provider_map_;
 
   // Whether the keywords have been loaded.
   bool loaded_;
@@ skipping 65 matching lines @@
   // Helper class to manage the default search engine.
   DefaultSearchManager default_search_manager_;
 
   std::unique_ptr<GoogleURLTracker::Subscription>
       google_url_updated_subscription_;
 
   DISALLOW_COPY_AND_ASSIGN(TemplateURLService);
 };
 
 #endif  // COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_