[ios] Enables the BuiltinProvider.

ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc

Index: ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
diff --git a/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc b/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
index 345b7a2d7a6309804c8efc78c290133eead9fca6..4a08d63899ce59e33124205255ce9e5f2f450230 100644
--- a/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
+++ b/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
@@ -119,14 +119,24 @@ AutocompleteProviderClientImpl::GetEmbedderRepresentationOfAboutScheme() {
 }
 
 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 {