Bug 79521 – [Qt][WK2] Use movementStarted/Ended signals instead of movingChanged on QtViewportInterationEngine

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Hugo Parente Lima]

The movingChanged() signal from Flickable QML component is emitted many times but the QtViewportInteractionEngine is only interested about when the pan starts and when it ends, the Flickable also provide signals for that, so would be better to use those signals (movementStarted()/movementEnded()) instead of movingChanged().

[Hugo Parente Lima]

Created attachment 128787 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 128787 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128787&action=review

Almost there!

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:129
> +    connect(m_flickProvider, SIGNAL(movementStarted()), SLOT(panMoveStarted()), Qt::DirectConnection);
> +    connect(m_flickProvider, SIGNAL(movementEnded()), SLOT(panMoveEnded()), Qt::DirectConnection);

I think we should rename those methods to flickableMoveStarted/Ended

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:-193
> -    if (m_flickProvider->isMoving()) {
> -        if (m_scrollUpdateDeferrer)

Can be please assert for this inside the panMoveStarted/Ended to ensure that the state is always correct?

[Hugo Parente Lima]

Sir, yes sir!

[Hugo Parente Lima]

Created attachment 129056 [details]
Patch

Assertions added, slots renamed.

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 129056 [details]:

css3/filters/effect-contrast-hw.html bug 79618 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

Comment on attachment 129056 [details]
Patch

Clearing flags on attachment: 129056

Committed r109094: <http://trac.webkit.org/changeset/109094>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.