Remove blank items from ExternalPopupMenu

Source/web/tests/ExternalPopupMenuTest.cpp

Index: Source/web/tests/ExternalPopupMenuTest.cpp
diff --git a/Source/web/tests/ExternalPopupMenuTest.cpp b/Source/web/tests/ExternalPopupMenuTest.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..91cae458aa47a6524e396fb40a68ec3ff496b746
--- /dev/null
+++ b/Source/web/tests/ExternalPopupMenuTest.cpp
@@ -0,0 +1,102 @@
+// Copyright (c) 2014 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 "config.h"

[tkent, 2014/08/21 00:53:33]

The main header file for this file is ExternalPopupMenu.h.  So, includes should
be:

#include "config.h"
#include "web/ExternalPopupMenu.h"

#include "platform/PopupMenu.h"
#include "Platform/PopupMenuClient.h"
...

[spartha, 2014/08/21 10:05:44]

Done.

+
+#include "platform/PopupMenu.h"
+
+#include "platform/PopupMenuClient.h"
+#include "public/web/WebPopupMenuInfo.h"
+#include "web/ExternalPopupMenu.h"
+#include <gtest/gtest.h>
+
+using namespace blink;

[tkent, 2014/08/21 00:53:33]

See #4 of http://www.chromium.org/blink/coding-style#TOC-using-Statements

[spartha, 2014/08/21 10:05:44]

I have changed the namespace usage pattern. Most of the test cases are in
anonymous namespace. Now this should be compliant with the coding styles.  

+
+namespace {
+
+const size_t kListSize = 7;
+
+class TestPopupMenuClient : public PopupMenuClient {
+public:
+    TestPopupMenuClient() : m_listSize(0) { }
+    virtual ~TestPopupMenuClient() { }
+
+    virtual void valueChanged(unsigned listIndex, bool fireEvents = true) OVERRIDE { }
+    virtual void selectionChanged(unsigned listIndex, bool fireEvents = true) OVERRIDE { }
+    virtual void selectionCleared() OVERRIDE { }
+
+    virtual String itemText(unsigned listIndex) const OVERRIDE { return emptyString(); }
+    virtual String itemToolTip(unsigned listIndex) const OVERRIDE { return emptyString(); }
+    virtual String itemAccessibilityText(unsigned listIndex) const OVERRIDE { return emptyString(); }
+    virtual bool itemIsEnabled(unsigned listIndex) const OVERRIDE { return true; }
+    virtual PopupMenuStyle itemStyle(unsigned listIndex) const OVERRIDE
+    {
+        FontDescription fontDescription;
+        fontDescription.setComputedSize(12.0);
+        Font font(fontDescription);
+        font.update(nullptr);
+        bool displayNone = m_displayNoneIndexSet.find(listIndex) != m_displayNoneIndexSet.end();
+        return PopupMenuStyle(Color::black, Color::white, font, true, displayNone, Length(), TextDirection(), false /* has text direction override */);
+    }
+    virtual PopupMenuStyle menuStyle() const OVERRIDE { return itemStyle(0); }
+    virtual LayoutUnit clientPaddingLeft() const OVERRIDE { return 0; }
+    virtual LayoutUnit clientPaddingRight() const OVERRIDE { return 0; }
+    virtual int listSize() const OVERRIDE { return m_listSize; }
+    virtual int selectedIndex() const OVERRIDE { return 0; }
+    virtual void popupDidHide() OVERRIDE { }
+    virtual bool itemIsSeparator(unsigned listIndex) const OVERRIDE { return false;}
+    virtual bool itemIsLabel(unsigned listIndex) const OVERRIDE { return false; }
+    virtual bool itemIsSelected(unsigned listIndex) const OVERRIDE { return listIndex == 0;}
+    virtual void setTextFromItem(unsigned listIndex) OVERRIDE { }
+    virtual bool multiple() const OVERRIDE { return false; }
+
+    void setListSize(size_t size) { m_listSize = size; }
+    void setDisplayNoneIndex(unsigned index) { m_displayNoneIndexSet.insert(index); }
+private:
+    size_t m_listSize;
+    std::set<unsigned> m_displayNoneIndexSet;
+};
+
+} // namespace
+
+namespace blink {
+
+class ExternalPopupMenuDisplayNoneItemsTest : public testing::Test {
+public:
+    ExternalPopupMenuDisplayNoneItemsTest() { }
+
+protected:
+    virtual void SetUp() OVERRIDE
+    {
+        m_popupMenuClient.setListSize(kListSize);
+
+        // Set the 4th an 5th items to have "display: none" property
+        m_popupMenuClient.setDisplayNoneIndex(3);
+        m_popupMenuClient.setDisplayNoneIndex(4);
+    }
+    virtual void TearDown() OVERRIDE { }

[tkent, 2014/08/21 00:53:33]

Remove this.  It's unnecessary.

[spartha, 2014/08/21 10:05:44]

Done.

+
+    TestPopupMenuClient m_popupMenuClient;
+};
+
+TEST_F(ExternalPopupMenuDisplayNoneItemsTest, PopupMenuInfoSizeTest)
+{
+    WebPopupMenuInfo info;
+    ExternalPopupMenu::getPopupMenuInfo(&info, &m_popupMenuClient);
+    EXPECT_EQ(5U, info.items.size());
+}
+
+TEST_F(ExternalPopupMenuDisplayNoneItemsTest, IndexMappingTest)
+{
+    // 6th indexed item in popupmenu would be the 4th item in ExternalPopupMenu,
+    // and vice-versa.
+    EXPECT_EQ(4, ExternalPopupMenu::toExternalPopupMenuItemIndex(6, &m_popupMenuClient));
+    EXPECT_EQ(6, ExternalPopupMenu::toPopupMenuItemIndex(4, &m_popupMenuClient));
+
+    // Invalid index, methods should return -1.
+    EXPECT_EQ(-1, ExternalPopupMenu::toExternalPopupMenuItemIndex(8, &m_popupMenuClient));
+    EXPECT_EQ(-1, ExternalPopupMenu::toPopupMenuItemIndex(8, &m_popupMenuClient));
+}
+
+} // namespace blink