Bug 81895 – [Qt] Don't resume the suspended page if the user is continuously flicking.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jocelyn Turcotte]

[Qt] Don't resume the suspended page if the user is continuously flicking.

[Jocelyn Turcotte]

Created attachment 133231 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 133231 [details]
Patch

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

Seems like an ok idea, though it doesnt fit so well with the current code.

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:301
> +void QtViewportInteractionEngine::notifyTouchBegin()

I am not sure how well this naming with together with what we already have in the class

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:304
> +    if (scrollAnimationActive())

This only works with the animation and not the "dragging"

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.h:153
>      OwnPtr<ViewportUpdateDeferrer> m_scaleUpdateDeferrer;
>      OwnPtr<ViewportUpdateDeferrer> m_scrollUpdateDeferrer;
> +    OwnPtr<ViewportUpdateDeferrer> m_panUpdateDeferrer;

These are not actually per gesture recognizer, so scroll vs pan is confusing here.

[Jocelyn Turcotte]

Created attachment 133237 [details]
Patch

[Jocelyn Turcotte]

(In reply to comment #2)
> > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:304
> > +    if (scrollAnimationActive())
> 
> This only works with the animation and not the "dragging"
> 
m_scrollUpdateDeferrer handles the dragging, flickable tells us that it's dragging through movementStarted/Ended.

Fixed the rest, tell me what you think about it.

[Kenneth Rohde Christiansen]

Comment on attachment 133237 [details]
Patch

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

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:304
> +    if (scrollAnimationActive())

I still think this might need to check isMoving instead of isFlicking.

[Kenneth Rohde Christiansen]

(In reply to comment #4)
> (In reply to comment #2)
> > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:304
> > > +    if (scrollAnimationActive())
> > 
> > This only works with the animation and not the "dragging"
> > 
> m_scrollUpdateDeferrer handles the dragging, flickable tells us that it's dragging through movementStarted/Ended.
> 
> Fixed the rest, tell me what you think about it.

cant we merge them then?

[Jocelyn Turcotte]

(In reply to comment #6)
> cant we merge them then?

I think it's pretty simple like this, the suspend count logic makes it all work.
The other way would be to have some heavy boolean logic to decide when to clear the deferrer.

[Jocelyn Turcotte]

Committed r111705: <http://trac.webkit.org/changeset/111705>

[Stephen Chenney]

*** Bug 81691 has been marked as a duplicate of this bug. ***