Update PasswordGenerator to only use one non-alphanumeric character.

Most sites only require one such character, and many sites don't allow such
characters. This should be more likely to generate a valid password and it will
also have a smaller edit distance to a valid password if the user needs to
change it.

BUG=165318


Review URL: https://chromiumcodereview.appspot.com/11511014

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

3 files changed, 21 insertions, 6 deletions

diff --git a/chrome/browser/autofill/password_generator.cc b/chrome/browser/autofill/password_generator.cc
index 9df082c..9b40f68 100644
--- a/chrome/browser/autofill/password_generator.cc
+++ b/chrome/browser/autofill/password_generator.cc
@@ -11,8 +11,6 @@
 #include "base/logging.h"
 #include "base/rand_util.h"
 
-const int kMinChar = 33;   // First printable character '!'
-const int kMaxChar = 126;  // Last printable character '~'
 const int kMinUpper = 65;  // First upper case letter 'A'
 const int kMaxUpper = 90;  // Last upper case letter 'Z'
 const int kMinLower = 97;  // First lower case letter 'a'
@@ -40,6 +38,15 @@ size_t GetLengthFromHint(size_t max_length, size_t default_length) {
     return default_length;
 }
 
+void InitializeAlphaNumericCharacters(std::vector<char>* characters) {
+  for (int i = kMinDigit; i <= kMaxDigit; ++i)
+    characters->push_back(static_cast<char>(i));
+  for (int i = kMinUpper; i <= kMaxUpper; ++i)
+    characters->push_back(static_cast<char>(i));
+  for (int i = kMinLower; i <= kMaxLower; ++i)
+    characters->push_back(static_cast<char>(i));
+}
+
 // Classic algorithm to randomly select |num_select| elements out of
 // |num_total| elements. One description can be found at:
 // "http://stackoverflow.com/questions/48087/select-a-random-n-elements-from-listt-in-c-sharp/48089#48089"
@@ -74,6 +81,10 @@ PasswordGenerator::~PasswordGenerator() {}
 
 std::string PasswordGenerator::Generate() const {
   std::string ret;
+  CR_DEFINE_STATIC_LOCAL(std::vector<char>, alphanumeric_characters, ());
+  if (alphanumeric_characters.empty())
+    InitializeAlphaNumericCharacters(&alphanumeric_characters);
+
   // First, randomly select 4 positions to hold one upper case letter,
   // one lower case letter, one digit, and one other symbol respectively,
   // to make sure at least one of each category of characters will be
@@ -101,8 +112,11 @@ std::string PasswordGenerator::Generate() const {
       ret.push_back(
           kOtherSymbols[base::RandInt(0, arraysize(kOtherSymbols) - 1)]);
     } else {
-      // Generate random character from all categories.
-      ret.push_back(static_cast<char>(base::RandInt(kMinChar, kMaxChar)));
+      // Generate random alphanumeric character. We don't use other symbols
+      // here as most sites don't allow a lot of non-alphanumeric characters.
+      ret.push_back(
+          alphanumeric_characters.at(
+              base::RandInt(0, alphanumeric_characters.size() - 1)));
     }
   }
   return ret;
diff --git a/chrome/browser/autofill/password_generator.h b/chrome/browser/autofill/password_generator.h
index dd33160..283fdf5 100644
--- a/chrome/browser/autofill/password_generator.h
+++ b/chrome/browser/autofill/password_generator.h
@@ -26,9 +26,10 @@ class PasswordGenerator {
   // (1) Each character is guaranteed to be a non-whitespace printable ASCII
   //     character.
   // (2) The generated password will contain AT LEAST one upper case letter, one
-  //     lower case letter, one digit, and one other symbol.
+  //     lower case letter, one digit, and EXACTLY one other symbol.
   // (3) The password length will be equal to |password_length_| (see comment
   //     for the constructor).
+  // Not thread safe.
   std::string Generate() const;
 
  private:
diff --git a/chrome/browser/autofill/password_generator_unittest.cc b/chrome/browser/autofill/password_generator_unittest.cc
index 1602a10..ca04b8b 100644
--- a/chrome/browser/autofill/password_generator_unittest.cc
+++ b/chrome/browser/autofill/password_generator_unittest.cc
@@ -43,7 +43,7 @@ TEST(PasswordGeneratorTest, PasswordPattern) {
   EXPECT_GT(num_upper_case_letters, 0);
   EXPECT_GT(num_lower_case_letters, 0);
   EXPECT_GT(num_digits, 0);
-  EXPECT_GT(num_other_symbols, 0);
+  EXPECT_EQ(num_other_symbols, 1);
 }
 
 TEST(PasswordGeneratorTest, Printable) {