[Autofill] Always show available data when encountering autocomplete attributes

components/autofill/core/browser/autofill_manager.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_manager.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <map>
@@ skipping 26 matching lines @@
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
 #include "components/autofill/core/browser/popup_item_ids.h"
+#include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_data_validation.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/form_data_predictions.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 322 matching lines @@
 
   external_delegate_->OnQuery(query_id, form, field, bounding_box);
 
   // Need to refresh models before using the form_event_loggers.
   bool is_autofill_possible = RefreshDataModels();
 
   FormStructure* form_structure = NULL;
   AutofillField* autofill_field = NULL;
   bool got_autofillable_form =
       GetCachedFormAndField(form, field, &form_structure, &autofill_field) &&
-      // Don't send suggestions or track forms that aren't auto-fillable.
-      form_structure->IsAutofillable();
+      // Don't send suggestions or track forms that should not be parsed.
+      form_structure->ShouldBeParsed();
 
   // Logging interactions of forms that are autofillable.
   if (got_autofillable_form) {
     if (autofill_field->Type().group() == CREDIT_CARD)
       credit_card_form_event_logger_->OnDidInteractWithAutofillableForm();
     else
       address_form_event_logger_->OnDidInteractWithAutofillableForm();
   }
 
   if (is_autofill_possible &&
@@ skipping 402 matching lines @@
 }
 
 bool AutofillManager::ShouldUploadForm(const FormStructure& form) {
   if (!IsAutofillEnabled())
     return false;
 
   if (driver_->IsOffTheRecord())
     return false;
 
   // Disregard forms that we wouldn't ever autofill in the first place.
-  if (!form.ShouldBeParsed())
+  // ShouldBeParsed can return true for small forms in certain situations so a
+  // specific check for the number of active fields is necessary to avoid
+  // uploading very small forms.
+  if (!form.ShouldBeParsed() ||
+      form.active_field_count() < kRequiredFieldsForPredictionRoutines) {

[Evan Stade, 2015/11/17 19:13:36]

nit: this function now looks like

if (a)
  return false;

if (b)
  return false;

if (c || d)
  return false;

which is oddly inconsistent. Why not

return IsAutofillEnabled() && !driver_->IsOffTheRecord() &&
form.ShouldBeParsed() && form.active_field_count() >=
kRequiredFieldsForPredictionRoutines;

Also I don't get why you would want to autofill a small form with autocomplete
attributes, but wouldn't want to upload that form. But if that does make sense,
your reuse of kRequiredFieldsForPredictionRoutines is the opposite of what you'd
expect because you're only using it to filter out some small non-prediction
forms. You should create a different constant called kRequiredFieldsForUpload or
something (which might just happen to have the same value as
kRequiredFieldForPredictionRoutines, or might not).

[sebsg, 2015/11/18 16:40:27]

Done.

     return false;
+  }
 
   return true;
 }
 
 void AutofillManager::ImportFormData(const FormStructure& submitted_form) {
   scoped_ptr<CreditCard> imported_credit_card;
   if (!personal_data_->ImportFormData(submitted_form, &imported_credit_card))
     return;
 
 #ifdef ENABLE_FORM_DEBUG_DUMP
@@ skipping 528 matching lines @@
 
 void AutofillManager::ParseForms(const std::vector<FormData>& forms) {
   if (forms.empty())
     return;
 
   std::vector<FormStructure*> non_queryable_forms;
   std::vector<FormStructure*> queryable_forms;
   for (const FormData& form : forms) {
     scoped_ptr<FormStructure> form_structure(new FormStructure(form));
 
+    form_structure->ParseFieldTypesFromAutocompleteAttributes();
+
     if (!form_structure->ShouldBeParsed()) {
       if (form_structure->has_password_field()) {
         AutofillMetrics::LogPasswordFormQueryVolume(
             AutofillMetrics::NEW_PASSWORD_QUERY);
       }
       continue;
     }
 
     form_structure->DetermineHeuristicTypes();
 
@@ skipping 134 matching lines @@
     if (i > 0)
       fputs("Next oldest form:\n", file);
   }
   fputs("\n", file);
 
   fclose(file);
 }
 #endif  // ENABLE_FORM_DEBUG_DUMP
 
 }  // namespace autofill