Bug 79521

Summary: [Qt][WK2] Use movementStarted/Ended signals instead of movingChanged on QtViewportInterationEngine
Product: WebKit Reporter: Hugo Parente Lima <hugo.lima>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, menard, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Hugo Parente Lima 2012-02-24 12:43:45 PST
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().
Comment 1 Hugo Parente Lima 2012-02-24 12:49:18 PST
Created attachment 128787 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-02-25 05:13:50 PST
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?
Comment 3 Hugo Parente Lima 2012-02-27 09:03:08 PST
Sir, yes sir!
Comment 4 Hugo Parente Lima 2012-02-27 09:52:33 PST
Created attachment 129056 [details]
Patch

Assertions added, slots renamed.
Comment 5 WebKit Review Bot 2012-02-28 02:40:17 PST
The commit-queue encountered the following flaky tests while processing attachment 129056 [details]:

css3/filters/effect-contrast-hw.html bug 79618 (author: cmarrin@apple.com)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Review Bot 2012-02-28 02:42:20 PST
Comment on attachment 129056 [details]
Patch

Clearing flags on attachment: 129056

Committed r109094: <http://trac.webkit.org/changeset/109094>
Comment 7 WebKit Review Bot 2012-02-28 02:42:24 PST
All reviewed patches have been landed.  Closing bug.