[Password Generation] Always query password forms via Autofill

components/autofill/core/browser/form_structure_unittest.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/form_structure.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
@@ skipping 464 matching lines @@
   form_structure->DetermineHeuristicTypes(TestAutofillMetrics());
   EXPECT_FALSE(form_structure->IsAutofillable());
   EXPECT_FALSE(form_structure->ShouldBeCrowdsourced());
 
   ASSERT_EQ(3U, form_structure->field_count());
   ASSERT_EQ(0U, form_structure->autofill_count());
 
   EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(0)->heuristic_type());
   EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(1)->heuristic_type());
   EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(2)->heuristic_type());
+
+  // Even with an 'autocomplete' attribute set, ShouldBeCrowdsourced() should
+  // return true if the structure contains a password field, since there are
+  // no local heuristics to depend upon in this case. Fields will still not be
+  // considered autofillable though.

[Ilya Sherman, 2014/10/24 01:24:45]

Please create a separate TEST() for this case.

[Garrett Casto, 2014/10/24 21:13:10]

Done.

+  field.label = ASCIIToUTF16("Password");
+  field.name = ASCIIToUTF16("Password");
+  field.form_control_type = "password";
+  form.fields.push_back(field);
+
+  form_structure.reset(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes(TestAutofillMetrics());
+  EXPECT_FALSE(form_structure->IsAutofillable());
+  EXPECT_TRUE(form_structure->ShouldBeCrowdsourced());
+
+  ASSERT_EQ(4U, form_structure->field_count());
+  ASSERT_EQ(0U, form_structure->autofill_count());
+
+  EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(0)->heuristic_type());
+  EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(1)->heuristic_type());
+  EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(2)->heuristic_type());
+  EXPECT_EQ(UNKNOWN_TYPE, form_structure->field(3)->heuristic_type());

[Ilya Sherman, 2014/10/24 01:24:45]

I'm not convinced that most of these expectations are needed.  I think you can
simplify the test to:

// Create test form.
...

FormStructure form_structure(form);
form_structure.DetermineHeuristicTypes(TestAutofillMetrics());
EXPECT_TRUE(form_structure.ShouldBeCrowdsourced());

[Garrett Casto, 2014/10/24 21:13:10]

Do we not want to check that IsAutofillable() returns false?

[Ilya Sherman, 2014/10/24 22:42:30]

It's not obvious to me why that's relevant to this test, but it also doesn't
hurt to add that one check.  The main advantage of dropping it is that it
emphasizes the single check that the test is really focused on.

 }
 
 // Verify that we can correctly process sections listed in the |autocomplete|
 // attribute.
 TEST(FormStructureTest, HeuristicsAutocompleteAttributeWithSections) {
   FormData form;
 
   FormFieldData field;
   field.form_control_type = "text";
 
@@ skipping 1846 matching lines @@
   // A freeform input (<input>) allows any value (overriding other <select>s).
   FormFieldData freeform_field;
   freeform_field.autocomplete_attribute = "billing country";
   form_data.fields.push_back(freeform_field);
   FormStructure form_structure2(form_data);
   form_structure2.ParseFieldTypesFromAutocompleteAttributes(&unused, &unused);
   EXPECT_EQ(0U, form_structure2.PossibleValues(ADDRESS_BILLING_COUNTRY).size());
 }
 
 }  // namespace autofill