Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(351)

Unified Diff: components/password_manager/core/browser/password_form_manager.cc

Issue 920683002: [Password Generation] Add negative votes for crowdsourcing (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Test fixes Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/password_manager/core/browser/password_form_manager.cc
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index 0757426e74078f313cb4e69df484ce868c4bd41f..b642296ddba651cc8b1c420f5699f3d818ff9310 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -610,7 +610,7 @@ void PasswordFormManager::SaveAsNewLogin(bool reset_preferred_login) {
if (pending_credentials_.times_used == 0) {
UploadPasswordForm(pending_credentials_.form_data, autofill::PASSWORD);
} else {
- CheckForAccountCreationForm(pending_credentials_, observed_form_);
+ CheckForAccountCreationForm(observed_form_, &pending_credentials_);
}
}
@@ -681,7 +681,7 @@ void PasswordFormManager::UpdateLogin() {
}
// Check to see if this form is a candidate for password generation.
- CheckForAccountCreationForm(pending_credentials_, observed_form_);
+ CheckForAccountCreationForm(observed_form_, &pending_credentials_);
UpdatePreferredLoginState(password_store);
@@ -761,42 +761,56 @@ bool PasswordFormManager::UpdatePendingCredentialsIfOtherPossibleUsername(
}
void PasswordFormManager::CheckForAccountCreationForm(
- const PasswordForm& pending,
- const PasswordForm& observed) {
- // We check to see if the saved form_data is the same as the observed
- // form_data, which should never be true for passwords saved on account
- // creation forms. This check is only made the first time a password is used
- // to cut down on false positives. Specifically a site may have multiple login
- // forms with different markup, which might look similar to a signup form.
- if (pending.times_used == 1) {
- FormStructure pending_structure(pending.form_data);
- FormStructure observed_structure(observed.form_data);
- // Ignore |pending_structure| if its FormData has no fields. This is to
- // weed out those credentials that were saved before FormData was added
- // to PasswordForm. Even without this check, these FormStructure's won't
- // be uploaded, but it makes it hard to see if we are encountering
- // unexpected errors.
- if (!pending.form_data.fields.empty() &&
- pending_structure.FormSignature() !=
- observed_structure.FormSignature()) {
- UploadPasswordForm(pending.form_data,
- autofill::ACCOUNT_CREATION_PASSWORD);
+ const PasswordForm& observed,
+ PasswordForm* pending) {
+ if (pending->form_data.fields.empty())
+ return;
+
+ FormStructure pending_structure(pending->form_data);
+ FormStructure observed_structure(observed.form_data);
+
+ // Ignore |pending_structure| if its FormData has no fields. This is to
+ // weed out those credentials that were saved before FormData was added
+ // to PasswordForm. Even without this check, these FormStructure's won't
+ // be uploaded, but it makes it hard to see if we are encountering
+ // unexpected errors.
+ if (pending_structure.FormSignature() != observed_structure.FormSignature()) {
+ // Only upload if this is the first time the password has been used.
+ // Otherwise the credentials have been used on the same field before so
+ // they aren't from an account creation form.
+ if (pending->times_used == 1) {
+ if (UploadPasswordForm(pending->form_data,
+ autofill::ACCOUNT_CREATION_PASSWORD)) {
+ pending->generation_upload_status =
+ autofill::PasswordForm::POSITIVE_SIGNAL_SENT;
+ }
+ }
+ } else if (pending->generation_upload_status ==
+ autofill::PasswordForm::POSITIVE_SIGNAL_SENT) {
+ // A signal was sent that this was an account creation form, but the
+ // credential is now being used on the same form again. This cancels out
+ // the previous vote.
+ if (UploadPasswordForm(pending->form_data,
+ autofill::NOT_ACCOUNT_CREATION_PASSWORD)) {
+ pending->generation_upload_status =
+ autofill::PasswordForm::NEGATIVE_SIGNAL_SENT;
}
}
}
-void PasswordFormManager::UploadPasswordForm(
+bool PasswordFormManager::UploadPasswordForm(
const autofill::FormData& form_data,
const autofill::ServerFieldType& password_type) {
autofill::AutofillManager* autofill_manager =
client_->GetAutofillManagerForMainFrame();
if (!autofill_manager)
- return;
+ return false;
// Note that this doesn't guarantee that the upload succeeded, only that
// |form_data| is considered uploadable.
bool success = autofill_manager->UploadPasswordForm(form_data, password_type);
UMA_HISTOGRAM_BOOLEAN("PasswordGeneration.UploadStarted", success);
+ return success;
}
int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {

Powered by Google App Engine
This is Rietveld 408576698