Cleaning up PointerIdManager and add id re-mapping

third_party/WebKit/Source/core/events/PointerEventFactoryTest.cpp

+// Copyright 2015 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"
+#include "core/events/PointerEventFactory.h"
+
+#include "core/frame/FrameView.h"
+#include "core/page/Page.h"
+#include "public/platform/WebPointerProperties.h"
+#include <gtest/gtest.h>
+
+namespace blink {
+
+class PointerEventFactoryTest : public ::testing::Test {
+protected:
+    void SetUp() override;
+    PassRefPtrWillBeRawPtr<PointerEvent> createAndCheckTouchEvent(
+        WebPointerProperties::PointerType, int id,
+        int uid, bool isPrimary);
+    PassRefPtrWillBeRawPtr<PointerEvent> createAndCheckMouseEvent(int uid, bool isPrimary);
+
+
+    class PlatformTouchPointBuilder : public PlatformTouchPoint {

[mustaq, 2015/11/25 21:43:40]

Please move the inner class def to the end of the parent class so that parent
class members are all together.

[Navid Zolghadr, 2015/11/26 20:15:10]

Sure.

+    public:
+        PlatformTouchPointBuilder(WebPointerProperties::PointerType, int);
+    };
+    PointerEventFactory m_pointerEventFactory;
+    unsigned m_expectedMouseId;

[mustaq, 2015/11/25 21:43:40]

private? const?

[Navid Zolghadr, 2015/11/26 20:15:09]

None of them is possible. As initializing the variables in SetUp function needs
them to be non-const. Also if I make anything private they cannot be accessed
inside TEST_F macros.

+    unsigned m_mappedIdStart;
+};
+
+void PointerEventFactoryTest::SetUp()
+{
+    m_expectedMouseId = 1;
+    m_mappedIdStart = 2;
+
+}
+
+PointerEventFactoryTest::PlatformTouchPointBuilder::PlatformTouchPointBuilder(
+    WebPointerProperties::PointerType pointerType, int id)
+{
+    m_pointerProperties.id = id;
+    m_pointerProperties.pointerType = pointerType;
+}
+
+PassRefPtrWillBeRawPtr<PointerEvent> PointerEventFactoryTest::createAndCheckTouchEvent(
+    WebPointerProperties::PointerType pointerType, int id,
+    int uid, bool isPrimary)

[mustaq, 2015/11/25 21:43:40]

Please clarify the names id vs uid. I would prefer rawId vs
generatedId/uniqueId.

[Navid Zolghadr, 2015/11/25 22:22:59]

Done.

+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent = m_pointerEventFactory.create(
+        EventTypeNames::pointerdown, PointerEventFactoryTest::PlatformTouchPointBuilder(pointerType, id), PlatformEvent::NoModifiers, 0, 0, 0, 0);
+    EXPECT_EQ(pointerEvent->pointerId(), uid);
+    EXPECT_EQ(pointerEvent->isPrimary(), isPrimary);
+    return pointerEvent;
+}
+
+PassRefPtrWillBeRawPtr<PointerEvent> PointerEventFactoryTest::createAndCheckMouseEvent(int uid, bool isPrimary)

[mustaq, 2015/11/25 21:43:40]

- Include pointerType in parameter, and add a test below for PlatformMouseEvents
of type Pen.
- Make the name uid same as suggested for the above function.

[Navid Zolghadr, 2015/11/25 22:22:59]

Done.

+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent = m_pointerEventFactory.create(
+        EventTypeNames::pointerenter, PlatformMouseEvent(), nullptr, nullptr);
+    EXPECT_EQ(pointerEvent->pointerId(), uid);
+    EXPECT_EQ(pointerEvent->isPrimary(), isPrimary);
+    return pointerEvent;
+}
+
+TEST_F(PointerEventFactoryTest, MousePointer)
+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent1 = createAndCheckMouseEvent(m_expectedMouseId, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent2 = createAndCheckMouseEvent(m_expectedMouseId, true);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+
+    createAndCheckMouseEvent(m_expectedMouseId, true);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+    m_pointerEventFactory.remove(pointerEvent2);
+}
+
+TEST_F(PointerEventFactoryTest, TouchPointerPrimaryRemovedWhileAnotherIsThere)
+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent1 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent2 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 1, m_mappedIdStart+1, false);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent3 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 2, m_mappedIdStart+2, false);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent4 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 1, m_mappedIdStart+1, false);
+

[mustaq, 2015/11/25 21:43:40]

Can remove the last empty line in the function. Or at least be consistent.

[Navid Zolghadr, 2015/11/25 22:22:59]

I was copying it from the other test class. I thought there has to be an empty
line at the end of TEST_F functions. That's why I have it at the end of all
those here :).

I guess the other test class was just adding it not on purpose then. So I just
remove it from all the functions here.

+}
+
+TEST_F(PointerEventFactoryTest, TouchPointerReleasedAndPressedAgain)
+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent1 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent2 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 1, m_mappedIdStart+1, false);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+    m_pointerEventFactory.remove(pointerEvent2);
+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent3 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 1, m_mappedIdStart+2, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent4 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart+3, false);
+
+    m_pointerEventFactory.clear();

[mustaq, 2015/11/25 21:43:40]

Great coverage of possible cases, thanks.

+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent5 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 10, m_mappedIdStart+4, true);
+
+}
+
+TEST_F(PointerEventFactoryTest, TouchAndDrag)
+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent1 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent2 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent3 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+    m_pointerEventFactory.remove(pointerEvent2);
+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent4 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart+1, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent5 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart+1, true);
+
+    m_pointerEventFactory.remove(pointerEvent1);
+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent6 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart+1, true);

[mustaq, 2015/11/25 21:43:40]

Shouldn't we expect m_mappedIdStart+2 here?

[Navid Zolghadr, 2015/11/25 22:22:59]

I'll add a comment to explain what happened here. I actually did not remove the
new pointer event in the previous line. Just removed an obsolete (already
removed) pointer event again and just wanted to make sure it doesn't affect the
behavior.

+
+}
+
+TEST_F(PointerEventFactoryTest, MouseAndTouchAndPen)
+{
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent1 = createAndCheckMouseEvent(m_expectedMouseId, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent2 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent3 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Pen, 0, m_mappedIdStart+1, true);
+
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent4 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 1, m_mappedIdStart+2, false);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent5 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 2, m_mappedIdStart+3, false);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent6 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Pen, 0, m_mappedIdStart+1, true);
+    RefPtrWillBeRawPtr<PointerEvent> pointerEvent7 = createAndCheckTouchEvent(WebPointerProperties::PointerType::Pen, 47213, m_mappedIdStart+4, false);
+
+    m_pointerEventFactory.remove(pointerEvent2);
+    m_pointerEventFactory.remove(pointerEvent4);
+    m_pointerEventFactory.remove(pointerEvent5);
+
+    createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 100, m_mappedIdStart+5, true);

[mustaq, 2015/11/25 21:43:40]

Please do a similar test for a new "primary" of type Pen. Remember to create
those PEs through both PlatformMouse & PlatformTouch events.

[Navid Zolghadr, 2015/11/25 22:22:59]

Done.

+
+    m_pointerEventFactory.clear();
+
+    createAndCheckMouseEvent(m_expectedMouseId, true);
+    createAndCheckTouchEvent(WebPointerProperties::PointerType::Touch, 0, m_mappedIdStart+6, true);
+    createAndCheckTouchEvent(WebPointerProperties::PointerType::Pen, 0, m_mappedIdStart+7, true);
+}
+
+} // namespace blink