[Payments] Avoid a crash caused by AddressValidatorTest.

third_party/libaddressinput/chromium/chrome_address_validator.cc

Index: third_party/libaddressinput/chromium/chrome_address_validator.cc
diff --git a/third_party/libaddressinput/chromium/chrome_address_validator.cc b/third_party/libaddressinput/chromium/chrome_address_validator.cc
index f76bcbd15fda6b170d7941f5f63842d5ea49afff..70d614a2b13469995edaaf036fd029450809eb40 100644
--- a/third_party/libaddressinput/chromium/chrome_address_validator.cc
+++ b/third_party/libaddressinput/chromium/chrome_address_validator.cc
@@ -163,10 +163,14 @@ void AddressValidator::RulesLoaded(bool success,
   if (success || attempts_number_[region_code] + 1 >= kMaxAttemptsNumber)
     return;
 
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, base::Bind(&AddressValidator::RetryLoadRules,
-                            weak_factory_.GetWeakPtr(), region_code),
-      GetBaseRetryPeriod() * pow(2, attempts_number_[region_code]++));
+  // No need to retry if it's synchronous (which means that it's a test.)
+  if (base::ThreadTaskRunnerHandle::IsSet()) {

[sebsg, 2017/06/23 17:26:04]

I don't think we should modify the production code for the sake of the test. I
think you fixed the crash by loading "OZ", and it make much more sense for that
test.

If you do this the test will still fail though, because in
OnAddressValidationRulesLoaded of the test class, it expects the rules to be
loaded successfully. You'll need to change that assumption to depend on a
variable, since in the case of this test is should be RULES_UNAVAILABLE.  I'd be
much more comfortable with that change since it only affects the test class.

Thanks!

[Hirondelle, 2017/06/23 20:30:11]

Anyway, if it's synchronous, the next line would crash, and if it's async, this
line would not make a difference. But at the end, if you insist that it's not a
good design, we can remove the test.
No. The crash was fixed by adding this line. That change was introduced later
(when I was taking a look at countryinfo.txt out of curiosity, and noticed
inconsistent data on "ZZ", and changed this to avoid confusion. Also, because OZ
reminds me of the "Wizard of Oz" ;) )
No. The OnAddressValidationRulesLoaded checks for 
ASSERT_EQ(success, status == AddressValidator::SUCCESS) and NOT for
ASSERT_EQ(success, AddressValidator::SUCCESS). 

which is mostly for the sake of testing Validate(). This class doesn't require
success == 1.
I was confused for a good while by this line, the tests passing, and success =
0! 

You'll need to change that assumption to depend on a

[sebsg, 2017/06/23 21:59:32]

What I suggest is changing the expected status for the test. Right now it's
hardcoded (line 82) but you can change it. You can make it be RulesUnavailable
for the SubKeysNotExist test. It seems like it works on my machine.

[Parastoo, 2017/06/27 18:14:31]

Done.

+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&AddressValidator::RetryLoadRules,
+                   weak_factory_.GetWeakPtr(), region_code),
+        GetBaseRetryPeriod() * pow(2, attempts_number_[region_code]++));
+  }
 }
 
 void AddressValidator::RetryLoadRules(const std::string& region_code) {