[Payments] Added region load failure tolerance and tests to PR editor.

chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc

 // Copyright 2017 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/views/payments/shipping_address_editor_view_controller.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 173 matching lines @@
       country_codes_.clear();
       for (size_t i = 0; i < model->countries().size(); ++i) {
         if (model->countries()[i].get())
           country_codes_.push_back(model->countries()[i]->country_code());
         else
           country_codes_.push_back("");  // Separator.
       }
       return std::unique_ptr<ui::ComboboxModel>(model.release());
     }
     case autofill::ADDRESS_HOME_STATE: {
-      return std::unique_ptr<
-          ui::ComboboxModel>(new autofill::RegionComboboxModel(
+      std::unique_ptr<autofill::RegionComboboxModel> model = base::MakeUnique<
+          autofill::RegionComboboxModel>(
           base::WrapUnique(new autofill::ChromeMetadataSource(
               I18N_ADDRESS_VALIDATION_DATA_URL,
               state()->GetPersonalDataManager()->GetURLRequestContextGetter())),
           autofill::ValidationRulesStorageFactory::CreateStorage(),
           state()->GetApplicationLocale(),
-          country_codes_[chosen_country_index_]));
+          country_codes_[chosen_country_index_]);
+      // If the data was already pre-loaded, the observer won't get notified so
+      // we have to check for failure here.
+      if (!model->pending_region_data_load()) {
+        failed_to_load_region_data_ = model->failed_to_load_data();
+        if (failed_to_load_region_data_)
+          OnDataChanged();
+      }
+      return std::unique_ptr<ui::ComboboxModel>(model.release());

[please use gerrit instead, 2017/04/11 21:05:35]

This should be "return std::move(model);" instead. Otherwise the code creates an
extra std::unique_ptr, which is not necessary.

[MAD, 2017/04/12 02:46:07]

Done.

     }
     default:
       NOTREACHED();
       break;
   }
   return std::unique_ptr<ui::ComboboxModel>();
 }
 
 void ShippingAddressEditorViewController::OnPerformAction(
     views::Combobox* sender) {
   EditorViewController::OnPerformAction(sender);
   if (sender->id() != autofill::ADDRESS_HOME_COUNTRY)
     return;
   DCHECK_GE(sender->selected_index(), 0);
   if (chosen_country_index_ != static_cast<size_t>(sender->selected_index())) {
     chosen_country_index_ = sender->selected_index();
-    OnCountryChanged(sender);
+    failed_to_load_region_data_ = false;
+    OnDataChanged();
   }
 }
 
 void ShippingAddressEditorViewController::UpdateEditorView() {
   EditorViewController::UpdateEditorView();
   if (chosen_country_index_ > 0UL) {
     views::Combobox* country_combo_box = static_cast<views::Combobox*>(
         dialog()->GetViewByID(autofill::ADDRESS_HOME_COUNTRY));
     DCHECK(country_combo_box);
     country_combo_box->SetSelectedIndex(chosen_country_index_);
   }
 }
 
 base::string16 ShippingAddressEditorViewController::GetSheetTitle() {
   return l10n_util::GetStringUTF16(
       IDS_PAYMENT_REQUEST_ADDRESS_EDITOR_ADD_TITLE);
 }
 
 void ShippingAddressEditorViewController::UpdateEditorFields() {
   editor_fields_.clear();
   std::string chosen_country_code;
   if (chosen_country_index_ < country_codes_.size())
     chosen_country_code = country_codes_[chosen_country_index_];
 
   std::unique_ptr<base::ListValue> components(new base::ListValue);
   std::string unused;
   autofill::GetAddressComponents(chosen_country_code,
                                  state()->GetApplicationLocale(),
                                  components.get(), &unused);
-
   for (size_t line_index = 0; line_index < components->GetSize();
        ++line_index) {
     const base::ListValue* line = nullptr;
     if (!components->GetList(line_index, &line)) {
       NOTREACHED();
       return;
     }
     DCHECK_NE(nullptr, line);
     for (size_t component_index = 0; component_index < line->GetSize();
          ++component_index) {
@@ skipping 20 matching lines @@
       EditorField::LengthHint length_hint = EditorField::LengthHint::HINT_LONG;
       if (field_length == autofill::kShortField)
         length_hint = EditorField::LengthHint::HINT_SHORT;
       else
         DCHECK_EQ(autofill::kLongField, field_length);
       autofill::ServerFieldType server_field_type =
           GetFieldTypeFromString(field_type);
       EditorField::ControlType control_type =
           EditorField::ControlType::TEXTFIELD;
       if (server_field_type == autofill::ADDRESS_HOME_COUNTRY ||
-          server_field_type == autofill::ADDRESS_HOME_STATE) {
+          (server_field_type == autofill::ADDRESS_HOME_STATE &&
+           !failed_to_load_region_data_)) {
         control_type = EditorField::ControlType::COMBOBOX;
       }
       editor_fields_.emplace_back(
           server_field_type, base::UTF8ToUTF16(field_name), length_hint,
           /*required=*/server_field_type != autofill::COMPANY_NAME,
           control_type);
       // Insert the Country combobox right after NAME_FULL.
       if (server_field_type == autofill::NAME_FULL) {
         editor_fields_.emplace_back(
             autofill::ADDRESS_HOME_COUNTRY,
             l10n_util::GetStringUTF16(
                 IDS_LIBADDRESSINPUT_COUNTRY_OR_REGION_LABEL),
             EditorField::LengthHint::HINT_LONG, /*required=*/true,
             EditorField::ControlType::COMBOBOX);
       }
     }
   }
   // Always add phone number at the end.
   editor_fields_.emplace_back(
       autofill::PHONE_HOME_NUMBER,
       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_PHONE),
       EditorField::LengthHint::HINT_LONG, /*required=*/false,
       EditorField::ControlType::TEXTFIELD);
 }
 
-void ShippingAddressEditorViewController::OnCountryChanged(
-    views::Combobox* combobox) {
+void ShippingAddressEditorViewController::OnDataChanged() {
   // TODO(crbug.com/703764): save the current state so we can map it to the new
   // country fields as best we can.
   UpdateEditorFields();
 
   // The editor can't be updated while in the middle of a combobox event.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&ShippingAddressEditorViewController::UpdateEditorView,
                  base::Unretained(this)));
 }
 
+void ShippingAddressEditorViewController::OnComboboxModelChanged(
+    views::Combobox* combobox) {
+  if (combobox->id() != autofill::ADDRESS_HOME_STATE)
+    return;
+  autofill::RegionComboboxModel* model =
+      static_cast<autofill::RegionComboboxModel*>(combobox->model());
+  if (model->pending_region_data_load())
+    return;
+  if (model->failed_to_load_data()) {
+    failed_to_load_region_data_ = true;
+    OnDataChanged();
+  }
+}
+
 ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
     ShippingAddressValidationDelegate(
         ShippingAddressEditorViewController* controller,
         const EditorField& field)
     : field_(field), controller_(controller) {}
 
 ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
     ~ShippingAddressValidationDelegate() {}
 
 bool ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
     ValidateTextfield(views::Textfield* textfield) {
   return ValidateValue(textfield->text());
 }
 
 bool ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
     ValidateCombobox(views::Combobox* combobox) {
   return ValidateValue(combobox->GetTextForRow(combobox->selected_index()));
 }
 
+void ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
+    ComboboxModelChanged(views::Combobox* combobox) {
+  controller_->OnComboboxModelChanged(combobox);
+}
+
 bool ShippingAddressEditorViewController::ShippingAddressValidationDelegate::
     ValidateValue(const base::string16& value) {
   if (!value.empty()) {
     if (field_.type == autofill::PHONE_HOME_NUMBER &&
         !autofill::IsValidPhoneNumber(
             value,
             controller_->country_codes_[controller_->chosen_country_index_])) {
       controller_->DisplayErrorMessageForField(
           field_, l10n_util::GetStringUTF16(
                       IDS_PAYMENTS_PHONE_INVALID_VALIDATION_MESSAGE));
       return false;
     }
     // As long as other field types are non-empty, they are valid.
     controller_->DisplayErrorMessageForField(field_, base::ASCIIToUTF16(""));
     return true;
   }
 
   bool is_required_valid = !field_.required;
   const base::string16 displayed_message =
       is_required_valid ? base::ASCIIToUTF16("")
                         : l10n_util::GetStringUTF16(
                               IDS_PAYMENTS_FIELD_REQUIRED_VALIDATION_MESSAGE);
   controller_->DisplayErrorMessageForField(field_, displayed_message);
   return is_required_valid;
 }
 
 }  // namespace payments