Datalist UI (WebKit part)

Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp

Index: Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
diff --git a/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp b/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
index c84ea12f60e9c7539f921907383c46a77a709664..54d275e3e8737df65e5376143fb5403a4d710032 100644
--- a/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
+++ b/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
@@ -52,9 +52,7 @@ using namespace WebCore;
 namespace WebKit {
 
 AutofillPopupMenuClient::AutofillPopupMenuClient()
-    : m_separatorIndex(-1)
-    , m_selectedIndex(-1)

[Ilya Sherman, 2012/04/11 00:27:52]

nit: We should still initialize m_selectedIndex.

[keishi, 2012/04/11 12:13:12]

Done.

-    , m_textField(0)
+    : m_textField(0)
 {
 }
 
@@ -64,37 +62,25 @@ AutofillPopupMenuClient::~AutofillPopupMenuClient()
 
 unsigned AutofillPopupMenuClient::getSuggestionsCount() const
 {
-    return m_names.size() + ((m_separatorIndex == -1) ? 0 : 1);
+    return m_names.size();
 }
 
 WebString AutofillPopupMenuClient::getSuggestion(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_names.size());
-    return m_names[index];
+    ASSERT(listIndex >= 0 && static_cast<size_t>(listIndex) < m_names.size());

[Ilya Sherman, 2012/04/11 00:27:52]

nit: No need to check nonnegativity nor to cast -- the listIndex is already
unsigned.

[keishi, 2012/04/11 12:13:12]

Done.

+    return m_names[listIndex];
 }
 
 WebString AutofillPopupMenuClient::getLabel(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_labels.size());
-    return m_labels[index];
+    ASSERT(listIndex >= 0 && static_cast<size_t>(listIndex) < m_labels.size());

[Ilya Sherman, 2012/04/11 00:27:52]

nit: Ditto

[keishi, 2012/04/11 12:13:12]

Done.

+    return m_labels[listIndex];
 }
 
 WebString AutofillPopupMenuClient::getIcon(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_icons.size());
-    return m_icons[index];
+    ASSERT(listIndex >= 0 && static_cast<size_t>(listIndex) < m_icons.size());

[Ilya Sherman, 2012/04/11 00:27:52]

nit: Ditto

[keishi, 2012/04/11 12:13:12]

Done.

+    return m_icons[listIndex];
 }
 
 void AutofillPopupMenuClient::removeSuggestionAtIndex(unsigned listIndex)
@@ -102,26 +88,19 @@ void AutofillPopupMenuClient::removeSuggestionAtIndex(unsigned listIndex)
     if (!canRemoveSuggestionAtIndex(listIndex))
         return;
 
-    int index = convertListIndexToInternalIndex(listIndex);
-
-    ASSERT(static_cast<unsigned>(index) < m_names.size());
+    ASSERT(static_cast<unsigned>(listIndex) < m_names.size());

[Ilya Sherman, 2012/04/11 00:27:52]

nit: No need to cast, the listIndex is already unsigned.

[keishi, 2012/04/11 12:13:12]

Done.

 
-    m_names.remove(index);
-    m_labels.remove(index);
-    m_icons.remove(index);
-    m_uniqueIDs.remove(index);
-
-    // Shift the separator index if necessary.
-    if (m_separatorIndex != -1)
-        m_separatorIndex--;
+    m_names.remove(listIndex);
+    m_labels.remove(listIndex);
+    m_icons.remove(listIndex);
+    m_itemIDs.remove(listIndex);
 }
 
 bool AutofillPopupMenuClient::canRemoveSuggestionAtIndex(unsigned listIndex)
 {
-    // Only allow deletion of items before the separator that have unique id 0
-    // (i.e. are autocomplete rather than autofill items).
-    int index = convertListIndexToInternalIndex(listIndex);
-    return !m_uniqueIDs[index] && (m_separatorIndex == -1 || listIndex < static_cast<unsigned>(m_separatorIndex));
+    return m_itemIDs[listIndex] > 0 || // Is autofill entry?

[Ilya Sherman, 2012/04/11 00:27:52]

It's not currently possible to remove Autofill entries.  I'm also not sure about
password suggestions.

[keishi, 2012/04/11 12:13:12]

Done. I'll match the current behavior.

+           m_itemIDs[listIndex] == AutocompleteEntryMenuItemID ||
+           m_itemIDs[listIndex] == PasswordEntryMenuItemID;
 }
 
 void AutofillPopupMenuClient::valueChanged(unsigned listIndex, bool fireEvents)
@@ -130,15 +109,12 @@ void AutofillPopupMenuClient::valueChanged(unsigned listIndex, bool fireEvents)
     if (!webView)
         return;
 
-    if (m_separatorIndex != -1 && listIndex > static_cast<unsigned>(m_separatorIndex))
-        --listIndex;
-
     ASSERT(listIndex < m_names.size());
 
     webView->autofillClient()->didAcceptAutofillSuggestion(WebNode(getTextField()),
                                                            m_names[listIndex],
                                                            m_labels[listIndex],
-                                                           m_uniqueIDs[listIndex],
+                                                           m_itemIDs[listIndex],
                                                            listIndex);
 }
 
@@ -148,15 +124,12 @@ void AutofillPopupMenuClient::selectionChanged(unsigned listIndex, bool fireEven
     if (!webView)
         return;
 
-    if (m_separatorIndex != -1 && listIndex > static_cast<unsigned>(m_separatorIndex))
-        --listIndex;
-
     ASSERT(listIndex < m_names.size());
 
     webView->autofillClient()->didSelectAutofillSuggestion(WebNode(getTextField()),
                                                            m_names[listIndex],
                                                            m_labels[listIndex],
-                                                           m_uniqueIDs[listIndex]);
+                                                           m_itemIDs[listIndex]);
 }
 
 void AutofillPopupMenuClient::selectionCleared()
@@ -228,17 +201,12 @@ void AutofillPopupMenuClient::popupDidHide()
 
 bool AutofillPopupMenuClient::itemIsSeparator(unsigned listIndex) const
 {
-    return (m_separatorIndex != -1 && static_cast<unsigned>(m_separatorIndex) == listIndex);
+    return m_itemIDs[listIndex] == SeparatorMenuItemID;
 }
 
 bool AutofillPopupMenuClient::itemIsWarning(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return false;
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_uniqueIDs.size());
-    return m_uniqueIDs[index] < 0;
+    return m_itemIDs[listIndex] == WarningMessageMenuItemID;
 }
 
 void AutofillPopupMenuClient::setTextFromItem(unsigned listIndex)
@@ -269,20 +237,18 @@ void AutofillPopupMenuClient::initialize(
     const WebVector<WebString>& names,
     const WebVector<WebString>& labels,
     const WebVector<WebString>& icons,
-    const WebVector<int>& uniqueIDs,
-    int separatorIndex)
+    const WebVector<int>& itemIDs)
 {
     ASSERT(names.size() == labels.size());
     ASSERT(names.size() == icons.size());
-    ASSERT(names.size() == uniqueIDs.size());
-    ASSERT(separatorIndex < static_cast<int>(names.size()));
+    ASSERT(names.size() == itemIDs.size());
 
     m_selectedIndex = -1;
     m_textField = textField;
 
     // The suggestions must be set before initializing the
     // AutofillPopupMenuClient.
-    setSuggestions(names, labels, icons, uniqueIDs, separatorIndex);
+    setSuggestions(names, labels, icons, itemIDs);
 
     FontDescription regularFontDescription;
     RenderTheme::defaultTheme()->systemFont(CSSValueWebkitControl,
@@ -313,42 +279,28 @@ void AutofillPopupMenuClient::initialize(
 void AutofillPopupMenuClient::setSuggestions(const WebVector<WebString>& names,
                                              const WebVector<WebString>& labels,
                                              const WebVector<WebString>& icons,
-                                             const WebVector<int>& uniqueIDs,
-                                             int separatorIndex)
+                                             const WebVector<int>& itemIDs)
 {
     ASSERT(names.size() == labels.size());
     ASSERT(names.size() == icons.size());
-    ASSERT(names.size() == uniqueIDs.size());
-    ASSERT(separatorIndex < static_cast<int>(names.size()));
+    ASSERT(names.size() == itemIDs.size());
 
     m_names.clear();
     m_labels.clear();
     m_icons.clear();
-    m_uniqueIDs.clear();
+    m_itemIDs.clear();
     for (size_t i = 0; i < names.size(); ++i) {
         m_names.append(names[i]);
         m_labels.append(labels[i]);
         m_icons.append(icons[i]);
-        m_uniqueIDs.append(uniqueIDs[i]);
+        m_itemIDs.append(itemIDs[i]);
     }
 
-    m_separatorIndex = separatorIndex;
-
     // Try to preserve selection if possible.
     if (getSelectedIndex() >= static_cast<int>(names.size()))
         setSelectedIndex(-1);
 }
 
-int AutofillPopupMenuClient::convertListIndexToInternalIndex(unsigned listIndex) const
-{
-    if (listIndex == static_cast<unsigned>(m_separatorIndex))
-        return -1;
-
-    if (m_separatorIndex == -1 || listIndex < static_cast<unsigned>(m_separatorIndex))
-        return listIndex;
-    return listIndex - 1;
-}
-
 WebViewImpl* AutofillPopupMenuClient::getWebView() const
 {
     Frame* frame = m_textField->document()->frame();