Add Delete Support to New Autofill UI

chrome/browser/autofill/autofill_popup_view.cc

 // Copyright (c) 2012 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 "chrome/browser/autofill/autofill_popup_view.h"
 
 #include "base/logging.h"
 #include "chrome/browser/autofill/autofill_external_delegate.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/navigation_controller.h"
@@ skipping 94 matching lines @@
   DCHECK_GE(selected_line_, 0);
   DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
 
   return external_delegate()->DidAcceptAutofillSuggestions(
       autofill_values_[selected_line_],
       autofill_unique_ids_[selected_line_],
       selected_line_);
 }
 
 bool AutofillPopupView::RemoveSelectedLine() {
-  if (selected_line_ == kNoSelection)
+  if (selected_line_ == kNoSelection ||
+      (separator_index_ != -1 && separator_index_ <= selected_line_))
     return false;

[Ilya Sherman, 2012/04/17 08:31:20]

It should also be impossible to delete a warning.

[csharp, 2012/04/18 15:35:58]

Done.

 
-  // TODO(csharp) add removal code.
-  return false;
+  DCHECK_GE(selected_line_, 0);
+  DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
+
+  // If the user attempts to delete an Autofill entry, just hide the popup.

[Ilya Sherman, 2012/04/17 08:31:20]

Ideally we would delete the autofill entry as well.  If that's easy to
implement, it would be great to implement it as part of this CL.

If it's not easy to implement, we should at least have a TODO or a bug filed :) 
Also, I'm not convinced that we really should be hiding the popup if we can't
delete the entry -- I'd prefer to just not handle the event at all.

[csharp, 2012/04/18 15:35:58]

K, I dug into the code a bit and got deletion of profiles and credit cards
hooked up.

I also noticed that shift-delete doesn't seem to delete profile in the current
Autofill UI, it just closes the popup. Is this a bug or was this just the old
way to do it?

+  if (autofill_unique_ids_[selected_line_] > 0) {
+    Hide();
+    return true;
+  }
+
+  external_delegate()->RemoveAutocompleteEntry(
+      autofill_values_[selected_line_]);
+
+  --separator_index_;
+
+  // Remove the deleted element.
+  autofill_values_.erase(autofill_values_.begin() + selected_line_);
+  autofill_labels_.erase(autofill_labels_.begin() + selected_line_);
+  autofill_icons_.erase(autofill_icons_.begin() + selected_line_);
+  autofill_unique_ids_.erase(autofill_unique_ids_.begin() + selected_line_);
+
+  SetSelectedLine(kNoSelection);
+
+  // Hide the popup if all the Autofill entries are gone.
+  if (autofill_values_.size() == 0 || separator_index_ == 0)
+    Hide();
+
+  return true;
 }
 
 void AutofillPopupView::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {
   if (type == content::NOTIFICATION_WEB_CONTENTS_HIDDEN
       || type == content::NOTIFICATION_NAV_ENTRY_COMMITTED)
     Hide();
 }