[Paradise] Fix local card decision metric for existing bin range.

If the user goes through checkout flow several times without closing the tab,
the local card migration manager won't be re-created which leads to the case
that the variable supported_card_bin_ranges pre-exist before GDFSC calls.

We do not filter the unsupported cards during GetMigratableCreditCards(). We
only call FilterOutUnsupportedLocalCards() when OnDidGetUploadDetails() for
checkout flow. We remove the local var of the supported bin ranges. If later
any usage of FilterOutUnsupportedLocalCards() outside OnDidGetUploadDetails,
we need to add back the local var.
Add some comments about these two functions to make it clearer.

Bug: 954367
Change-Id: I7fc7cbdecf76d25b34737ef6ddc25737f7540657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614456Reviewed-by: Siyu An <siyua@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Commit-Queue: Sujie Zhu <sujiezhu@google.com>
Cr-Commit-Position: refs/heads/master@{#661058}

components/autofill/core/browser/payments/local_card_migration_manager.cc

@@ -226,7 +226,6 @@ void LocalCardMigrationManager::OnDidGetUploadDetails(
     migration_request_.context_token = context_token;
     legal_message_ = base::DictionaryValue::From(std::move(legal_message));
     migration_request_.risk_data.clear();
-    supported_card_bin_ranges_ = supported_card_bin_ranges;
     // If we successfully received the legal docs, trigger the offer-to-migrate
     // dialog. If triggered from settings page, we pop-up the main prompt
     // directly. If not, we pop up the intermediate bubble.
@@ -237,8 +236,8 @@ void LocalCardMigrationManager::OnDidGetUploadDetails(
       // Pops up a larger, modal dialog showing the local cards to be uploaded.
       ShowMainMigrationDialog();
     } else {
-      // Filter the migratable credit cards with |supported_card_bin_ranges_|.
-      FilterOutUnsupportedLocalCards();
+      // Filter the migratable credit cards with |supported_card_bin_ranges|.
+      FilterOutUnsupportedLocalCards(supported_card_bin_ranges);
       // Abandon the migration if no supported card left.
       if (migratable_credit_cards_.empty()) {
         AutofillMetrics::LogLocalCardMigrationDecisionMetric(
@@ -416,23 +415,19 @@ void LocalCardMigrationManager::GetMigratableCreditCards() {
       migratable_credit_cards_.push_back(MigratableCreditCard(*credit_card));
     }
   }
-
-  // Filter out Unsupported local cards when |supported_card_bin_ranges_| is not
-  // empty.
-  FilterOutUnsupportedLocalCards();
 }
 
-void LocalCardMigrationManager::FilterOutUnsupportedLocalCards() {
+void LocalCardMigrationManager::FilterOutUnsupportedLocalCards(
+    const std::vector<std::pair<int, int>>& supported_card_bin_ranges) {
   if (base::FeatureList::IsEnabled(
           features::kAutofillDoNotMigrateUnsupportedLocalCards) &&
-      !supported_card_bin_ranges_.empty()) {
+      !supported_card_bin_ranges.empty()) {
     // Update the |migratable_credit_cards_| with the
     // |supported_card_bin_ranges|. This will remove any card from
     // |migratable_credit_cards_| of which the card number is not in
     // |supported_card_bin_ranges|.
     auto card_is_unsupported =
-        [& supported_card_bin_ranges =
-             supported_card_bin_ranges_](MigratableCreditCard& card) {
+        [&supported_card_bin_ranges](MigratableCreditCard& card) {
           return !payments::IsCreditCardSupported(card.credit_card(),
                                                   supported_card_bin_ranges);
         };

components/autofill/core/browser/payments/local_card_migration_manager.h

@@ -124,6 +124,13 @@ class LocalCardMigrationManager {
   int GetDetectedValues() const;
 
   // Fetch all migratable credit cards and store in |migratable_credit_cards_|.
+  // Migratable cards are cards whose card number passed luhn check and
+  // expiration date are valid. We do NOT filter unsupported cards here.
+  // Any other usage of this function other than ShouldOfferLocalCardMigration()
+  // and from settings page after OnDidGetUploadDetails, you should call
+  // FilterOutUnsupportedLocalCards right after this function to filter out
+  // unsupported cards. If so, the first OnDidGetUploadDetails() will need to
+  // store the supported ranges locally.
   void GetMigratableCreditCards();
 
   // For testing.
@@ -177,9 +184,12 @@ class LocalCardMigrationManager {
   // Returns the LocalCardMigrationStrikeDatabase for |client_|.
   LocalCardMigrationStrikeDatabase* GetLocalCardMigrationStrikeDatabase();
 
-  // Filter the |migratable_credit_cards_| with |supported_card_bin_ranges_| and
+  // Filter the |migratable_credit_cards_| with |supported_card_bin_ranges| and
   // keep supported local cards in |migratable_credit_cards_|.
-  void FilterOutUnsupportedLocalCards();
+  // Effective after one successful GetUploadDetails call where we fetch the
+  // |supported_card_bin_ranges|.
+  void FilterOutUnsupportedLocalCards(
+      const std::vector<std::pair<int, int>>& supported_card_bin_ranges);
 
   // Pops up a larger, modal dialog showing the local cards to be uploaded.
   void ShowMainMigrationDialog();
@@ -229,10 +239,6 @@ class LocalCardMigrationManager {
   std::unique_ptr<LocalCardMigrationStrikeDatabase>
       local_card_migration_strike_database_;
 
-  // List of BIN prefix ranges which are supoorted, with the first and second
-  // number in the pair being the start and end of the range.
-  std::vector<std::pair<int, int>> supported_card_bin_ranges_;
-
   base::WeakPtrFactory<LocalCardMigrationManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationManager);