PasswordStore: Clean up expectations about rewriting vectors of forms

components/password_manager/core/browser/login_database.cc

 // Copyright 2014 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/password_manager/core/browser/login_database.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 653 matching lines @@
       SQL_FROM_HERE,
       "DELETE FROM logins WHERE date_synced >= ? AND date_synced < ?"));
   s.BindInt64(0, delete_begin.ToInternalValue());
   s.BindInt64(1,
               delete_end.is_null() ? base::Time::Max().ToInternalValue()
                                    : delete_end.ToInternalValue());
 
   return s.Run();
 }
 
+// static
 LoginDatabase::EncryptionResult LoginDatabase::InitPasswordFormFromStatement(
     PasswordForm* form,
-    sql::Statement& s) const {
+    sql::Statement& s) {
   std::string encrypted_password;
   s.ColumnBlobAsString(COLUMN_PASSWORD_VALUE, &encrypted_password);
   base::string16 decrypted_password;
   EncryptionResult encryption_result =
       DecryptedString(encrypted_password, &decrypted_password);
   if (encryption_result != ENCRYPTION_RESULT_SUCCESS)
     return encryption_result;
 
   std::string tmp = s.ColumnString(COLUMN_ORIGIN_URL);
   form->origin = GURL(tmp);
@@ skipping 57 matching lines @@
       "username_element, username_value, "
       "password_element, password_value, submit_element, "
       "signon_realm, ssl_valid, preferred, date_created, blacklisted_by_user, "
       "scheme, password_type, possible_usernames, times_used, form_data, "
       "date_synced, display_name, avatar_url, "
       "federation_url, skip_zero_click, generation_upload_status "
       "FROM logins WHERE signon_realm == ? ";
   sql::Statement s;
   const GURL signon_realm(form.signon_realm);
   std::string registered_domain = GetRegistryControlledDomain(signon_realm);
-  PSLDomainMatchMetric psl_domain_match_metric = PSL_DOMAIN_MATCH_NONE;
   const bool should_PSL_matching_apply =
       form.scheme == PasswordForm::SCHEME_HTML &&
       ShouldPSLDomainMatchingApply(registered_domain);
   // PSL matching only applies to HTML forms.
   if (should_PSL_matching_apply) {
     // We are extending the original SQL query with one that includes more
     // possible matches based on public suffix domain matching. Using a regexp
     // here is just an optimization to not have to parse all the stored entries
     // in the |logins| table. The result (scheme, domain and port) is verified
     // further down using GURL. See the functions SchemeMatches,
@@ skipping 14 matching lines @@
     const std::string port = signon_realm.port();
     // For a signon realm such as http://foo.bar/, this regexp will match
     // domains on the form http://foo.bar/, http://www.foo.bar/,
     // http://www.mobile.foo.bar/. It will not match http://notfoo.bar/.
     // The scheme and port has to be the same as the observed form.
     std::string regexp = "^(" + scheme + ":\\/\\/)([\\w-]+\\.)*" +
                          registered_domain + "(:" + port + ")?\\/$";
     s.BindString(0, form.signon_realm);
     s.BindString(1, regexp);
   } else {
-    psl_domain_match_metric = PSL_DOMAIN_MATCH_NOT_USED;
+    UMA_HISTOGRAM_ENUMERATION("PasswordManager.PslDomainMatchTriggering",
+                              PSL_DOMAIN_MATCH_NOT_USED,
+                              PSL_DOMAIN_MATCH_COUNT);
     s.Assign(db_.GetCachedStatement(SQL_FROM_HERE, sql_query.c_str()));
     s.BindString(0, form.signon_realm);
   }
 
-  while (s.Step()) {
-    scoped_ptr<PasswordForm> new_form(new PasswordForm());
-    EncryptionResult result = InitPasswordFormFromStatement(new_form.get(), s);
-    if (result == ENCRYPTION_RESULT_SERVICE_FAILURE)
-      return false;
-    if (result == ENCRYPTION_RESULT_ITEM_FAILURE)
-      continue;
-    DCHECK(result == ENCRYPTION_RESULT_SUCCESS);
-    if (should_PSL_matching_apply) {
-      if (!IsPublicSuffixDomainMatch(new_form->signon_realm,
-                                     form.signon_realm)) {
-        // The database returned results that should not match. Skipping result.
-        continue;
-      }
-      if (form.signon_realm != new_form->signon_realm) {
-        // Ignore non-HTML matches.
-        if (new_form->scheme != PasswordForm::SCHEME_HTML)
-          continue;
-
-        psl_domain_match_metric = PSL_DOMAIN_MATCH_FOUND;
-        // This is not a perfect match, so we need to create a new valid result.
-        // We do this by copying over origin, signon realm and action from the
-        // observed form and setting the original signon realm to what we found
-        // in the database. We use the fact that |original_signon_realm| is
-        // non-empty to communicate that this match was found using public
-        // suffix matching.
-        new_form->original_signon_realm = new_form->signon_realm;
-        new_form->origin = form.origin;
-        new_form->signon_realm = form.signon_realm;
-        new_form->action = form.action;
-      }
-    }
-    forms->push_back(new_form.release());
-  }
-  UMA_HISTOGRAM_ENUMERATION("PasswordManager.PslDomainMatchTriggering",
-                            psl_domain_match_metric,
-                            PSL_DOMAIN_MATCH_COUNT);
-  return s.Succeeded();
+  return StatementToForms(&s, should_PSL_matching_apply ? &form : nullptr,
+                          forms);
 }
 
 bool LoginDatabase::GetLoginsCreatedBetween(
     const base::Time begin,
     const base::Time end,
     ScopedVector<autofill::PasswordForm>* forms) const {
   DCHECK(forms);
   sql::Statement s(db_.GetCachedStatement(
       SQL_FROM_HERE,
       "SELECT origin_url, action_url, "
       "username_element, username_value, "
       "password_element, password_value, submit_element, "
       "signon_realm, ssl_valid, preferred, date_created, blacklisted_by_user, "
       "scheme, password_type, possible_usernames, times_used, form_data, "
       "date_synced, display_name, avatar_url, "
       "federation_url, skip_zero_click, generation_upload_status FROM logins "
       "WHERE date_created >= ? AND date_created < ?"
       "ORDER BY origin_url"));
   s.BindInt64(0, begin.ToInternalValue());
   s.BindInt64(1, end.is_null() ? std::numeric_limits<int64>::max()
                                : end.ToInternalValue());
 
-  while (s.Step()) {
-    scoped_ptr<PasswordForm> new_form(new PasswordForm());
-    EncryptionResult result = InitPasswordFormFromStatement(new_form.get(), s);
-    if (result == ENCRYPTION_RESULT_SERVICE_FAILURE)
-      return false;
-    if (result == ENCRYPTION_RESULT_ITEM_FAILURE)
-      continue;
-    DCHECK(result == ENCRYPTION_RESULT_SUCCESS);
-    forms->push_back(new_form.release());
-  }
-  return s.Succeeded();
+  return StatementToForms(&s, nullptr, forms);
 }
 
 bool LoginDatabase::GetLoginsSyncedBetween(
     const base::Time begin,
     const base::Time end,
     ScopedVector<autofill::PasswordForm>* forms) const {
   DCHECK(forms);
   sql::Statement s(db_.GetCachedStatement(
       SQL_FROM_HERE,
       "SELECT origin_url, action_url, username_element, username_value, "
       "password_element, password_value, submit_element, signon_realm, "
       "ssl_valid, preferred, date_created, blacklisted_by_user, "
       "scheme, password_type, possible_usernames, times_used, form_data, "
       "date_synced, display_name, avatar_url, "
       "federation_url, skip_zero_click, generation_upload_status FROM logins "
       "WHERE date_synced >= ? AND date_synced < ?"
       "ORDER BY origin_url"));
   s.BindInt64(0, begin.ToInternalValue());
   s.BindInt64(1,
               end.is_null() ? base::Time::Max().ToInternalValue()
                             : end.ToInternalValue());
 
-  while (s.Step()) {
-    scoped_ptr<PasswordForm> new_form(new PasswordForm());
-    EncryptionResult result = InitPasswordFormFromStatement(new_form.get(), s);
-    if (result == ENCRYPTION_RESULT_SERVICE_FAILURE)
-      return false;
-    if (result == ENCRYPTION_RESULT_ITEM_FAILURE)
-      continue;
-    DCHECK(result == ENCRYPTION_RESULT_SUCCESS);
-    forms->push_back(new_form.release());
-  }
-  return s.Succeeded();
+  return StatementToForms(&s, nullptr, forms);
 }
 
 bool LoginDatabase::GetAutofillableLogins(
     ScopedVector<autofill::PasswordForm>* forms) const {
   return GetAllLoginsWithBlacklistSetting(false, forms);
 }
 
 bool LoginDatabase::GetBlacklistLogins(
     ScopedVector<autofill::PasswordForm>* forms) const {
   return GetAllLoginsWithBlacklistSetting(true, forms);
 }
 
 bool LoginDatabase::GetAllLoginsWithBlacklistSetting(
     bool blacklisted,
     ScopedVector<autofill::PasswordForm>* forms) const {
   DCHECK(forms);
   // You *must* change LoginTableColumns if this query changes.
   sql::Statement s(db_.GetCachedStatement(
       SQL_FROM_HERE,
       "SELECT origin_url, action_url, "
       "username_element, username_value, "
       "password_element, password_value, submit_element, "
       "signon_realm, ssl_valid, preferred, date_created, blacklisted_by_user, "
       "scheme, password_type, possible_usernames, times_used, form_data, "
       "date_synced, display_name, avatar_url, "
       "federation_url, skip_zero_click, generation_upload_status FROM logins "
       "WHERE blacklisted_by_user == ? ORDER BY origin_url"));
   s.BindInt(0, blacklisted ? 1 : 0);
 
-  while (s.Step()) {
-    scoped_ptr<PasswordForm> new_form(new PasswordForm());
-    EncryptionResult result = InitPasswordFormFromStatement(new_form.get(), s);
-    if (result == ENCRYPTION_RESULT_SERVICE_FAILURE)
-      return false;
-    if (result == ENCRYPTION_RESULT_ITEM_FAILURE)
-      continue;
-    DCHECK(result == ENCRYPTION_RESULT_SUCCESS);
-    forms->push_back(new_form.release());
-  }
-  return s.Succeeded();
+  return StatementToForms(&s, nullptr, forms);
 }
 
 bool LoginDatabase::DeleteAndRecreateDatabaseFile() {
   DCHECK(db_.is_open());
   meta_table_.Reset();
   db_.Close();
   sql::Connection::Delete(db_path_);
   return Init();
 }
 
+// static
+bool LoginDatabase::StatementToForms(
+    sql::Statement* statement,
+    const autofill::PasswordForm* psl_match,
+    ScopedVector<autofill::PasswordForm>* forms) {
+  PSLDomainMatchMetric psl_domain_match_metric = PSL_DOMAIN_MATCH_NONE;
+
+  // Swap |collected_forms| into |*forms| first on success, to avoid returning

[engedy, 2015/03/11 19:25:35]

nit: Do we need this anymore?

[vabr (Chromium), 2015/03/12 15:30:51]

We don't. Removed for the same reason as in KWallet.

Done.

+  // partial results on error.
+  ScopedVector<autofill::PasswordForm> collected_forms;
+
+  while (statement->Step()) {
+    scoped_ptr<PasswordForm> new_form(new PasswordForm());
+    EncryptionResult result =
+        InitPasswordFormFromStatement(new_form.get(), *statement);
+    if (result == ENCRYPTION_RESULT_SERVICE_FAILURE)
+      return false;
+    if (result == ENCRYPTION_RESULT_ITEM_FAILURE)
+      continue;
+    DCHECK(result == ENCRYPTION_RESULT_SUCCESS);
+    if (psl_match && psl_match->signon_realm != new_form->signon_realm) {
+      if (new_form->scheme != PasswordForm::SCHEME_HTML)
+        continue;  // Ignore non-HTML matches.
+
+      if (!IsPublicSuffixDomainMatch(new_form->signon_realm,
+                                     psl_match->signon_realm)) {
+        continue;
+      }
+
+      psl_domain_match_metric = PSL_DOMAIN_MATCH_FOUND;
+      // This is not a perfect match, so we need to create a new valid result.
+      // We do this by copying over origin, signon realm and action from the
+      // observed form and setting the original signon realm to what we found
+      // in the database. We use the fact that |original_signon_realm| is
+      // non-empty to communicate that this match was found using public
+      // suffix matching.
+      new_form->original_signon_realm = new_form->signon_realm;
+      new_form->origin = psl_match->origin;
+      new_form->signon_realm = psl_match->signon_realm;
+      new_form->action = psl_match->action;
+    }
+    collected_forms.push_back(new_form.Pass());
+  }
+
+  if (psl_match) {
+    UMA_HISTOGRAM_ENUMERATION("PasswordManager.PslDomainMatchTriggering",
+                              psl_domain_match_metric, PSL_DOMAIN_MATCH_COUNT);
+  }
+
+  if (!statement->Succeeded())
+    return false;
+  forms->swap(collected_forms);
+  return true;
+}
+
 }  // namespace password_manager