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..e5713f3c433f982c90b46cf8cc6c92d42956d98a 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 {
 
@@ -23,7 +26,6 @@ AutofillPopupView::AutofillPopupView(
     content::WebContents* web_contents,
     AutofillExternalDelegate* external_delegate)
     : external_delegate_(external_delegate),
-      separator_index_(0),
       selected_line_(kNoSelection) {
   if (!web_contents)
     return;
@@ -49,15 +51,12 @@ void AutofillPopupView::Hide() {
 void AutofillPopupView::Show(const std::vector<string16>& autofill_values,
                              const std::vector<string16>& autofill_labels,
                              const std::vector<string16>& autofill_icons,
-                             const std::vector<int>& autofill_unique_ids,
-                             int separator_index) {
+                             const std::vector<int>& autofill_unique_ids) {
   autofill_values_ = autofill_values;
   autofill_labels_ = autofill_labels;
   autofill_icons_ = autofill_icons;
   autofill_unique_ids_ = autofill_unique_ids;
 
-  separator_index_ = separator_index;
-
   ShowInternal();
 }
 
@@ -75,8 +74,7 @@ void AutofillPopupView::SetSelectedLine(int selected_line) {
 
   if (selected_line_ != kNoSelection) {
     external_delegate_->SelectAutofillSuggestionAtIndex(
-        autofill_unique_ids_[selected_line_],
-        selected_line);
+        autofill_unique_ids_[selected_line_]);
   }
 }
 
@@ -115,8 +113,47 @@ bool AutofillPopupView::RemoveSelectedLine() {
   if (selected_line_ == kNoSelection)
     return false;
 
-  // TODO(csharp) add removal code.
-  return false;
+  DCHECK_GE(selected_line_, 0);
+  DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
+
+  if (!CanDelete(autofill_unique_ids_[selected_line_]))
+    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_]);
+  }
+
+  // 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 || !CanDelete(autofill_unique_ids_[0]))

[Ilya Sherman, 2012/04/24 20:08:12]

I don't think this check will quite make sense once we include DataList
suggestions, since they might be above Autofill suggestions.

[csharp, 2012/04/25 15:05:09]

Done.

+    Hide();
+
+  return true;
+}
+
+bool AutofillPopupView::IsSeparatorIndex(int index) {
+  return (index > 0 && autofill_unique_ids_[index - 1] >= 0 &&
+          autofill_unique_ids_[index] < 0);

[Ilya Sherman, 2012/04/24 20:08:12]

Hmm, why isn't this just "return autofill_unique_ids[index] ==
WebAutofillClient::MenuItemIDSeparator"?

[csharp, 2012/04/25 15:05:09]

WebAutofillClient::MenuItemIDsperator is only set in the old Autofill UI right
now. I've added a bug to change this to use that id since its more work than I'd
like to add to this change as it would introduce an elements that doesn't appear
as a full row when drawing the popup.

MenuItemIDSeparator isn't set for any of the lists passed to the new Autofill UI
right now, only 

+}
+
+bool AutofillPopupView::CanDelete(int id) {
+  return id != WebAutofillClient::MenuItemIDWarningMessage &&
+      id != WebAutofillClient::MenuItemIDClearForm &&
+      id != WebAutofillClient::MenuItemIDAutofillOptions;

[Ilya Sherman, 2012/04/24 20:08:12]

Since the full set of ids might grow over time, it's probably better to check
for equality to deletable types rather than inequality to non-deletable types
here.

[csharp, 2012/04/25 15:05:09]

Done.

 }
 
 void AutofillPopupView::Observe(int type,