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

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

Index: ios/chrome/browser/ui/payments/address_edit_mediator.mm
diff --git a/ios/chrome/browser/ui/payments/address_edit_mediator.mm b/ios/chrome/browser/ui/payments/address_edit_mediator.mm
index 568608fb8338ecb9c4b46d5c49626704c24134e4..d80869ce998b72e5bcfc82662726b275d1f37679 100644
--- a/ios/chrome/browser/ui/payments/address_edit_mediator.mm
+++ b/ios/chrome/browser/ui/payments/address_edit_mediator.mm
@@ -68,7 +68,6 @@
 @synthesize consumer = _consumer;
 @synthesize countries = _countries;
 @synthesize selectedCountryCode = _selectedCountryCode;
-@synthesize regions = _regions;
 @synthesize paymentRequest = _paymentRequest;
 @synthesize address = _address;
 @synthesize fieldsMap = _fieldsMap;
@@ -126,28 +125,28 @@
 #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];
   });
 }