Add Delete Support to New Autofill UI

chrome/browser/autofill/autofill_popup_view.cc

Index: chrome/browser/autofill/autofill_popup_view.cc
diff --git a/chrome/browser/autofill/autofill_popup_view.cc b/chrome/browser/autofill/autofill_popup_view.cc
index 657fc4b7640d51470107e455f4fc979b65c529f2..731f7f469c91536caca18aa591f02442e546161c 100644
--- a/chrome/browser/autofill/autofill_popup_view.cc
+++ b/chrome/browser/autofill/autofill_popup_view.cc
@@ -11,6 +11,9 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h"
+
+using WebKit::WebAutofillClient;
 
 namespace {
 
@@ -112,11 +115,45 @@ bool AutofillPopupView::AcceptSelectedLine() {
 }
 
 bool AutofillPopupView::RemoveSelectedLine() {
-  if (selected_line_ == kNoSelection)
+  if (selected_line_ == kNoSelection ||
+      (separator_index_ != -1 && separator_index_ <= selected_line_))
+    return false;
+
+  DCHECK_GE(selected_line_, 0);
+  DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
+
+  // Don't remove  warnings
+  if (autofill_unique_ids_[selected_line_] ==
+      WebAutofillClient::MenuItemIDWarningMessage)
     return false;
 
-  // TODO(csharp) add removal code.
-  return false;
+  if (autofill_unique_ids_[selected_line_] > 0) {
+    external_delegate()->RemoveAutofillProfileOrCreditCard(
+        autofill_unique_ids_[selected_line_]);
+  } else {
+    external_delegate()->RemoveAutocompleteEntry(
+        autofill_values_[selected_line_]);
+  }
+
+  // Decrease the separator since there is one less entry above it.
+  --separator_index_;

[Ilya Sherman, 2012/04/19 21:01:57]

Do we still need the separator index here, or can we just use the item ids
instead?

[csharp, 2012/04/20 15:03:18]

The only reason to keep the separator index is so that the drawing code knows
where it needs to draw the separator line.

We could modify the drawing code to figure out where to draw it on its own, but
since ever UI will need it I feel that its worth keeping it in.

[Ilya Sherman, 2012/04/20 21:30:48]

In order to support DataList entries -- which is what [
https://chromiumcodereview.appspot.com/10024059 ] added to the renderer-based
popup -- we'll need to be able to support multiple separators.  So, long-term,
we won't be able to use the separator_index anyway; given that, I think we might
as well go ahead and drop it here.

[csharp, 2012/04/24 14:18:48]

Done.

+
+  // 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_);
+
+  // Resize the popup.
+  ResizePopup();
+
+  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,