Fix a DCHECK failure in AutocompleteEditModel::OnPopupDataChanged()

chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc

Index: chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
index 01918989d1ac8cc96b6a32c7c9659137b99b36f4..ab256c7b1ebe9be3791d1bd50a4ad6b6846dd7b5 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
@@ -96,15 +96,15 @@ const struct TestHistoryEntry {
   int typed_count;
   bool starred;
 } kHistoryEntries[] = {
-  {"http://www.bar.com/1", "Page 1", kSearchText, 1, 1, false },
-  {"http://www.bar.com/2", "Page 2", kSearchText, 1, 1, false },
-  {"http://www.bar.com/3", "Page 3", kSearchText, 1, 1, false },
-  {"http://www.bar.com/4", "Page 4", kSearchText, 1, 1, false },
-  {"http://www.bar.com/5", "Page 5", kSearchText, 1, 1, false },
-  {"http://www.bar.com/6", "Page 6", kSearchText, 1, 1, false },
-  {"http://www.bar.com/7", "Page 7", kSearchText, 1, 1, false },
-  {"http://www.bar.com/8", "Page 8", kSearchText, 1, 1, false },
-  {"http://www.bar.com/9", "Page 9", kSearchText, 1, 1, false },
+  {"http://www.bar.com/1", "Page 1", kSearchText, 10, 10, false },
+  {"http://www.bar.com/2", "Page 2", kSearchText, 9, 9, false },
+  {"http://www.bar.com/3", "Page 3", kSearchText, 8, 8, false },
+  {"http://www.bar.com/4", "Page 4", kSearchText, 7, 7, false },
+  {"http://www.bar.com/5", "Page 5", kSearchText, 6, 6, false },
+  {"http://www.bar.com/6", "Page 6", kSearchText, 5, 5, false },
+  {"http://www.bar.com/7", "Page 7", kSearchText, 4, 4, false },
+  {"http://www.bar.com/8", "Page 8", kSearchText, 3, 3, false },
+  {"http://www.bar.com/9", "Page 9", kSearchText, 2, 2, false },
 
   // To trigger inline autocomplete.
   {"http://www.def.com", "Page def", kSearchText, 10000, 10000, true },
@@ -244,6 +244,11 @@ class AutocompleteEditViewTest : public InProcessBrowserTest,
     }
 
     ASSERT_TRUE(model->loaded());
+    // Remove old template urls to avoid interfering our tests.

[Peter Kasting, 2011/01/25 21:18:24]

Nit: What are the old URLs and how would they interfere?

[James Su, 2011/01/25 21:47:03]

Done.

+    model->SetDefaultSearchProvider(NULL);
+    std::vector<const TemplateURL*> old = model->GetTemplateURLs();
+    for (size_t i = 0; i < old.size(); ++i)

[Peter Kasting, 2011/01/25 21:18:24]

Nit: An iterator might be nice

[James Su, 2011/01/25 21:47:03]

Done.

+      model->Remove(old[i]);
 
     TemplateURL* template_url = new TemplateURL();
     template_url->SetURL(kSearchURL, 0, 0);
@@ -509,7 +514,8 @@ class AutocompleteEditViewTest : public InProcessBrowserTest,
     ASSERT_NO_FATAL_FAILURE(SendKeySequence(kDesiredTLDKeys));
     ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
     ASSERT_TRUE(popup_model->IsOpen());
-    // ctrl-Enter triggers desired_tld feature, thus www.bar.com shall be opened.
+    // ctrl-Enter triggers desired_tld feature, thus www.bar.com shall be
+    // opened.
     ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_RETURN, true, false, false));
 
     GURL url = browser()->GetSelectedTabContents()->GetURL();
@@ -788,6 +794,101 @@ class AutocompleteEditViewTest : public InProcessBrowserTest,
     ASSERT_TRUE(edit_view->GetText().empty());
   }
 
+  void DeleteItemTest() {
+    // Disable the search provider, to make sure the popup contains only history
+    // items.
+    TemplateURLModel* model = browser()->profile()->GetTemplateURLModel();
+    model->SetDefaultSearchProvider(NULL);
+
+    ui_test_utils::NavigateToURL(browser(), GURL(chrome::kAboutBlankURL));
+    browser()->FocusLocationBar();
+
+    AutocompleteEditView* edit_view = NULL;
+    ASSERT_NO_FATAL_FAILURE(GetAutocompleteEditView(&edit_view));
+
+    AutocompletePopupModel* popup_model = edit_view->model()->popup_model();
+    ASSERT_TRUE(popup_model);
+
+    std::wstring old_text = edit_view->GetText();
+
+    // Input something that can match history items.
+    edit_view->SetUserText(L"bar");
+    ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
+    ASSERT_TRUE(popup_model->IsOpen());
+
+    // Delete the inline autocomplete part.
+    ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_DELETE, false, false, false));
+    std::wstring user_text = edit_view->GetText();
+    ASSERT_EQ(L"bar", user_text);
+    edit_view->SelectAll(true);
+    ASSERT_TRUE(edit_view->IsSelectAll());
+
+    // The first item should be the default match.
+    size_t default_line = popup_model->selected_line();
+    std::string default_url =
+        popup_model->result().match_at(default_line).destination_url.spec();
+
+    // Move down.
+    edit_view->model()->OnUpOrDownKeyPressed(1);
+    ASSERT_EQ(default_line + 1, popup_model->selected_line());
+    std::wstring selected_text =
+        popup_model->result().match_at(default_line + 1).fill_into_edit;
+    // Temporary text is shown.
+    ASSERT_EQ(selected_text, edit_view->GetText());
+    ASSERT_FALSE(edit_view->IsSelectAll());
+
+    // Delete the item.
+    popup_model->TryDeletingCurrentItem();
+    ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
+    // The selected line shouldn't be changed, because we have more than two
+    // items.
+    ASSERT_EQ(default_line + 1, popup_model->selected_line());
+    // Make sure the item is really deleted.
+    ASSERT_NE(selected_text,
+              popup_model->result().match_at(default_line + 1).fill_into_edit);
+    selected_text =
+        popup_model->result().match_at(default_line + 1).fill_into_edit;
+    // New temporary text is shown.
+    ASSERT_EQ(selected_text, edit_view->GetText());
+
+    // Revert to the default match.
+    ASSERT_TRUE(edit_view->model()->OnEscapeKeyPressed());
+    ASSERT_EQ(default_line, popup_model->selected_line());
+    ASSERT_EQ(user_text, edit_view->GetText());
+    ASSERT_TRUE(edit_view->IsSelectAll());
+
+    // Move down and up to select the default match as temporary text.
+    edit_view->model()->OnUpOrDownKeyPressed(1);
+    ASSERT_EQ(default_line + 1, popup_model->selected_line());
+    edit_view->model()->OnUpOrDownKeyPressed(-1);
+    ASSERT_EQ(default_line, popup_model->selected_line());
+
+    selected_text = popup_model->result().match_at(default_line).fill_into_edit;
+    // New temporary text is shown.
+    ASSERT_EQ(selected_text, edit_view->GetText());
+    ASSERT_FALSE(edit_view->IsSelectAll());
+
+    // Delete the default item.
+    popup_model->TryDeletingCurrentItem();
+    ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
+    // The selected line shouldn't be changed, but the default item should have
+    // been changed.
+    ASSERT_EQ(default_line, popup_model->selected_line());
+    // Make sure the item is really deleted.
+    ASSERT_NE(selected_text,
+              popup_model->result().match_at(default_line).fill_into_edit);
+    selected_text =
+        popup_model->result().match_at(default_line).fill_into_edit;
+    // New temporary text is shown.
+    ASSERT_EQ(selected_text, edit_view->GetText());
+
+    // As the current selected item is the new default item, pressing Escape key
+    // should revert all directly.
+    ASSERT_TRUE(edit_view->model()->OnEscapeKeyPressed());
+    ASSERT_EQ(old_text, edit_view->GetText());
+    ASSERT_TRUE(edit_view->IsSelectAll());
+  }
+
 };
 
 // Test if ctrl-* accelerators are workable in omnibox.
@@ -841,6 +942,10 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, AcceptKeywordBySpace) {
   AcceptKeywordBySpaceTest();
 }
 
+IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, DeleteItem) {
+  DeleteItemTest();
+}
+
 #if defined(OS_LINUX)
 // TODO(oshima): enable these tests for views-implmentation when
 // these featuers are supported.
@@ -1017,4 +1122,8 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewViewsTest, AcceptKeywordBySpace) {
   AcceptKeywordBySpaceTest();
 }
 
+IN_PROC_BROWSER_TEST_F(AutocompleteEditViewViewsTest, DeleteItem) {
+  DeleteItemTest();
+}
+
 #endif