Allow deleting autofill password suggestions on Shift+Delete

components/autofill/content/renderer/password_autofill_agent.cc

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 d59aeefe9761283cca12ef84d1080f9a578a23ac..195ab719793de2d5a1241c00e326ec8c1438c073 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -381,6 +381,8 @@ bool PasswordAutofillAgent::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(PasswordAutofillAgent, message)
     IPC_MESSAGE_HANDLER(AutofillMsg_FillPasswordForm, OnFillPasswordForm)
+    IPC_MESSAGE_HANDLER(AutofillMsg_RemovePasswordSuggestion,
+                        OnRemovePasswordSuggestion)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -556,6 +558,36 @@ void PasswordAutofillAgent::OnFillPasswordForm(
   }
 }
 
+void PasswordAutofillAgent::OnRemovePasswordSuggestion(
+    const PasswordForm& password_form) {
+
+  FormElementsList forms;
+  // We own the FormElements* in forms.

[vabr (Chromium), 2014/02/05 20:34:20]

nit: Add the vertical bars around forms: |forms|, to avoid confusion with the
general meaning of the word.

[vabr (Chromium), 2014/02/05 20:34:20]

Actually, could you please remove the comment? It only seemed needed because of
the non-obvious use of scoped_ptr |form_elements| below, which I would like to
eliminated (see below).

[riadh.chtara, 2014/02/11 21:03:46]

Done.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

+  blink::WebInputElement username_element;
+  FindFormElements(render_view()->GetWebView(), password_form.form_data, &forms);

[vabr (Chromium), 2014/02/05 20:34:20]

This line, and a couple of lines below, seem to be longer than 80 characters,
please reformat.

(Did the presubmit check not warn you?)

[riadh.chtara, 2014/02/11 21:03:46]

Done.

+  FormElementsList::iterator iter;
+  for (iter = forms.begin(); iter != forms.end(); ++iter) {
+    scoped_ptr<FormElements> form_elements(*iter);

[vabr (Chromium), 2014/02/05 20:34:20]

Please don't do this. If you leave the for cycle earlier (and you will, through
the break statement), the rest of the FormElements will never get freed.

Instead leave |forms| as is in this cycle, and add
STLDeleteElements(forms);
after the for cycle.

+
+    // First, get pointers to username element.
+    username_element =
+        form_elements->input_elements[password_form.form_data.fields[0].name];
+    if (login_to_password_info_.find(username_element) !=

[vabr (Chromium), 2014/02/05 20:34:20]

Please prefer ContainsKey from base/stl_util.h to the find != end pattern, for
readability of the code.

[vabr (Chromium), 2014/02/05 20:34:20]

nit: Instead of doing

for (...) {
  if (condition) {
    // many lines
  }
}

do

for (...) {
  if (!condition)
    continue;
  // many lines
}

This will indent the "//many lines" part to the left, and decrease the level of
nesting, which is better for readability.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

+      login_to_password_info_.end()) {
+      login_to_password_info_.erase(username_element);
+      Send(new AutofillHostMsg_RemovePasswordSuggestion(routing_id(),
+                                                  password_form));

[vabr (Chromium), 2014/02/05 20:34:20]

Please indent correctly.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

+      std::vector<PasswordForm> password_forms;
+      scoped_ptr<PasswordForm> password_form(CreatePasswordForm(username_element.form()));

[vabr (Chromium), 2014/02/05 20:34:20]

Also, note that now you have a name collision: the function argument
|password_form| and the local scoped_ptr |password_form|. Please avoid that.

[riadh.chtara, 2014/02/11 21:03:46]

Done.

+      if (password_form.get()) {
+        password_forms.push_back(*password_form);
+        Send(new AutofillHostMsg_PasswordFormsParsed(routing_id(), password_forms));

[vabr (Chromium), 2014/02/05 20:34:20]

I'm not sure I follow you here: Why are you sending the PasswordFormsParsed
message?

[riadh.chtara, 2014/02/11 21:03:46]

|AutofillHostMsg_PasswordFormsParsed| is sent so that every thing related to the
|password_forms| is reinitialized 

+      }
+      break;
+    }
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillAgent, private:
 
@@ -612,6 +644,10 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
   FindFormAndFieldForInputElement(
       user_input, &form, &field, REQUIRE_NONE);
 
+  scoped_ptr<PasswordForm> password_form =
+      CreatePasswordForm(user_input.form());
+  DCHECK(password_form);
+
   blink::WebInputElement selected_element = user_input;
   gfx::Rect bounding_box(selected_element.boundsInViewportSpace());
 
@@ -622,6 +658,7 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
                                  bounding_box.height() * scale);
   Send(new AutofillHostMsg_ShowPasswordSuggestions(routing_id(),
                                                    field,
+                                                   *password_form,
                                                    bounding_box_scaled,
                                                    suggestions,
                                                    realms));