rAc - Only show countries we're able to fill in.

chrome/browser/ui/autofill/country_combobox_model.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/autofill/country_combobox_model.h"
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "components/autofill/core/browser/autofill_country.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "ui/base/l10n/l10n_util_collator.h"
 #include "ui/base/models/combobox_model_observer.h"
 
 // TODO(rouslan): Remove this check. http://crbug.com/337587
 #if defined(ENABLE_AUTOFILL_DIALOG)
 #include "chrome/browser/ui/autofill/autofill_dialog_i18n_input.h"
 #include "third_party/libaddressinput/chromium/cpp/include/libaddressinput/address_ui.h"
 #endif
 
 namespace autofill {
 
 namespace {
 
-bool ShouldShowCountry(const std::string& country_code) {
+bool ShouldShowCountry(const std::string& country_code,
+                       bool show_partially_supported_countries,
+                       const std::set<base::string16> candidate_countries) {

[Dan Beam, 2014/03/01 03:38:10]

nit: const-ref

[Evan Stade, 2014/03/04 00:11:30]

good catch, done.

 #if defined(ENABLE_AUTOFILL_DIALOG)
-  return i18ninput::CountryIsFullySupported(country_code);
-#else
+  if (!show_partially_supported_countries &&
+      !i18ninput::CountryIsFullySupported(country_code)) {
+    return false;
+  }
+#endif
+

[Dan Beam, 2014/03/01 03:38:10]

if (candidate_countries.empty())
  return true;

// the rest...

[Evan Stade, 2014/03/04 00:11:30]

not really an improvement, but done

[Dan Beam, 2014/03/04 00:19:50]

doesn't construct an AutofillCountry needlessly...

+  AutofillCountry country(country_code,
+                          g_browser_process->GetApplicationLocale());
+  if (!candidate_countries.empty() &&
+      !candidate_countries.count(base::ASCIIToUTF16(country_code)) &&
+      !candidate_countries.count(country.name())) {
+    return false;
+  }

[Ilya Sherman, 2014/03/03 23:55:54]

What happens if the <select> dropdown includes localized country names, and
doesn't use country codes for the option values?  For example, Wikipedia
generally lists each country name as it is written by residents of that country,
character set and all.  I'd expect that you'd want to extend candidate_countries
to contain all of the country codes that we can deduce from existing members of
the set.

[Evan Stade, 2014/03/04 00:11:30]

what happens is that requestAutocomplete won't work properly. But given that
we're creating a new "standard" and authors are coding to our implementation,
they should just use the country codes. In fact, I'd be willing to remove the
country.name() check altogether.

[Dan Beam, 2014/03/04 00:19:50]

in general i think this was a cool idea, but +1 to just removing name check
assuming it makes our code faster/easier to understand and eases cognitive load
on web authors.

[Ilya Sherman, 2014/03/04 00:53:57]

IMO we should either only support country codes, or try harder to support
localized names.  I'd expect the in-between state that this CL puts us in to be
confusing to developers, who'd essentially see that some fraction of their
countries aren't usable, and then they'd have to debug a black box to figure out
why.

+
   return true;
-#endif
 }
 
 }  // namespace
 
 CountryComboboxModel::CountryComboboxModel(
     const PersonalDataManager& manager,
+    const std::set<base::string16>& country_filter,
     bool show_partially_supported_countries) {
   // Insert the default country at the top as well as in the ordered list.
-  const std::string& app_locale = g_browser_process->GetApplicationLocale();
   std::string default_country_code =
       manager.GetDefaultCountryCodeForNewAddress();
   DCHECK(!default_country_code.empty());
 
-  if (show_partially_supported_countries ||
-      ShouldShowCountry(default_country_code)) {
+  const std::string& app_locale = g_browser_process->GetApplicationLocale();
+  if (ShouldShowCountry(default_country_code,
+                        show_partially_supported_countries,
+                        country_filter)) {
     countries_.push_back(new AutofillCountry(default_country_code, app_locale));
     // The separator item.
     countries_.push_back(NULL);
   }
 
   // The sorted list of countries.
 #if defined(ENABLE_AUTOFILL_DIALOG)
   const std::vector<std::string>& available_countries =
       ::i18n::addressinput::GetRegionCodes();
 #else
   std::vector<std::string> available_countries;
   AutofillCountry::GetAvailableCountries(&available_countries);
 #endif
 
   std::vector<AutofillCountry*> sorted_countries;
   for (std::vector<std::string>::const_iterator it =
            available_countries.begin(); it != available_countries.end(); ++it) {
-    if (show_partially_supported_countries || ShouldShowCountry(*it))
+    if (ShouldShowCountry(*it,
+                          show_partially_supported_countries,
+                          country_filter)) {
       sorted_countries.push_back(new AutofillCountry(*it, app_locale));
+    }
   }
 
   l10n_util::SortStringsUsingMethod(app_locale,
                                     &sorted_countries,
                                     &AutofillCountry::name);
   countries_.insert(countries_.end(),
                     sorted_countries.begin(),
                     sorted_countries.end());
 }
 
@@ skipping 15 matching lines @@
 
 bool CountryComboboxModel::IsItemSeparatorAt(int index) {
   return !countries_[index];
 }
 
 std::string CountryComboboxModel::GetDefaultCountryCode() const {
   return countries_[GetDefaultIndex()]->country_code();
 }
 
 }  // namespace autofill