Add Delete Support to New Autofill UI

chrome/browser/autofill/autofill_popup_unittest.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 "base/memory/scoped_ptr.h"
 #include "chrome/browser/autofill/autofill_popup_view.h"
 #include "chrome/browser/autofill/test_autofill_external_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+using ::testing::_;
+using ::testing::AtLeast;
+
 namespace {
 
+const int kNoSelection = -1;
+
 class MockAutofillExternalDelegate : public TestAutofillExternalDelegate {
  public:
   MockAutofillExternalDelegate() : TestAutofillExternalDelegate(NULL, NULL) {};
   virtual ~MockAutofillExternalDelegate() {};
 
   virtual void SelectAutofillSuggestionAtIndex(int unique_id, int list_index)
       OVERRIDE {}
+
+  virtual void ClearPreviewedForm() OVERRIDE {}
 };
 
 class TestAutofillPopupView : public AutofillPopupView {
  public:
   explicit TestAutofillPopupView(AutofillExternalDelegate* external_delegate) :
       AutofillPopupView(NULL, external_delegate) {
-    std::vector<string16> autofill_values;
-    autofill_values.push_back(string16());
-    autofill_values.push_back(string16());
+    std::vector<string16> autofill_values(2, string16());
 
-    std::vector<int> autofill_ids;
-    autofill_ids.push_back(0);
-    autofill_ids.push_back(1);
+    std::vector<int> autofill_ids(2, 0);
 
-    Show(autofill_values, autofill_values, autofill_values, autofill_ids, 0);
+    Show(autofill_values, autofill_values, autofill_values, autofill_ids, 1);

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

Hmm, it's not great that there is this long-distance dependency between the
constructor and the tests that expect these particular values to be shown.  It
would be better to do this set up in each test separately (it's not a whole lot
of code), so that the tests are more independent.

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

Done.

   }
   virtual ~TestAutofillPopupView() {}
 
   // Making protected functions public for testing
   const std::vector<string16>& autofill_values() const {
     return AutofillPopupView::autofill_values();
   }
   int selected_line() const {
     return AutofillPopupView::selected_line();
   }
   void SetSelectedLine(size_t selected_line) {
     AutofillPopupView::SetSelectedLine(selected_line);
   }
   void SelectNextLine() {
     AutofillPopupView::SelectNextLine();
   }
   void SelectPreviousLine() {
     AutofillPopupView::SelectPreviousLine();
   }
+  bool RemoveSelectedLine() {
+    return AutofillPopupView::RemoveSelectedLine();
+  }
 
   MOCK_METHOD1(InvalidateRow, void(size_t));
+  MOCK_METHOD0(HideInternal, void());
 
  private:
   virtual void ShowInternal() OVERRIDE {}
-  virtual void HideInternal() OVERRIDE {}
 };
 
 }  // namespace
 
 class AutofillPopupViewUnitTest : public ::testing::Test {
  public:
   AutofillPopupViewUnitTest() {
     autofill_popup_view_.reset(new TestAutofillPopupView(&external_delegate_));
   }
   virtual ~AutofillPopupViewUnitTest() {}
@@ skipping 30 matching lines @@
   autofill_popup_view_->SetSelectedLine(selected_line);
 
   // Ensure that the row isn't invalidated if it didn't change.
   EXPECT_CALL(*autofill_popup_view_, InvalidateRow(selected_line)).Times(0);
   autofill_popup_view_->SetSelectedLine(selected_line);
 
   // Change back to no selection.
   EXPECT_CALL(*autofill_popup_view_, InvalidateRow(selected_line));
   autofill_popup_view_->SetSelectedLine(-1);
 }
+
+TEST_F(AutofillPopupViewUnitTest, RemoveLine) {
+  // To remove warnings.
+  EXPECT_CALL(*autofill_popup_view_, InvalidateRow(_)).Times(AtLeast(0));
+
+  // No line is selected so the removal should fail.
+  EXPECT_FALSE(autofill_popup_view_->RemoveSelectedLine());
+
+  // Try to remove the last entry and ensure it fails (it is an option).
+  autofill_popup_view_->SetSelectedLine(
+      autofill_popup_view_->autofill_values().size() - 1);
+  EXPECT_FALSE(autofill_popup_view_->RemoveSelectedLine());
+  EXPECT_NE(kNoSelection, autofill_popup_view_->selected_line());

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

nit: Perhaps EXPECT_GE(0, ...) so that kNoSelection is not part of the public
contract for the class?  Or, if you prefer to use the constant in the test,
please declare it in a place that both the test code and the regular
implementation can access.

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

Replaced with EXPECT_LE(0, 

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

Sorry, I meant EXPECT_GE(..., 0) -- for the EXPECT_LE-type macros, the more
constant-y number typically goes on the right.

+
+  // Remove the first (and only) entry. The popup should then be hidden since
+  // there are no Autofill entries left.
+  EXPECT_CALL(*autofill_popup_view_, HideInternal());
+  autofill_popup_view_->SetSelectedLine(0);
+  EXPECT_TRUE(autofill_popup_view_->RemoveSelectedLine());
+}