[ios] Enables the BuiltinProvider.

ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc

 // Copyright 2015 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.
 
 #include "ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/keyed_service/core/service_access_type.h"
@@ skipping 100 matching lines @@
 
 std::string AutocompleteProviderClientImpl::GetAcceptLanguages() const {
   return browser_state_->GetPrefs()->GetString(prefs::kAcceptLanguages);
 }
 
 std::string
 AutocompleteProviderClientImpl::GetEmbedderRepresentationOfAboutScheme() {
   return kChromeUIScheme;
 }
 
 std::vector<base::string16> AutocompleteProviderClientImpl::GetBuiltinURLs() {

[Peter Kasting, 2017/06/19 21:00:43]

It looks like these two functions are basically copies of the ones in
ChromeAutocompleteProviderClient with the "if !ANDROID" stuff removed.

Is there any place we can put this code that will be shared so we don't need to
duplicate it?

[rohitrao (ping after 24h), 2017/06/20 12:08:20]

The Client interfaces exist to allow for embedder-specific logic, so almost by
definition there shouldn't be much shared code in these functions.

We could rework the interface to move more of the duplicated code into the
callers.  In this specific case, the std::sort() could move into the
BuiltinProvider constructor.  The rest of the duplicated code is just converting
from std::string to string16.

My take is that this code isn't really interesting enough (one sort, one string
conversion) to worry about deduplicating.  Let me know if you feel differently.

[Justin Donnelly, 2017/06/20 15:16:39]

I had the same reaction as Peter (ugh, duplication) but came to the same
conclusion as Rohit (there's really very little here that's being duplicated).

[Peter Kasting, 2017/06/21 01:10:15]

Is there any reason not to just change the API to fetch an unsorted list of
std::strings?  It seems like that's just as reasonable of an embedder API and
would allow for deduping these bits.  I agree that they're minor, but I don't
see much win in trying to keep them separate.

You're fine to land either way, but that would be my suggestion.

[rohitrao (ping after 24h), 2017/06/21 15:15:35]

I'll send this out as a followup; we can look at the diff and see how we feel.

-  // iOS does not supports BuiltinProvider.
-  return std::vector<base::string16>();
+  std::vector<std::string> chrome_builtins(
+      kChromeHostURLs,
+      kChromeHostURLs + kNumberOfChromeHostURLs);
+  std::sort(chrome_builtins.begin(), chrome_builtins.end());
+
+  std::vector<base::string16> builtins;
+  for (std::vector<std::string>::iterator i(chrome_builtins.begin());

[Peter Kasting, 2017/06/19 21:00:43]

Nit: Range-based for can help here

[rohitrao (ping after 24h), 2017/06/20 12:08:20]

Done.

+       i != chrome_builtins.end(); ++i)
+    builtins.push_back(base::ASCIIToUTF16(*i));
+  return builtins;
 }
 
 std::vector<base::string16>
 AutocompleteProviderClientImpl::GetBuiltinsToProvideAsUserTypes() {
-  // iOS does not supports BuiltinProvider.
-  return std::vector<base::string16>();
+  std::vector<base::string16> builtins_to_provide;
+  builtins_to_provide.push_back(base::ASCIIToUTF16(kChromeUIChromeURLsURL));
+  builtins_to_provide.push_back(base::ASCIIToUTF16(kChromeUIVersionURL));
+  return builtins_to_provide;

[Peter Kasting, 2017/06/19 21:00:43]

Nit: I bet you could just do:

  return {base::ASCIIToUTF16(kChromeUIChromeURLsURL),
          base::ASCIIToUTF16(kChromeUIVersionURL)};

[rohitrao (ping after 24h), 2017/06/20 12:08:20]

Done.

 }
 
 bool AutocompleteProviderClientImpl::IsOffTheRecord() const {
   return browser_state_->IsOffTheRecord();
 }
 
 bool AutocompleteProviderClientImpl::SearchSuggestEnabled() const {
   return browser_state_->GetPrefs()->GetBoolean(prefs::kSearchSuggestEnabled);
 }
 
@@ skipping 20 matching lines @@
     const base::string16& term) {
   GetHistoryService()->DeleteMatchingURLsForKeyword(keyword_id, term);
 }
 
 void AutocompleteProviderClientImpl::PrefetchImage(const GURL& url) {}
 
 void AutocompleteProviderClientImpl::OnAutocompleteControllerResultReady(
     AutocompleteController* controller) {
   // iOS currently has no client for this event.
 }