Index: components/autofill/content/renderer/password_autofill_agent.cc |
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc |
index f2882c9ff03c8f317012bc560fe544ab5dbe9f69..ac534c0f7d6dc04fb9f98e75bf2bf6793b8b2eae 100644 |
--- a/components/autofill/content/renderer/password_autofill_agent.cc |
+++ b/components/autofill/content/renderer/password_autofill_agent.cc |
@@ -5,6 +5,7 @@ |
#include "components/autofill/content/renderer/password_autofill_agent.h" |
#include "base/bind.h" |
+#include "base/command_line.h" |
#include "base/memory/scoped_ptr.h" |
#include "base/message_loop/message_loop.h" |
#include "base/metrics/histogram.h" |
@@ -13,6 +14,7 @@ |
#include "components/autofill/content/renderer/form_autofill_util.h" |
#include "components/autofill/content/renderer/password_form_conversion_utils.h" |
#include "components/autofill/content/renderer/renderer_save_password_progress_logger.h" |
+#include "components/autofill/core/common/autofill_switches.h" |
#include "components/autofill/core/common/form_field_data.h" |
#include "components/autofill/core/common/password_autofill_util.h" |
#include "components/autofill/core/common/password_form.h" |
@@ -39,6 +41,12 @@ |
namespace autofill { |
namespace { |
+enum FillUserNameAndPasswordOptions { |
+ EXACT_USERNAME_MATCH = 1 << 0, |
+ SET_SELECTION = 1 << 1, |
+ FILL_PREFERRED_USERNAME = 1 << 2 |
+}; |
+ |
// The size above which we stop triggering autocomplete. |
static const size_t kMaximumTextSizeForAutocomplete = 1000; |
@@ -62,6 +70,23 @@ struct FormElements { |
typedef std::vector<FormElements*> FormElementsList; |
+bool ShouldFillOnAccountSelect() { |
+ if (CommandLine::ForCurrentProcess()->HasSwitch( |
+ switches::kEnableFillOnAccountSelect)) { |
+ return true; |
+ } |
+ |
+ // This is made explicit in anticiption of experimental groups being added. |
+ if (CommandLine::ForCurrentProcess()->HasSwitch( |
+ switches::kDisableFillOnAccountSelect)) { |
+ return false; |
+ } |
+ |
+ // TODO(jww): Add experimental groups check here. |
+ |
+ return false; |
+} |
+ |
// Helper to search the given form element for the specified input elements |
// in |data|, and add results to |result|. |
static bool FindFormInputElements(blink::WebFormElement* fe, |
@@ -281,14 +306,16 @@ bool GetSuggestionsStats(const PasswordFormFillData& fill_data, |
// with values from |fill_data|. The |password_element| will only have the |
// |suggestedValue| set, and will be registered for copying that to the real |
// value through |registration_callback|. The function returns true when |
-// selected username comes from |fill_data.other_possible_usernames|. |
+// selected username comes from |fill_data.other_possible_usernames|. |options| |
+// should be a bitwise mask of FillUserNameAndPasswordOptions values. |
bool FillUserNameAndPassword( |
blink::WebInputElement* username_element, |
blink::WebInputElement* password_element, |
const PasswordFormFillData& fill_data, |
- bool exact_username_match, |
- bool set_selection, |
+ const int options, |
base::Callback<void(blink::WebInputElement*)> registration_callback) { |
+ bool exact_username_match = (options & EXACT_USERNAME_MATCH) != 0; |
+ bool set_selection = (options & SET_SELECTION) != 0; |
bool other_possible_username_selected = false; |
// Don't fill username if password can't be set. |
if (!IsElementAutocompletable(*password_element)) |
@@ -383,7 +410,7 @@ bool FillUserNameAndPassword( |
// |suggestedValue| set, and |suggestedValue| will be registered for copying to |
// the real value through |registration_callback|. Returns true when the |
// username gets selected from |other_possible_usernames|, else returns false. |
-bool FillFormOnPasswordRecieved( |
+bool FillFormOnPasswordReceived( |
const PasswordFormFillData& fill_data, |
blink::WebInputElement username_element, |
blink::WebInputElement password_element, |
@@ -402,23 +429,36 @@ bool FillFormOnPasswordRecieved( |
if (!IsElementAutocompletable(password_element)) |
return false; |
- // Try to set the username to the preferred name, but only if the field |
- // can be set and isn't prefilled. |
- if (form_contains_username_field && |
- IsElementAutocompletable(username_element) && |
- username_element.value().isEmpty()) { |
- // TODO(tkent): Check maxlength and pattern. |
- username_element.setValue(fill_data.basic_data.fields[0].value, true); |
+ // Try to set the username to the preferred name, but only if: |
+ // (a) The username element is autocompletable, and |
+ // (b) The fill-on-account-select flag is not set, and |
+ // (b) The username element isn't prefilled |
Garrett Casto
2014/11/04 23:35:30
Is that last line supposed to be (c)?
jww
2014/11/06 21:28:56
Yup :-)
|
+ // |
+ // If (a) is true, but (b) is false, then just mark the username element as |
+ // autofilled and return so the fill step is skipped. |
+ // |
+ // If (a) is false, but (b) is true, then the username element should not be |
+ // autofilled, but the user should still be able to select to fill the |
+ // password element, so the password element must be marked as autofilled and |
+ // the fill step should also be skipped. |
+ if (IsElementAutocompletable(username_element)) { |
Garrett Casto
2014/11/04 23:35:30
I think that you dropped the form_contains_usernam
jww
2014/11/06 21:28:56
Done.
Garrett Casto
2014/11/23 07:49:48
Can you add a test so that this would have been ca
jww
2014/11/25 02:46:25
Yup, unit tests are my next step.
|
+ if (ShouldFillOnAccountSelect()) { |
+ username_element.setAutofilled(true); |
+ return false; |
+ } else if (username_element.value().isEmpty()) { |
+ // TODO(tkent): Check maxlength and pattern. |
+ username_element.setValue(fill_data.basic_data.fields[0].value, true); |
+ } |
+ } else if (ShouldFillOnAccountSelect()) { |
+ password_element.setAutofilled(true); |
+ return false; |
} |
// Fill if we have an exact match for the username. Note that this sets |
// username to autofilled. |
- return FillUserNameAndPassword(&username_element, |
- &password_element, |
- fill_data, |
- true /* exact_username_match */, |
- false /* set_selection */, |
- registration_callback); |
+ return FillUserNameAndPassword( |
+ &username_element, &password_element, fill_data, |
+ EXACT_USERNAME_MATCH | FILL_PREFERRED_USERNAME, registration_callback); |
} |
// Takes a |map| with pointers as keys and linked_ptr as values, and returns |
@@ -520,11 +560,7 @@ bool PasswordAutofillAgent::TextFieldDidEndEditing( |
// Do not set selection when ending an editing session, otherwise it can |
// mess with focus. |
if (FillUserNameAndPassword( |
- &username, |
- &password, |
- fill_data, |
- true /* exact_username_match */, |
- false /* set_selection */, |
+ &username, &password, fill_data, EXACT_USERNAME_MATCH, |
base::Bind(&PasswordValueGatekeeper::RegisterElement, |
base::Unretained(&gatekeeper_)))) { |
usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED; |
@@ -684,11 +720,28 @@ bool PasswordAutofillAgent::DidClearAutofillSelection( |
return true; |
} |
+PasswordAutofillAgent::LoginToPasswordInfoMap::iterator |
+PasswordAutofillAgent::FindPasswordInfoForElement( |
+ const blink::WebInputElement& element) { |
+ const blink::WebInputElement* username_element; |
Garrett Casto
2014/11/04 23:35:30
Why does this need to be a pointer?
jww
2014/11/06 21:28:56
I was trying to have only return statement, and si
|
+ if (!element.isPasswordField()) { |
+ username_element = &element; |
+ } else { |
+ PasswordToLoginMap::const_iterator password_iter = |
+ password_to_username_.find(element); |
Garrett Casto
2014/11/04 23:35:30
Where is this defined and populated?
jww
2014/11/06 21:28:57
It's defined in password_autofill_agent.h and it i
|
+ if (password_iter == password_to_username_.end()) |
+ return login_to_password_info_.end(); |
+ username_element = &password_iter->second; |
+ } |
+ |
+ return login_to_password_info_.find(*username_element); |
+} |
+ |
bool PasswordAutofillAgent::ShowSuggestions( |
const blink::WebInputElement& element, |
bool show_all) { |
LoginToPasswordInfoMap::const_iterator iter = |
- login_to_password_info_.find(element); |
+ FindPasswordInfoForElement(element); |
if (iter == login_to_password_info_.end()) |
return false; |
@@ -700,7 +753,13 @@ bool PasswordAutofillAgent::ShowSuggestions( |
!IsElementAutocompletable(iter->second.password_field)) |
return true; |
- return ShowSuggestionPopup(iter->second.fill_data, element, show_all); |
+ // Chrome should never show more than one account for a password element since |
+ // this implies that the username element cannot be modified. Thus even if |
+ // |show_all| is true, check if the element in question is a password element |
+ // for the call to ShowSuggestionPopup. |
+ return ShowSuggestionPopup(iter->second.fill_data, iter->first, |
+ show_all && !element.isPasswordField(), |
+ element.isPasswordField()); |
Garrett Casto
2014/11/04 23:35:30
Is there any reason why you need the fourth parame
jww
2014/11/06 21:28:57
Yes, because we ultimately need the username as th
Garrett Casto
2014/11/23 07:49:48
I'm not sure that I understand why we need the use
jww
2014/11/25 02:46:25
I believe it's because the browser process can onl
|
} |
bool PasswordAutofillAgent::OriginCanAccessPasswordManager( |
@@ -1049,10 +1108,8 @@ void PasswordAutofillAgent::OnFillPasswordForm( |
// If wait_for_username is true, we don't want to initially fill the form |
// until the user types in a valid username. |
if (!form_data.wait_for_username && |
- FillFormOnPasswordRecieved( |
- form_data, |
- username_element, |
- password_element, |
+ FillFormOnPasswordReceived( |
+ form_data, username_element, password_element, |
base::Bind(&PasswordValueGatekeeper::RegisterElement, |
base::Unretained(&gatekeeper_)))) { |
usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED; |
@@ -1095,7 +1152,8 @@ PasswordAutofillAgent::PasswordInfo::PasswordInfo() |
bool PasswordAutofillAgent::ShowSuggestionPopup( |
const PasswordFormFillData& fill_data, |
const blink::WebInputElement& user_input, |
- bool show_all) { |
+ bool show_all, |
+ bool show_on_password_field) { |
blink::WebFrame* frame = user_input.document().frame(); |
if (!frame) |
return false; |
@@ -1110,6 +1168,12 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( |
user_input, &form, &field, REQUIRE_NONE); |
blink::WebInputElement selected_element = user_input; |
+ if (show_on_password_field) { |
+ LoginToPasswordInfoMap::const_iterator iter = |
+ login_to_password_info_.find(user_input); |
+ DCHECK(iter != login_to_password_info_.end()); |
+ selected_element = iter->second.password_field; |
+ } |
gfx::Rect bounding_box(selected_element.boundsInViewportSpace()); |
float scale = web_view_->pageScaleFactor(); |
@@ -1117,8 +1181,15 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( |
bounding_box.y() * scale, |
bounding_box.width() * scale, |
bounding_box.height() * scale); |
- Send(new AutofillHostMsg_ShowPasswordSuggestions( |
- routing_id(), field, user_input.value(), show_all, bounding_box_scaled)); |
+ if (show_on_password_field) { |
+ Send(new AutofillHostMsg_ShowPasswordSuggestionsForPasswordField( |
+ routing_id(), field, user_input.value(), show_all, |
+ bounding_box_scaled)); |
+ } else { |
+ Send(new AutofillHostMsg_ShowPasswordSuggestions( |
+ routing_id(), field, user_input.value(), show_all, |
+ bounding_box_scaled)); |
+ } |
bool suggestions_present = false; |
if (GetSuggestionsStats(fill_data, user_input.value(), show_all, |
@@ -1152,11 +1223,7 @@ void PasswordAutofillAgent::PerformInlineAutocomplete( |
// Fill the user and password field with the most relevant match. Android |
// only fills in the fields after the user clicks on the suggestion popup. |
if (FillUserNameAndPassword( |
- &username, |
- &password, |
- fill_data, |
- false /* exact_username_match */, |
- true /* set_selection */, |
+ &username, &password, fill_data, SET_SELECTION, |
base::Bind(&PasswordValueGatekeeper::RegisterElement, |
base::Unretained(&gatekeeper_)))) { |
usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED; |
@@ -1197,7 +1264,7 @@ bool PasswordAutofillAgent::FindLoginInfo(const blink::WebNode& node, |
return false; |
blink::WebInputElement input = element.to<blink::WebInputElement>(); |
- LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(input); |
+ LoginToPasswordInfoMap::iterator iter = FindPasswordInfoForElement(input); |
if (iter == login_to_password_info_.end()) |
return false; |