[Android] Sort custom search engines based on last visited time and display only top 3 most recentl…

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 119 matching lines @@
   // |url| is the URL of the search query.  This is used to prevent auto-adding
   // a keyword for hosts already associated with a manually-edited keyword.
   bool CanAddAutogeneratedKeyword(const base::string16& keyword,
                                   const GURL& url,
                                   TemplateURL** template_url_to_replace);
 
   // Returns whether the engine is a "pre-existing" engine, either from the
   // prepopulate list or created by policy.
   bool IsPrepopulatedOrCreatedByPolicy(const TemplateURL* template_url);
 
-  // Returns whether |template_url| is the current default or return true for
-  // IsPrepopulatedOrCreatedByPolicy(). This is meant to highlight the current,
-  // default, as well as the other most likely choices of default engine,
-  // separately from a full list of all TemplateURLs (which might be very long).
+  // Returns whether |template_url| should be shown in the list of engines
+  // most likely to be selected as a default engine. This is meant to highlight
+  // the current default, as well as the other most likely choices of default
+  // engine, separately from a full list of all TemplateURLs (which might be
+  // very long).
   bool ShowInDefaultList(const TemplateURL* template_url);
 
   // Adds to |matches| all TemplateURLs whose keywords begin with |prefix|,
   // sorted shortest-keyword-first. If |supports_replacement_only| is true, only
   // TemplateURLs that support replacement are returned.
   void AddMatchingKeywords(const base::string16& prefix,
                            bool supports_replacement_only,
                            TURLsAndMeaningfulLengths* matches);
 
   // Adds to |matches| all TemplateURLs for search engines with the domain
@@ skipping 216 matching lines @@
   // Returns a SearchTermsData which can be used to call TemplateURL methods.
   const SearchTermsData& search_terms_data() const {
     return *search_terms_data_;
   }
 
   // Returns a SyncData with a sync representation of the search engine data
   // from |turl|.
   static syncer::SyncData CreateSyncDataFromTemplateURL(
       const TemplateURL& turl);
 
+  // Updates the last_visited time of |url| to the current time.
+  void UpdateTemplateURLVisitTime(TemplateURL* url);

[Peter Kasting, 2016/12/16 08:37:18]

Nit: If you're going to move this, you need to move the definition to match. 
.cc and .h files must stay in the same order.

Consider also where in this list this should go; try to group related things. 
In this case, you've inserted your method right between two "create XXX" static
methods for sync that are closely related to each other, which doesn't make
sense.

Looking at how you use these, it's merely being exposed to a test-only function
on TemplateUrlServiceAndroid, which is almost an implementation-detail class. 
It probably makes sense to leave it where it was and expose via a friend
declaration instead, since we don't really want anyone else calling this method.

+
   // Creates a new heap-allocated TemplateURL* which is populated by overlaying
   // |sync_data| atop |existing_turl|.  |existing_turl| may be NULL; if not it
   // remains unmodified.  The caller owns the returned TemplateURL*.
   //
   // If the created TemplateURL is migrated in some way from out-of-date sync
   // data, an appropriate SyncChange is added to |change_list|.  If the sync
   // data is bad for some reason, an ACTION_DELETE change is added and the
   // function returns NULL.
   static std::unique_ptr<TemplateURL>
   CreateTemplateURLFromTemplateURLAndSyncData(
@@ skipping 172 matching lines @@
   // If the TemplateURL's sync GUID matches the kSyncedDefaultSearchProviderGUID
   // preference it will be used to update the DSE in memory and as persisted in
   // preferences.
   void MaybeUpdateDSEAfterSync(TemplateURL* synced_turl);
 
   // Iterates through the TemplateURLs to see if one matches the visited url.
   // For each TemplateURL whose url matches the visited url
   // SetKeywordSearchTermsForURL is invoked.
   void UpdateKeywordSearchTermsForURL(const URLVisitedDetails& details);
 
-  // Updates the last_visited time of |url| to the current time.
-  void UpdateTemplateURLVisitTime(TemplateURL* url);
-
   // If necessary, generates a visit for the site http:// + t_url.keyword().
   void AddTabToSearchVisit(const TemplateURL& t_url);
 
   // 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();
@@ skipping 247 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_