 Chromium Code Reviews
 Chromium Code Reviews Issue 225823006:
  Rewrite functions from WebPasswordFormData and WebPasswordFormUtils in  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 225823006:
  Rewrite functions from WebPasswordFormData and WebPasswordFormUtils in  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: components/autofill/content/renderer/password_form_conversion_utils.cc | 
| diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc | 
| index 856f6100b53213da64c2bc456c0f235053ccb8ee..5346a8d13f2223c6d103fed447ffd0fe9a5a2eb1 100644 | 
| --- a/components/autofill/content/renderer/password_form_conversion_utils.cc | 
| +++ b/components/autofill/content/renderer/password_form_conversion_utils.cc | 
| @@ -4,44 +4,36 @@ | 
| #include "components/autofill/content/renderer/password_form_conversion_utils.h" | 
| +#include "base/logging.h" | 
| 
Garrett Casto
2014/04/16 22:56:55
I think that this is unnecessary.
 | 
| #include "components/autofill/content/renderer/form_autofill_util.h" | 
| #include "components/autofill/core/common/password_form.h" | 
| +#include "third_party/WebKit/public/platform/WebString.h" | 
| +#include "third_party/WebKit/public/web/WebDocument.h" | 
| #include "third_party/WebKit/public/web/WebFormControlElement.h" | 
| -#include "third_party/WebKit/public/web/WebPasswordFormData.h" | 
| +#include "third_party/WebKit/public/web/WebInputElement.h" | 
| +using blink::WebDocument; | 
| +using blink::WebFormControlElement; | 
| using blink::WebFormElement; | 
| -using blink::WebPasswordFormData; | 
| +using blink::WebInputElement; | 
| +using blink::WebString; | 
| +using blink::WebVector; | 
| namespace autofill { | 
| namespace { | 
| +// Maximum number of password fields we will observe before throwing our | 
| +// hands in the air and giving up with a given form. | 
| +static const size_t kMaxPasswords = 3; | 
| + | 
| scoped_ptr<PasswordForm> InitPasswordFormFromWebPasswordForm( | 
| const WebFormElement& web_form, | 
| - const blink::WebPasswordFormData& web_password_form) { | 
| - PasswordForm* password_form = new PasswordForm(); | 
| - password_form->signon_realm = web_password_form.signonRealm.utf8(); | 
| - password_form->origin = web_password_form.origin; | 
| - password_form->action = web_password_form.action; | 
| - password_form->submit_element = web_password_form.submitElement; | 
| - password_form->username_element = web_password_form.userNameElement; | 
| - password_form->username_value = web_password_form.userNameValue; | 
| - password_form->other_possible_usernames.insert( | 
| - password_form->other_possible_usernames.begin(), | 
| - web_password_form.possibleUserNames.data(), | 
| - web_password_form.possibleUserNames.data() + | 
| - web_password_form.possibleUserNames.size()); | 
| - password_form->password_element = web_password_form.passwordElement; | 
| - password_form->password_value = web_password_form.passwordValue; | 
| - password_form->password_autocomplete_set = | 
| - web_password_form.passwordShouldAutocomplete; | 
| - password_form->old_password_element = web_password_form.oldPasswordElement; | 
| - password_form->old_password_value = web_password_form.oldPasswordValue; | 
| - password_form->scheme = PasswordForm::SCHEME_HTML; | 
| - password_form->ssl_valid = false; | 
| - password_form->preferred = false; | 
| - password_form->blacklisted_by_user = false; | 
| - password_form->type = PasswordForm::TYPE_MANUAL; | 
| - password_form->use_additional_authentication = false; | 
| + PasswordForm* password_form) { | 
| + GetPasswordForm(web_form, password_form); | 
| + | 
| + if (!password_form->action.is_valid()) | 
| + return scoped_ptr<PasswordForm>(); | 
| + | 
| WebFormElementToFormData(web_form, | 
| blink::WebFormControlElement(), | 
| REQUIRE_NONE, | 
| @@ -51,13 +43,160 @@ scoped_ptr<PasswordForm> InitPasswordFormFromWebPasswordForm( | 
| return scoped_ptr<PasswordForm>(password_form); | 
| } | 
| +// Helper to determine which password is the main one, and which is | 
| +// an old password (e.g on a "make new password" form), if any. | 
| +bool LocateSpecificPasswords(std::vector<WebInputElement> passwords, | 
| + WebInputElement* password, | 
| + WebInputElement* old_password) | 
| +{ | 
| 
Garrett Casto
2014/04/16 22:56:55
In Chromium, starting braces don't get their own l
 | 
| + switch (passwords.size()) { | 
| + case 1: | 
| + // Single password, easy. | 
| + *password = passwords[0]; | 
| + break; | 
| + case 2: | 
| + if (passwords[0].value() == passwords[1].value()) { | 
| + // Treat two identical passwords as a single password. | 
| + *password = passwords[0]; | 
| + } else { | 
| + // Assume first is old password, second is new (no choice but to guess). | 
| + *old_password = passwords[0]; | 
| + *password = passwords[1]; | 
| + } | 
| + break; | 
| + case 3: | 
| + if (passwords[0].value() == passwords[1].value() && | 
| + passwords[0].value() == passwords[2].value()) { | 
| + // All three passwords the same? Just treat as one and hope. | 
| + *password = passwords[0]; | 
| + } else if (passwords[0].value() == passwords[1].value()) { | 
| + // Two the same and one different -> old password is duplicated one. | 
| + *old_password = passwords[0]; | 
| + *password = passwords[2]; | 
| + } else if (passwords[1].value() == passwords[2].value()) { | 
| + *old_password = passwords[0]; | 
| + *password = passwords[1]; | 
| + } else { | 
| + // Three different passwords, or first and last match with middle | 
| + // different. No idea which is which, so no luck. | 
| + return false; | 
| + } | 
| + break; | 
| + default: | 
| + return false; | 
| + } | 
| + return true; | 
| +} | 
| + | 
| } // namespace | 
| -scoped_ptr<PasswordForm> CreatePasswordForm(const WebFormElement& webform) { | 
| - WebPasswordFormData web_password_form(webform); | 
| - if (web_password_form.isValid()) | 
| - return InitPasswordFormFromWebPasswordForm(webform, web_password_form); | 
| - return scoped_ptr<PasswordForm>(); | 
| +void GetPasswordForm(const WebFormElement& form, PasswordForm* password_form) | 
| +{ | 
| + WebInputElement latest_input_element; | 
| + std::vector<WebInputElement> passwords; | 
| + std::vector<base::string16> other_possible_usernames; | 
| + | 
| + WebVector<WebFormControlElement> control_elements; | 
| + form.getFormControlElements(control_elements); | 
| + | 
| + for (size_t i = 0; i < control_elements.size(); ++i) { | 
| + WebFormControlElement control_element = control_elements[i]; | 
| + if (control_element.isActivatedSubmit()) | 
| + password_form->submit_element = control_element.formControlName(); | 
| + | 
| + WebInputElement* input_element = toWebInputElement(&control_element); | 
| + if (!input_element) | 
| + continue; | 
| + | 
| + if (!input_element->isEnabled()) | 
| 
Garrett Casto
2014/04/16 22:56:55
Nit: Can you merge this with the previous if state
 | 
| + continue; | 
| + | 
| + if ((passwords.size() < kMaxPasswords) && | 
| + input_element->isPasswordField()) { | 
| + // We assume that the username element is the input element before the | 
| + // first password element. | 
| + if (passwords.empty() && !latest_input_element.isNull()) { | 
| + password_form->username_element = | 
| + latest_input_element.nameForAutofill(); | 
| + password_form->username_value = latest_input_element.value(); | 
| + // Remove the selected username from other_possible_usernames. | 
| + if (!other_possible_usernames.empty() && | 
| + !latest_input_element.value().isEmpty()) | 
| + other_possible_usernames.resize(other_possible_usernames.size() - 1); | 
| + } | 
| + passwords.push_back(*input_element); | 
| + } | 
| + | 
| + // Various input types such as text, url, email can be a username field. | 
| + if (input_element->isTextField() && !input_element->isPasswordField()) { | 
| + latest_input_element = *input_element; | 
| + // We ignore elements that have no value. Unlike username_element, | 
| + // other_possible_usernames is used only for autofill, not for form | 
| + // identification, and blank autofill entries are not useful. | 
| + if (!input_element->value().isEmpty()) | 
| + other_possible_usernames.push_back(input_element->value()); | 
| + } | 
| + } | 
| + | 
| + // Get the document URL | 
| + GURL full_origin(form.document().url()); | 
| + | 
| + // Calculate the canonical action URL | 
| + WebString action = form.action(); | 
| + if (action.isNull()) | 
| + action = WebString(""); // missing 'action' attribute implies current URL | 
| + GURL full_action(form.document().completeURL(action)); | 
| + if (!full_action.is_valid()) | 
| + return; | 
| + | 
| + // We want to keep the path but strip any authentication data, as well as | 
| + // query and ref portions of URL, for the form action and form origin. | 
| + GURL::Replacements rep; | 
| + rep.ClearUsername(); | 
| + rep.ClearPassword(); | 
| + rep.ClearQuery(); | 
| + rep.ClearRef(); | 
| + password_form->action = full_action.ReplaceComponents(rep); | 
| 
Garrett Casto
2014/04/16 22:56:55
You don't want to do that here. Checking to see if
 | 
| + password_form->origin = full_origin.ReplaceComponents(rep); | 
| + | 
| + rep.SetPathStr(""); | 
| + password_form->signon_realm = full_origin.ReplaceComponents(rep).spec(); | 
| + | 
| + password_form->other_possible_usernames.insert( | 
| 
Garrett Casto
2014/04/16 22:56:55
Inserting in this way was only necessary because t
 | 
| + password_form->other_possible_usernames.begin(), | 
| + other_possible_usernames.data(), | 
| + other_possible_usernames.data() + | 
| + other_possible_usernames.size()); | 
| + | 
| + WebInputElement password; | 
| + WebInputElement old_password; | 
| + if (!LocateSpecificPasswords(passwords, &password, &old_password)) | 
| + return; | 
| + | 
| + if (!password.isNull()) { | 
| + password_form->password_element = password.nameForAutofill(); | 
| + password_form->password_value = password.value(); | 
| + password_form->password_autocomplete_set = password.autoComplete(); | 
| + } | 
| + if (!old_password.isNull()) { | 
| + password_form->old_password_element = old_password.nameForAutofill(); | 
| + password_form->old_password_value = old_password.value(); | 
| + } | 
| + | 
| + password_form->scheme = PasswordForm::SCHEME_HTML; | 
| + password_form->ssl_valid = false; | 
| + password_form->preferred = false; | 
| + password_form->blacklisted_by_user = false; | 
| + password_form->type = PasswordForm::TYPE_MANUAL; | 
| + password_form->use_additional_authentication = false; | 
| +} | 
| + | 
| +scoped_ptr<PasswordForm> CreatePasswordForm(const WebFormElement& web_form) { | 
| + if (web_form.isNull()) | 
| + return scoped_ptr<PasswordForm>(); | 
| + | 
| + PasswordForm* password_form = new PasswordForm(); | 
| + return InitPasswordFormFromWebPasswordForm(web_form, password_form); | 
| } | 
| -} // namespace autofill | 
| +} // namespace autofill |