Autofill item deletion on android

chrome/browser/ui/android/autofill/autofill_popup_view_android.cc

Index: chrome/browser/ui/android/autofill/autofill_popup_view_android.cc
diff --git a/chrome/browser/ui/android/autofill/autofill_popup_view_android.cc b/chrome/browser/ui/android/autofill/autofill_popup_view_android.cc
index b376d9cc24bc7ef5b56cffdeb1afef7eb2759d5d..a03ff99e4b1314599bd750e8e50d9d09a5bce268 100644
--- a/chrome/browser/ui/android/autofill/autofill_popup_view_android.cc
+++ b/chrome/browser/ui/android/autofill/autofill_popup_view_android.cc
@@ -25,7 +25,8 @@ namespace autofill {
 
 AutofillPopupViewAndroid::AutofillPopupViewAndroid(
     AutofillPopupController* controller)
-    : controller_(controller) {}
+    : controller_(controller),
+      deleting_index_(-1) {}
 
 AutofillPopupViewAndroid::~AutofillPopupViewAndroid() {}
 
@@ -77,6 +78,8 @@ void AutofillPopupViewAndroid::UpdateBoundsAndRedrawPopup() {
           controller_->GetIconResourceID(suggestion.icon));
     }
 
+    bool deletable =
+        controller_->GetRemovalConfirmationText(i, nullptr, nullptr);
     Java_AutofillPopupBridge_addToAutofillSuggestionArray(
         env,
         data_array.obj(),
@@ -84,7 +87,8 @@ void AutofillPopupViewAndroid::UpdateBoundsAndRedrawPopup() {
         value.obj(),
         label.obj(),
         android_icon_id,
-        suggestion.frontend_id);
+        suggestion.frontend_id,
+        deletable);
   }
 
   Java_AutofillPopupBridge_show(
@@ -99,6 +103,37 @@ void AutofillPopupViewAndroid::SuggestionSelected(JNIEnv* env,
     controller_->AcceptSuggestion(list_index);
 }
 
+void AutofillPopupViewAndroid::DeleteSuggestion(JNIEnv* env,

[Ted C, 2015/05/14 21:56:39]

Why does it need to call down to this for deletion?  Can't the index be tracked
purely in javaland?

Seems like all we really need to expose is a method to get the removal
confirmation text body.

[Evan Stade, 2015/05/14 23:12:05]

The title as well as the body, but sure, that's a possible alternative. What is
the main advantage? It seems functionally pretty similar.

[Ted C, 2015/05/14 23:21:18]

To me it's just a bit less jumping around.  In most cases, I would just keep the
id local to the callback that will handle the delete.

The fact that you need both the title and body and it is a complex method, I
think this is ok.

I might rename this method to something like triggerDeleteConfirmation or
something since deleteSuggestion makes me think it's actually doing the delete.

[Evan Stade, 2015/05/15 16:49:59]

changed to DeletionRequested. Thanks!

+                                                jobject obj,
+                                                jint list_index) {
+  if (!controller_)
+    return;
+
+  base::string16 confirmation_title, confirmation_body;
+  if (!controller_->GetRemovalConfirmationText(list_index, &confirmation_title,
+          &confirmation_body)) {
+    return;
+  }
+
+  deleting_index_ = list_index;
+  Java_AutofillPopupBridge_confirmDeletion(
+      env,
+      java_object_.obj(),
+      base::android::ConvertUTF16ToJavaString(
+          env, confirmation_title).obj(),
+      base::android::ConvertUTF16ToJavaString(
+          env, confirmation_body).obj());
+}
+
+void AutofillPopupViewAndroid::DeletionConfirmed(JNIEnv* env,
+                                                 jobject obj) {
+  if (!controller_)
+    return;
+
+  CHECK_GE(deleting_index_, 0);
+  controller_->RemoveSuggestion(deleting_index_);
+}
+
 void AutofillPopupViewAndroid::PopupDismissed(JNIEnv* env, jobject obj) {
   if (controller_)
     controller_->ViewDestroyed();