Cleaning up PointerIdManager and add id re-mapping

third_party/WebKit/Source/core/events/PointerEventFactory.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"
+
+namespace blink {
+
+namespace {
+
+inline int toInt(WebPointerProperties::PointerType t) { return static_cast<int>(t); }
+
+const char* pointerTypeNameForWebPointPointerType(WebPointerProperties::PointerType type)
+{
+    switch (type) {
+    case WebPointerProperties::PointerType::Unknown:
+        return "";
+    case WebPointerProperties::PointerType::Touch:
+        return "touch";
+    case WebPointerProperties::PointerType::Pen:
+        return "pen";
+    case WebPointerProperties::PointerType::Mouse:
+        return "mouse";
+    }
+    ASSERT_NOT_REACHED();
+    return "";
+}
+
+} // namespace
+
+const PointerEventFactory::MappedId PointerEventFactory::s_invalidId = 0;
+// Mouse id is 1 to behave the same as MS Edge for compatibility reasons.

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

Please add an empty line above the comment to clarify its "target".

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

Done.

+const PointerEventFactory::MappedId PointerEventFactory::s_mouseId = 1;
+
+PassRefPtrWillBeRawPtr<PointerEvent> PointerEventFactory::create(const AtomicString& type,
+    const PlatformMouseEvent& mouseEvent,
+    PassRefPtrWillBeRawPtr<Node> relatedTarget,
+    PassRefPtrWillBeRawPtr<AbstractView> view)
+{
+    PointerEventInit pointerEventInit;
+
+    pointerEventInit.setPointerId(s_mouseId);
+    pointerEventInit.setPointerType(pointerTypeNameForWebPointPointerType(WebPointerProperties::PointerType::Mouse));

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

Please use mouseEvent.pointerProperties().pointerType() instead of hardcoding
::Mouse here. Blink receives stylus events as PlatformMouseEvents in ChromeOS.

Same for the id above: use s_mouseId only if type==Mouse.

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

I guess we need to further look into the stylus as mouse events and see how we
want to treat them. If they come as MouseEvents I guess they will not have ids.
So we consider that there will be only one stylus at a time just like mouse?
Maybe we can look into that together tomorrow.

+    pointerEventInit.setIsPrimary(true);
+
+    pointerEventInit.setScreenX(mouseEvent.globalPosition().x());
+    pointerEventInit.setScreenY(mouseEvent.globalPosition().y());
+    pointerEventInit.setClientX(mouseEvent.position().x());
+    pointerEventInit.setClientY(mouseEvent.position().y());
+
+    pointerEventInit.setButton(mouseEvent.button());
+    pointerEventInit.setButtons(MouseEvent::platformModifiersToButtons(mouseEvent.modifiers()));
+
+
+

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

Too many empty lines?

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

Done.

+    UIEventWithKeyState::setFromPlatformModifiers(pointerEventInit, mouseEvent.modifiers());
+
+    pointerEventInit.setBubbles(type != EventTypeNames::pointerenter
+        && type != EventTypeNames::pointerleave);
+    pointerEventInit.setCancelable(type != EventTypeNames::pointerenter
+        && type != EventTypeNames::pointerleave && type != EventTypeNames::pointercancel);
+
+    pointerEventInit.setView(view);
+    if (relatedTarget)
+        pointerEventInit.setRelatedTarget(relatedTarget);
+
+    return PointerEvent::create(type, pointerEventInit);
+}
+
+PassRefPtrWillBeRawPtr<PointerEvent> PointerEventFactory::create(const AtomicString& type,
+    const PlatformTouchPoint& touchPoint, PlatformEvent::Modifiers modifiers,
+    const double width, const double height,
+    const double clientX, const double clientY)
+{
+    const PlatformTouchPoint::State pointState = touchPoint.state();
+
+    bool pointerReleasedOrCancelled = pointState == PlatformTouchPoint::TouchReleased
+        || pointState == PlatformTouchPoint::TouchCancelled;
+    const WebPointerProperties::PointerType pointerType = touchPoint.pointerProperties().pointerType;
+    const String& pointerTypeStr = pointerTypeNameForWebPointPointerType(pointerType);
+
+    bool isEnterOrLeave = false;
+
+    MappedId pointerId = add(PointerEventFactory::GeneratedPointer(pointerType, touchPoint.id()));
+    PointerEventInit pointerEventInit;
+    pointerEventInit.setPointerId(pointerId);
+    pointerEventInit.setWidth(width);
+    pointerEventInit.setHeight(height);
+    pointerEventInit.setPressure(touchPoint.force());
+    pointerEventInit.setTiltX(touchPoint.pointerProperties().tiltX);
+    pointerEventInit.setTiltY(touchPoint.pointerProperties().tiltY);
+    pointerEventInit.setPointerType(pointerTypeStr);
+    pointerEventInit.setIsPrimary(isPrimary(pointerId));
+    pointerEventInit.setScreenX(touchPoint.screenPos().x());
+    pointerEventInit.setScreenY(touchPoint.screenPos().y());
+    pointerEventInit.setClientX(clientX);
+    pointerEventInit.setClientY(clientY);
+    pointerEventInit.setButton(0);
+    pointerEventInit.setButtons(pointerReleasedOrCancelled ? 0 : 1);
+
+    UIEventWithKeyState::setFromPlatformModifiers(pointerEventInit, modifiers);
+
+    pointerEventInit.setBubbles(!isEnterOrLeave);
+    pointerEventInit.setCancelable(!isEnterOrLeave && pointState != PlatformTouchPoint::TouchCancelled);
+
+    return PointerEvent::create(type, pointerEventInit);
+}
+
+
+PassRefPtrWillBeRawPtr<PointerEvent> PointerEventFactory::createPointerCancel(const PlatformTouchPoint& touchPoint)
+{
+    const WebPointerProperties::PointerType pointerType = touchPoint.pointerProperties().pointerType;
+    const String& pointerTypeStr = pointerTypeNameForWebPointPointerType(pointerType);
+
+    MappedId pointerId = add(PointerEventFactory::GeneratedPointer(pointerType, touchPoint.id()));
+    PointerEventInit pointerEventInit;
+    pointerEventInit.setPointerId(pointerId);
+    pointerEventInit.setPointerType(pointerTypeStr);
+    pointerEventInit.setIsPrimary(isPrimary(pointerId));
+    pointerEventInit.setBubbles(true);
+    pointerEventInit.setCancelable(false);
+
+    return PointerEvent::create(EventTypeNames::pointercancel, pointerEventInit);
+}
+
+PointerEventFactory::PointerEventFactory()
+: m_currentId(PointerEventFactory::s_mouseId+1)
+{
+    clear();
+}
+
+PointerEventFactory::~PointerEventFactory()
+{
+    clear();
+}
+
+void PointerEventFactory::clear()
+{
+    for (int type = 0; type <= toInt(WebPointerProperties::PointerType::LastEntry); type++) {
+        m_ids[type].clear();
+        m_primaryId[type] = PointerEventFactory::s_invalidId;
+    }
+    m_reverseMapping.clear();
+
+    // Always add mouse pointer in initialization and never remove it.
+    // No need to add it to m_ids as it is not going to be used with the existing APIs
+    m_primaryId[toInt(WebPointerProperties::PointerType::Mouse)] = s_mouseId;
+    m_reverseMapping.add(s_mouseId, GeneratedPointer(WebPointerProperties::PointerType::Mouse, 0));
+}
+
+PointerEventFactory::MappedId PointerEventFactory::add(const GeneratedPointer p)
+{
+    // Do not add extra mouse pointer as it was added in initialization
+    if (p.first == WebPointerProperties::PointerType::Mouse)
+        return s_mouseId;
+
+    int type = toInt(p.first);
+    unsigned id = p.second;
+    if (m_ids[type].contains(id))
+        return m_ids[type].get(id);
+    // We do not handle the overflow of m_currentId as it should be very rare
+    MappedId mappedId = m_currentId++;
+    if (m_ids[type].isEmpty())
+        m_primaryId[type] = mappedId;
+    m_ids[type].add(id, mappedId);
+    m_reverseMapping.add(mappedId, p);
+    return static_cast<PointerEventFactory::MappedId>(mappedId);
+}
+
+void PointerEventFactory::remove(const PassRefPtrWillBeRawPtr<PointerEvent> pointerEvent)
+{
+    MappedId mappedId = pointerEvent->pointerId();
+    // Do not remove mouse pointer id as it should always be there
+    if (mappedId == s_mouseId || !m_reverseMapping.contains(mappedId))
+        return;
+
+    GeneratedPointer p = m_reverseMapping.get(mappedId);
+    int type = toInt(p.first);
+    unsigned id = p.second;
+    m_reverseMapping.remove(mappedId);
+    m_ids[type].remove(id);
+    if (m_primaryId[type] == mappedId)
+        m_primaryId[type] = PointerEventFactory::s_invalidId;
+}
+
+bool PointerEventFactory::isPrimary(PointerEventFactory::MappedId mappedId) const

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

Let's nuke isPrimary(), getPrimaryId() and getType(), it doesn't make sense to
complicate the code only for a predicted use case. As we have discussed offline,
we can bring them back if needed. If we ever do that, I don't believe the mapped
id will need to be exposed; instead the functions will take (type,rawId) as
parameter/return value. I will comment on the bug accordingly.

As a result, you can get rid of reverseMapping. Also, m_ids won't need to be an
array, just a (type,rawId)=>generatedId mapping. The special checks for
type=Mouse will be limited to your add() and remove(), so the code will be much
cleaner.

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

Reverse mapping will be use particurarly for the remove api. I'll be looking
into changing m_ids to one hashmap instead of an array to see what I can do
there.

Functions like isPrimary is used in the create functions of this class to find
out whether the generate id is primary to set the attribute of pointerEvent. But
surely I can get rid of getType and getPrimaryId to make the class simpler.

+{
+    if (!m_reverseMapping.contains(mappedId))
+        return false;
+
+    GeneratedPointer p = m_reverseMapping.get(mappedId);
+    int type = toInt(p.first);
+    return m_primaryId[type] == mappedId;
+}
+
+PointerEventFactory::MappedId PointerEventFactory::getPrimaryId(const WebPointerProperties::PointerType type) const
+{
+    return m_primaryId[toInt(type)];
+}
+
+WebPointerProperties::PointerType PointerEventFactory::getType(const PointerEventFactory::MappedId mappedId) const
+{
+    if (!m_reverseMapping.contains(mappedId))
+        return WebPointerProperties::PointerType::Unknown;
+
+    return m_reverseMapping.get(mappedId).first;
+}
+
+
+} // namespace blink