Add Delete Support to New Autofill UI

chrome/browser/autofill/autofill_manager.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_manager.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <map>
@@ skipping 669 matching lines @@
 void AutofillManager::OnShowPasswordGenerationPopup(const gfx::Rect& bounds) {
 #if defined(OS_ANDROID)
   NOTIMPLEMENTED();
 #else
   Browser* browser = BrowserList::GetLastActiveWithProfile(
       Profile::FromBrowserContext(web_contents()->GetBrowserContext()));
   browser->window()->ShowPasswordGenerationBubble(bounds);
 #endif  // #if defined(OS_ANDROID)
 }
 
+void AutofillManager::RemoveAutofillProfileOrCreditCard(int unique_id) {
+ const std::vector<AutofillProfile*>& profiles = personal_data_->profiles();

[Ilya Sherman, 2012/04/18 18:12:51]

nit: indentation?

[csharp, 2012/04/19 15:33:24]

Done.

+  const std::vector<CreditCard*>& credit_cards = personal_data_->credit_cards();
+  const AutofillProfile* profile = NULL;
+  const CreditCard* credit_card = NULL;
+  size_t variant = 0;

[Ilya Sherman, 2012/04/18 18:12:51]

Hmm, I wonder if we should really delete the whole profile if the variant is not
the default one...  More likely, we should just delete that variant.  WDYT?

[csharp, 2012/04/19 15:33:24]

That sounds good to me. I did a little poking around though and I'm not sure how
to delete just a variant of a profile (I don't see any existing functions that
do that).

I've add a todo and filled a bug (crbug.com/124211). I didn't make myself owner
because I haven't poked around with the internals of Autofill too much. I'd be
willing to tackle it after wrapping up the other UI parts though.

+  if (!GetProfileOrCreditCard(unique_id, profiles, credit_cards, &profile,
+                              &credit_card, &variant))
+    return;

[Ilya Sherman, 2012/04/18 18:12:51]

nit: I think we can add a NOTREACHED() to this if-stmt.  Does that sound right
to you, or can you think of a legitimate reason why this might fail?

[csharp, 2012/04/19 15:33:24]

Seems reasonable. If we get here then so thing has already gone wrong, since the
unique_ids should always be mapping to something.

+
+  if (!personal_data_)
+    return;

[Ilya Sherman, 2012/04/18 18:12:51]

You already used personal_data_ above... (lines 691 and 692)

[csharp, 2012/04/19 15:33:24]

That's what I get for copy the GetProfileOrCreditCard function call

+
+  if (profile) {
+    personal_data_->RemoveProfile(profile->guid());
+  } else if (credit_card) {

[Ilya Sherman, 2012/04/18 18:12:51]

nit: No need for the "if" part of the else stmt -- at least one of the two is
guaranteed to be non-null.

[csharp, 2012/04/19 15:33:24]

Done.

+    personal_data_->RemoveCreditCard(credit_card->guid());
+  }

[Ilya Sherman, 2012/04/18 18:12:51]

nit: No need for curly braces.

[csharp, 2012/04/19 15:33:24]

Done.

+}
+
 void AutofillManager::OnAddPasswordFormMapping(
       const webkit::forms::FormField& form,
       const webkit::forms::PasswordFormFillData& fill_data) {
   if (external_delegate_)
     external_delegate_->AddPasswordFormMapping(form, fill_data);
 }
 
 void AutofillManager::OnShowPasswordSuggestions(
     const webkit::forms::FormField& field,
     const gfx::Rect& bounds,
@@ skipping 617 matching lines @@
   *profile_guid = IDToGUID(profile_id);
 }
 
 void AutofillManager::UpdateInitialInteractionTimestamp(
     const TimeTicks& interaction_timestamp) {
   if (initial_interaction_timestamp_.is_null() ||
       interaction_timestamp < initial_interaction_timestamp_) {
     initial_interaction_timestamp_ = interaction_timestamp;
   }
 }