Retry downloading rules for libaddressinput.

Retry downloading rules after 8, 16, 32, 64, 128, 256, and 512 seconds,
while requestAutocomplete dialog is open. (512 seconds is ~8.5 minutes.)

TEST=libaddressinput_unittests:FailingAddressValidatorTest.*
BUG=343397

Review URL: https://codereview.chromium.org/392083002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283712 0039d316-1c4b-4281-b951-d872f2087c98

third_party/libaddressinput/chromium/chrome_address_validator.cc

@@ -4,26 +4,21 @@
 
 #include "third_party/libaddressinput/chromium/chrome_address_validator.h"
 
-#include <cstddef>
-#include <string>
-#include <vector>
+#include <cmath>
 
-#include "base/basictypes.h"
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/logging.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "third_party/libaddressinput/chromium/addressinput_util.h"
 #include "third_party/libaddressinput/chromium/input_suggester.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_field.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_normalizer.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_validator.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/callback.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/downloader.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/preload_supplier.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/storage.h"
 
 namespace autofill {
+namespace {
 
 using ::i18n::addressinput::AddressData;
 using ::i18n::addressinput::AddressField;
@@ -38,6 +33,11 @@ using ::i18n::addressinput::ADMIN_AREA;
 using ::i18n::addressinput::DEPENDENT_LOCALITY;
 using ::i18n::addressinput::POSTAL_CODE;
 
+// The maximum number attempts to load rules.
+static const int kMaxAttemptsNumber = 8;
+
+}  // namespace
+
 AddressValidator::AddressValidator(const std::string& validation_data_url,
                                    scoped_ptr<Downloader> downloader,
                                    scoped_ptr<Storage> storage,
@@ -50,12 +50,13 @@ AddressValidator::AddressValidator(const std::string& validation_data_url,
       validator_(new ::i18n::addressinput::AddressValidator(supplier_.get())),
       validated_(BuildCallback(this, &AddressValidator::Validated)),
       rules_loaded_(BuildCallback(this, &AddressValidator::RulesLoaded)),
-      load_rules_listener_(load_rules_listener) {}
+      load_rules_listener_(load_rules_listener),
+      weak_factory_(this) {}
 
 AddressValidator::~AddressValidator() {}
 
 void AddressValidator::LoadRules(const std::string& region_code) {
-  DCHECK(supplier_);
+  attempts_number_[region_code] = 0;
   supplier_->LoadRules(region_code, *rules_loaded_);
 }
 
@@ -63,9 +64,6 @@ AddressValidator::Status AddressValidator::ValidateAddress(
     const AddressData& address,
     const FieldProblemMap* filter,
     FieldProblemMap* problems) const {
-  DCHECK(supplier_);
-  DCHECK(validator_);
-
   if (supplier_->IsPending(address.region_code)) {
     if (problems)
       addressinput::ValidateRequiredFields(address, filter, problems);
@@ -96,9 +94,6 @@ AddressValidator::Status AddressValidator::GetSuggestions(
     AddressField focused_field,
     size_t suggestion_limit,
     std::vector<AddressData>* suggestions) const {
-  DCHECK(supplier_);
-  DCHECK(input_suggester_);
-
   if (supplier_->IsPending(user_input.region_code))
     return RULES_NOT_READY;
 
@@ -121,10 +116,6 @@ AddressValidator::Status AddressValidator::GetSuggestions(
 
 bool AddressValidator::CanonicalizeAdministrativeArea(
     AddressData* address) const {
-  DCHECK(address);
-  DCHECK(supplier_);
-  DCHECK(normalizer_);
-
   if (!supplier_->IsLoaded(address->region_code))
     return false;
 
@@ -136,7 +127,12 @@ bool AddressValidator::CanonicalizeAdministrativeArea(
   return true;
 }
 
-AddressValidator::AddressValidator() : load_rules_listener_(NULL) {}
+AddressValidator::AddressValidator()
+    : load_rules_listener_(NULL), weak_factory_(this) {}
+
+base::TimeDelta AddressValidator::GetBaseRetryPeriod() const {
+  return base::TimeDelta::FromSeconds(8);
+}
 
 void AddressValidator::Validated(bool success,
                                  const AddressData&,
@@ -145,10 +141,26 @@ void AddressValidator::Validated(bool success,
 }
 
 void AddressValidator::RulesLoaded(bool success,
-                                   const std::string& country_code,
+                                   const std::string& region_code,
                                    int) {
   if (load_rules_listener_)
-    load_rules_listener_->OnAddressValidationRulesLoaded(country_code, success);
+    load_rules_listener_->OnAddressValidationRulesLoaded(region_code, success);
+
+  // Count the first failed attempt to load rules as well.
+  if (success || attempts_number_[region_code] + 1 >= kMaxAttemptsNumber)
+    return;
+
+  base::MessageLoop::current()->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) {
+  // Do not reset retry count.
+  supplier_->LoadRules(region_code, *rules_loaded_);
 }
 
 }  // namespace autofill

third_party/libaddressinput/chromium/chrome_address_validator.h

@@ -5,12 +5,14 @@
 #ifndef THIRD_PARTY_LIBADDRESSINPUT_CHROMIUM_CHROME_ADDRESS_VALIDATOR_H_
 #define THIRD_PARTY_LIBADDRESSINPUT_CHROMIUM_CHROME_ADDRESS_VALIDATOR_H_
 
-#include <cstddef>
+#include <map>
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_field.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_validator.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/callback.h"
@@ -35,14 +37,14 @@ class LoadRulesListener {
  public:
   virtual ~LoadRulesListener() {}
 
-  // Called when the validation rules for the |country_code| have been loaded.
-  // The validation rules include the generic rules for the |country_code| and
+  // Called when the validation rules for the |region_code| have been loaded.
+  // The validation rules include the generic rules for the |region_code| and
   // specific rules for the country's administrative areas, localities, and
   // dependent localities. If a country has language-specific validation rules,
   // then these are also loaded.
   //
   // The |success| parameter is true when the rules were loaded successfully.
-  virtual void OnAddressValidationRulesLoaded(const std::string& country_code,
+  virtual void OnAddressValidationRulesLoaded(const std::string& region_code,
                                               bool success) = 0;
 };
 
@@ -138,12 +140,15 @@ class AddressValidator {
   virtual bool CanonicalizeAdministrativeArea(
       ::i18n::addressinput::AddressData* address) const;
 
- private:
-  friend class MockAddressValidator;
-
+ protected:
   // Constructor used only for MockAddressValidator.
   AddressValidator();
 
+  // Returns the period of time to wait between the first attempt's failure and
+  // the second attempt's initiation to load rules. Exposed for testing.
+  virtual base::TimeDelta GetBaseRetryPeriod() const;
+
+ private:
   // Verifies that |validator_| succeeded. Invoked by |validated_| callback.
   void Validated(bool success,
                  const ::i18n::addressinput::AddressData&,
@@ -151,7 +156,10 @@ class AddressValidator {
 
   // Invokes the |load_rules_listener_|, if it's not NULL. Called by
   // |rules_loaded_| callback.
-  void RulesLoaded(bool success, const std::string& country_code, int);
+  void RulesLoaded(bool success, const std::string& region_code, int);
+
+  // Retries loading rules without resetting the retry counter.
+  void RetryLoadRules(const std::string& region_code);
 
   // Loads and stores aggregate rules at COUNTRY level.
   const scoped_ptr< ::i18n::addressinput::PreloadSupplier> supplier_;
@@ -178,6 +186,14 @@ class AddressValidator {
   // NULL.
   LoadRulesListener* const load_rules_listener_;
 
+  // A mapping of region codes to the number of attempts to retry loading rules.
+  std::map<std::string, int> attempts_number_;
+
+  // Member variables should appear before the WeakPtrFactory, to ensure that
+  // any WeakPtrs to AddressValidator are invalidated before its members
+  // variable's destructors are executed, rendering them invalid.
+  base::WeakPtrFactory<AddressValidator> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(AddressValidator);
 };
 

third_party/libaddressinput/chromium/chrome_address_validator_unittest.cc

@@ -8,25 +8,19 @@
 #include <string>
 #include <vector>
 
-#include "base/basictypes.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_field.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_problem.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_ui.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_validator.h"
-#include "third_party/libaddressinput/src/cpp/include/libaddressinput/callback.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/downloader.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/null_storage.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/storage.h"
 #include "third_party/libaddressinput/src/cpp/test/fake_downloader.h"
 
-namespace {
+namespace autofill {
 
-using ::autofill::AddressValidator;
-using ::autofill::LoadRulesListener;
 using ::i18n::addressinput::AddressData;
 using ::i18n::addressinput::AddressField;
 using ::i18n::addressinput::AddressProblem;
@@ -84,7 +78,7 @@ class AddressValidatorTest : public testing::Test, LoadRulesListener {
   DISALLOW_COPY_AND_ASSIGN(AddressValidatorTest);
 };
 
-// Use this text fixture if you're going to use a region with a large set of
+// Use this test fixture if you're going to use a region with a large set of
 // validation rules. All rules should be loaded in SetUpTestCase().
 class LargeAddressValidatorTest : public testing::Test {
  protected:
@@ -737,4 +731,164 @@ TEST_F(AddressValidatorTest,
   EXPECT_TRUE(problems.empty());
 }
 
-}  // namespace
+// Use this test fixture for configuring the number of failed attempts to load
+// rules.
+class FailingAddressValidatorTest : public testing::Test, LoadRulesListener {
+ protected:
+  // A validator that retries loading rules without delay.
+  class TestAddressValidator : public AddressValidator {
+   public:
+    // Takes ownership of |downloader| and |storage|.
+    TestAddressValidator(
+        const std::string& validation_data_url,
+        scoped_ptr< ::i18n::addressinput::Downloader> downloader,
+        scoped_ptr< ::i18n::addressinput::Storage> storage,
+        LoadRulesListener* load_rules_listener)
+        : AddressValidator(validation_data_url,
+                           downloader.Pass(),
+                           storage.Pass(),
+                           load_rules_listener) {}
+
+    virtual ~TestAddressValidator() {}
+
+   protected:
+    virtual base::TimeDelta GetBaseRetryPeriod() const OVERRIDE {
+      return base::TimeDelta::FromSeconds(0);
+    }
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(TestAddressValidator);
+  };
+
+  // A downloader that always fails |failures_number| times before downloading
+  // data.
+  class FailingDownloader : public Downloader {
+   public:
+    explicit FailingDownloader() : failures_number_(0), attempts_number_(0) {}
+    virtual ~FailingDownloader() {}
+
+    // Sets the number of times to fail before downloading data.
+    void set_failures_number(int failures_number) {
+      failures_number_ = failures_number;
+    }
+
+    // Downloader implementation.
+    // Always fails for the first |failures_number| times.
+    virtual void Download(const std::string& url,
+                          const Callback& callback) const OVERRIDE {
+      ++attempts_number_;
+      // |callback| takes ownership of the |new std::string|.
+      if (failures_number_-- > 0)
+        callback(false, url, new std::string);
+      else
+        actual_downloader_.Download(url, callback);
+    }
+
+    // Returns the number of download attempts.
+    int attempts_number() const { return attempts_number_; }
+
+   private:
+    // The number of times to fail before downloading data.
+    mutable int failures_number_;
+
+    // The number of times Download was called.
+    mutable int attempts_number_;
+
+    // The downloader to use for successful downloads.
+    FakeDownloader actual_downloader_;
+
+    DISALLOW_COPY_AND_ASSIGN(FailingDownloader);
+  };
+
+  FailingAddressValidatorTest()
+      : downloader_(new FailingDownloader),
+        validator_(
+            new TestAddressValidator(FakeDownloader::kFakeAggregateDataUrl,
+                                     scoped_ptr<Downloader>(downloader_),
+                                     scoped_ptr<Storage>(new NullStorage),
+                                     this)),
+        load_rules_success_(false) {}
+
+  virtual ~FailingAddressValidatorTest() {}
+
+  FailingDownloader* downloader_;  // Owned by |validator_|.
+  scoped_ptr<AddressValidator> validator_;
+  bool load_rules_success_;
+
+ private:
+  // LoadRulesListener implementation.
+  virtual void OnAddressValidationRulesLoaded(const std::string&,
+                                              bool success) OVERRIDE {
+    load_rules_success_ = success;
+  }
+
+  base::MessageLoop ui_;
+
+  DISALLOW_COPY_AND_ASSIGN(FailingAddressValidatorTest);
+};
+
+// The validator will attempt to load rules at most 8 times.
+TEST_F(FailingAddressValidatorTest, RetryLoadingRulesHasLimit) {
+  downloader_->set_failures_number(99);
+  validator_->LoadRules("CH");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(load_rules_success_);
+  EXPECT_EQ(8, downloader_->attempts_number());
+}
+
+// The validator will load rules successfully if the downloader returns data
+// before the maximum number of retries.
+TEST_F(FailingAddressValidatorTest, RuleRetryingWillSucceed) {
+  downloader_->set_failures_number(4);
+  validator_->LoadRules("CH");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(load_rules_success_);
+  EXPECT_EQ(5, downloader_->attempts_number());
+}
+
+// The delayed task to retry loading rules should stop (instead of crashing) if
+// the validator is destroyed before it fires.
+TEST_F(FailingAddressValidatorTest, DestroyedValidatorStopsRetries) {
+  downloader_->set_failures_number(4);
+  validator_->LoadRules("CH");
+
+  // Destroy the validator.
+  validator_.reset();
+
+  // Fire the delayed task to retry loading rules.
+  EXPECT_NO_FATAL_FAILURE(base::RunLoop().RunUntilIdle());
+}
+
+// Each call to LoadRules should reset the number of retry attempts. If the
+// first call to LoadRules exceeded the maximum number of retries, the second
+// call to LoadRules should start counting the retries from zero.
+TEST_F(FailingAddressValidatorTest, LoadingRulesSecondTimeSucceeds) {
+  downloader_->set_failures_number(11);
+  validator_->LoadRules("CH");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(load_rules_success_);
+  EXPECT_EQ(8, downloader_->attempts_number());
+
+  validator_->LoadRules("CH");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(load_rules_success_);
+  EXPECT_EQ(12, downloader_->attempts_number());
+}
+
+// Calling LoadRules("CH") and LoadRules("GB") simultaneously should attempt to
+// load both rules up to the maximum number of attempts for each region.
+TEST_F(FailingAddressValidatorTest, RegionsShouldRetryIndividually) {
+  downloader_->set_failures_number(99);
+  validator_->LoadRules("CH");
+  validator_->LoadRules("GB");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(load_rules_success_);
+  EXPECT_EQ(16, downloader_->attempts_number());
+}
+
+}  // namespace autofill