[Autofill] Credit card signin promo: do not require a local suggestion first.

components/autofill/core/browser/autofill_external_delegate.cc

 // Copyright 2013 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.
 
 #include "components/autofill/core/browser/autofill_external_delegate.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 15 matching lines @@
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace autofill {
 
 AutofillExternalDelegate::AutofillExternalDelegate(AutofillManager* manager,
                                                    AutofillDriver* driver)
     : manager_(manager),
       driver_(driver),
       query_id_(0),
-      has_suggestion_(false),
+      has_autofill_suggestions_(false),
       has_shown_popup_for_current_edit_(false),
       should_show_scan_credit_card_(false),
       should_show_cc_signin_promo_(false),
       has_shown_address_book_prompt(false),
       weak_ptr_factory_(this) {
   DCHECK(manager);
 }
 
 AutofillExternalDelegate::~AutofillExternalDelegate() {}
 
@@ skipping 13 matching lines @@
   should_show_cc_signin_promo_ =
       manager_->ShouldShowCreditCardSigninPromo(query_form_, query_field_);
 }
 
 void AutofillExternalDelegate::OnSuggestionsReturned(
     int query_id,
     const std::vector<Suggestion>& input_suggestions) {
   if (query_id != query_id_)
     return;
 
+  // The suggestions and warnings are "above the fold" and are separated from
+  // other menu items with a separator.
   std::vector<Suggestion> suggestions(input_suggestions);
-
   // Add or hide warnings as appropriate.
   ApplyAutofillWarnings(&suggestions);
 
 #if !defined(OS_ANDROID)
-  // Add a separator to go between the values and menu items.
-  suggestions.push_back(Suggestion());
-  suggestions.back().frontend_id = POPUP_ITEM_ID_SEPARATOR;
+  // If there are above the fold suggestions at this point, add a separator to
+  // go between the values and menu items.
+  if (suggestions.size()) {

[sebsg, 2016/11/01 21:31:15]

I think (!suggestions.empty()) might be more clear?

[Mathieu, 2016/11/02 21:01:03]

Done.

+    suggestions.push_back(Suggestion());
+    suggestions.back().frontend_id = POPUP_ITEM_ID_SEPARATOR;
+  }
 #endif
 
   if (should_show_scan_credit_card_) {
     Suggestion scan_credit_card(
         l10n_util::GetStringUTF16(IDS_AUTOFILL_SCAN_CREDIT_CARD));
     scan_credit_card.frontend_id = POPUP_ITEM_ID_SCAN_CREDIT_CARD;
     scan_credit_card.icon = base::ASCIIToUTF16("scanCreditCardIcon");
     suggestions.push_back(scan_credit_card);
 
     if (!has_shown_popup_for_current_edit_) {
       AutofillMetrics::LogScanCreditCardPromptMetric(
           AutofillMetrics::SCAN_CARD_ITEM_SHOWN);
     }
   }
 
   // Only include "Autofill Options" special menu item if we have Autofill
   // suggestions.
-  has_suggestion_ = false;
+  has_autofill_suggestions_ = false;
   for (size_t i = 0; i < suggestions.size(); ++i) {
     if (suggestions[i].frontend_id > 0) {
-      has_suggestion_ = true;
+      has_autofill_suggestions_ = true;
       break;
     }
   }
 
-  if (has_suggestion_)
+  if (has_autofill_suggestions_)
     ApplyAutofillOptions(&suggestions);
 
   // Append the credit card signin promo, if appropriate.
-  if (has_suggestion_ && should_show_cc_signin_promo_) {
+  if (should_show_cc_signin_promo_) {
 // No separator on Android.
 #if !defined(OS_ANDROID)
-    Suggestion separator;
-    separator.frontend_id = POPUP_ITEM_ID_SEPARATOR;
-    suggestions.push_back(separator);
+    // If there are autofill suggestions, the "Autofill options" row was added
+    // above. Add a separator between it and the signin promo.
+    if (has_autofill_suggestions_) {
+      suggestions.push_back(Suggestion());
+      suggestions.back().frontend_id = POPUP_ITEM_ID_SEPARATOR;
+    }
 #endif
 
     Suggestion signin_promo_suggestion(
         l10n_util::GetStringUTF16(IDS_AUTOFILL_CREDIT_CARD_SIGNIN_PROMO));
     signin_promo_suggestion.frontend_id =
         POPUP_ITEM_ID_CREDIT_CARD_SIGNIN_PROMO;
     suggestions.push_back(signin_promo_suggestion);
     base::RecordAction(
         base::UserMetricsAction("Signin_Impression_FromAutofillDropdown"));
   }
 
 #if !defined(OS_ANDROID)
-  // Remove the separator if it is the last element.
-  DCHECK_GT(suggestions.size(), 0U);
-  if (suggestions.back().frontend_id == POPUP_ITEM_ID_SEPARATOR)
+  // Remove the separator if there is one, and if it is the last element.
+  if (suggestions.size() &&

[sebsg, 2016/11/01 21:31:15]

ditto

[Mathieu, 2016/11/02 21:01:03]

Done.

+      suggestions.back().frontend_id == POPUP_ITEM_ID_SEPARATOR) {
     suggestions.pop_back();
+  }
 #endif
 
   // If anything else is added to modify the values after inserting the data
   // list, AutofillPopupControllerImpl::UpdateDataListValues will need to be
   // updated to match.
   InsertDataListValues(&suggestions);
 
   if (suggestions.empty()) {
     // No suggestions, any popup currently showing is obsolete.
     manager_->client()->HideAutofillPopup();
@@ skipping 14 matching lines @@
     const std::vector<base::string16>& data_list_labels) {
   data_list_values_ = data_list_values;
   data_list_labels_ = data_list_labels;
 
   manager_->client()->UpdateAutofillPopupDataListValues(data_list_values,
                                                         data_list_labels);
 }
 
 void AutofillExternalDelegate::OnPopupShown() {
   manager_->DidShowSuggestions(
-      has_suggestion_ && !has_shown_popup_for_current_edit_,
-      query_form_,
-      query_field_);
-  has_shown_popup_for_current_edit_ |= has_suggestion_;
+      has_autofill_suggestions_ && !has_shown_popup_for_current_edit_,
+      query_form_, query_field_);
+  has_shown_popup_for_current_edit_ |= has_autofill_suggestions_;
 }
 
 void AutofillExternalDelegate::OnPopupHidden() {
   driver_->PopupHidden();
 }
 
 void AutofillExternalDelegate::DidSelectSuggestion(
     const base::string16& value,
     int identifier) {
   ClearPreviewedForm();
@@ skipping 181 matching lines @@
   suggestions->insert(suggestions->begin(), data_list_values_.size(),
                       Suggestion());
   for (size_t i = 0; i < data_list_values_.size(); i++) {
     (*suggestions)[i].value = data_list_values_[i];
     (*suggestions)[i].label = data_list_labels_[i];
     (*suggestions)[i].frontend_id = POPUP_ITEM_ID_DATALIST_ENTRY;
   }
 }
 
 }  // namespace autofill