Allow deleting autofill password suggestions on Shift+Delete

components/autofill/content/renderer/password_autofill_agent.cc

 // Copyright 2013 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/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ skipping 363 matching lines @@
                                                    password_forms));
   } else {
     Send(new AutofillHostMsg_PasswordFormsParsed(routing_id(), password_forms));
   }
 }
 
 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;
 }
 
 void PasswordAutofillAgent::DidStartLoading() {
   if (usernames_usage_ != NOTHING_TO_AUTOFILL) {
     UMA_HISTOGRAM_ENUMERATION("PasswordManager.OtherPossibleUsernamesUsage",
                               usernames_usage_, OTHER_POSSIBLE_USERNAMES_MAX);
     usernames_usage_ = NOTHING_TO_AUTOFILL;
@@ skipping 155 matching lines @@
     FormFieldData field;
     FindFormAndFieldForInputElement(
         username_element, &form, &field, REQUIRE_NONE);
     Send(new AutofillHostMsg_AddPasswordFormMapping(
         routing_id(),
         field,
         form_data));
   }
 }
 
+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:
 
 void PasswordAutofillAgent::GetSuggestions(
     const PasswordFormFillData& fill_data,
     const base::string16& input,
     std::vector<base::string16>* suggestions,
     std::vector<base::string16>* realms) {
   if (StartsWith(fill_data.basic_data.fields[0].value, input, false)) {
     suggestions->push_back(fill_data.basic_data.fields[0].value);
@@ skipping 36 matching lines @@
   std::vector<base::string16> suggestions;
   std::vector<base::string16> realms;
   GetSuggestions(fill_data, user_input.value(), &suggestions, &realms);
   DCHECK_EQ(suggestions.size(), realms.size());
 
   FormData form;
   FormFieldData field;
   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());
 
   float scale = web_view_->pageScaleFactor();
   gfx::RectF bounding_box_scaled(bounding_box.x() * scale,
                                  bounding_box.y() * scale,
                                  bounding_box.width() * scale,
                                  bounding_box.height() * scale);
   Send(new AutofillHostMsg_ShowPasswordSuggestions(routing_id(),
                                                    field,
+                                                   *password_form,
                                                    bounding_box_scaled,
                                                    suggestions,
                                                    realms));
   return !suggestions.empty();
 }
 
 void PasswordAutofillAgent::FillFormOnPasswordRecieved(
     const PasswordFormFillData& fill_data,
     blink::WebInputElement username_element,
     blink::WebInputElement password_element) {
@@ skipping 189 matching lines @@
 }
 
 PasswordAutofillAgent::AutofillWebUserGestureHandler::
     AutofillWebUserGestureHandler(PasswordAutofillAgent* agent)
     : agent_(agent) {}
 
 PasswordAutofillAgent::AutofillWebUserGestureHandler::
     ~AutofillWebUserGestureHandler() {}
 
 }  // namespace autofill