[webkit-reviews] review denied: [Bug 107101] [EFL][WK2] Implement pan and flick gesture. : [Attachment 209443] Patch

[bugzilla-daemon at webkit.org]

Gyuyoung Kim <gyuyoung.kim at samsung.com> has denied EunMi Lee
<eunmi15.lee at samsung.com>'s request for review:
Bug 107101: [EFL][WK2] Implement pan and flick gesture.
https://bugs.webkit.org/show_bug.cgi?id=107101

Attachment 209443: Patch
https://bugs.webkit.org/attachment.cgi?id=209443&action=review

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=209443&action=review


It looks this patch need to be done refactoring a little further.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:138
> +    gestureHandler->m_ewkView->scrollBy(gestureHandler->m_lastPoint -
gestureHandler->m_currentPoint);

It would be good if you make view() to return m_ewkView.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:151
> +    m_history.resize(0);

Can't we reset these variables in handlePanFinished() ? I don't think we need
to keep it until panning is started again.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:164
> +	   m_history[m_firstHistoryItemIndex++] = item;

I wonder if we should use m_firstHistoryItemIndex memeber variable. Can't we
simplify this as below ?

if (m_history.size() < m_history.capacity())
    m_history.append(item)
else
    m_history.clear();

Then, find first item.

const HistoryItem& firstHistoryItem = m_history[m_firstHistoryItemIndex];
=> const HistoryItem& firstHistoryItem = m_history.first();