[Payment Request] Populates region field if region code/name is valid.

ios/chrome/browser/ui/payments/address_edit_mediator.mm

 // 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.
 
 #import "ios/chrome/browser/ui/payments/address_edit_mediator.h"
 
 #include <map>
 #include <memory>
 #include <string>
 #include <utility>
@@ skipping 50 matching lines @@
 @property(nonatomic, strong) EditorField* regionField;
 
 @end
 
 @implementation AddressEditMediator
 
 @synthesize state = _state;
 @synthesize consumer = _consumer;
 @synthesize countries = _countries;
 @synthesize selectedCountryCode = _selectedCountryCode;
-@synthesize regions = _regions;
 @synthesize paymentRequest = _paymentRequest;
 @synthesize address = _address;
 @synthesize fieldsMap = _fieldsMap;
 @synthesize fields = _fields;
 @synthesize regionField = _regionField;
 
 - (instancetype)initWithPaymentRequest:(PaymentRequest*)paymentRequest
                                address:(autofill::AutofillProfile*)address {
   self = [super init];
   if (self) {
@@ skipping 37 matching lines @@
   return NO;
 }
 
 - (UIImage*)iconIdentifyingEditorField:(EditorField*)field {
   return nil;
 }
 
 #pragma mark - RegionDataLoaderConsumer
 
 - (void)regionDataLoaderDidSucceedWithRegions:
-    (NSMutableArray<NSString*>*)regions {
-  self.regions = regions;
+    (NSDictionary<NSString*, NSString*>*)regions {
   // Enable the previously disabled field.
   self.regionField.enabled = YES;
 
-  // If an address is being edited and it has a valid region, the field value is
-  // set to that region. Otherwise, set the field value to nil. If creating an
-  // address, set the first available region as the field value, if possible.
+  // If an address is being edited and it has a valid region code or name, the
+  // field value is set to that respective region code. Otherwise, it is set to
+  // nil.
+  self.regionField.value = nil;
   if (self.address) {
     NSString* region =
         [self fieldValueFromProfile:self.address
                           fieldType:autofill::ADDRESS_HOME_STATE];
-    self.regionField.value =
-        [self.regions containsObject:region] ? region : nil;
-  } else {
-    self.regionField.value = regions.count ? regions[0] : nil;
+    if ([regions objectForKey:region])

[macourteau, 2017/06/20 16:24:59]

This block needs more documentation:
- what are the keys and values in regions?
- why is |region| used as either a key or as a value when searching in
|regions|?

My understanding after looking at this for too long: |regions| maps codes to
names. |region| could be either a code or a name, but self.regionField.value
needs to be a code. If we find the |region| key in |regions|, then set it's a
valid code and set it to that. Otherwise, if we find a value that matches
|region|, use the key for that value as the code.

[Moe, 2017/06/20 18:28:34]

Done.

+      self.regionField.value = region;
+    else if ([[regions allKeysForObject:region] count])

[macourteau, 2017/06/20 16:24:59]

Might want to check that count returns exactly one, unless there's a valid
reason why it wouldn't? If if there is, how do we decide which key is the
correct one to use?

[Moe, 2017/06/20 18:28:33]

There should be a one to one mapping between region codes and region names.
autofill::RegionComboboxModel should be unit tested for that. If for some reason
there's a many to one mapping from region codes to region names, there is no way
for us to know which code is the correct code for the given name, so picking the
first one should be good enough.  

[macourteau, 2017/06/20 18:31:30]

Right, so then maybe add a DCHECK that count is only one?

+      self.regionField.value = [regions allKeysForObject:region][0];

[please use gerrit instead, 2017/06/20 15:38:31]

Please get this reviewed by someone who can read Objective C :-P

[Moe, 2017/06/20 18:58:35]

Done.

   }
 
   // Notify the view controller asynchronously to allow for the view to update.
   __weak AddressEditMediator* weakSelf = self;
   dispatch_async(dispatch_get_main_queue(), ^{
-    [weakSelf.consumer setOptions:weakSelf.regions
+    [weakSelf.consumer setOptions:[regions allKeys]
                    forEditorField:weakSelf.regionField];
   });
 }
 
 #pragma mark - Helper methods
 
 // Loads the country codes and names and sets the default selected country code.
 - (void)loadCountries {
   autofill::CountryComboboxModel countryModel;
   countryModel.SetCountries(*_paymentRequest->GetPersonalDataManager(),
@@ skipping 153 matching lines @@
 // corresponding field value. Returns nil if |profile| is nullptr.
 - (NSString*)fieldValueFromProfile:(autofill::AutofillProfile*)profile
                          fieldType:(autofill::ServerFieldType)fieldType {
   return profile ? base::SysUTF16ToNSString(profile->GetInfo(
                        autofill::AutofillType(fieldType),
                        GetApplicationContext()->GetApplicationLocale()))
                  : nil;
 }
 
 @end