895904 - NativePanZoomController.abortAnimation() needs to be hooked up

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Botond Ballo [:botond]]

When content does a scrollTo(), if a pan/zoom animation is in progress it should be cancelled. On Fennec+APZC, this means hooking up NativePanZoomController.abortAnimation() to call AsyncPanZoomController::CancelAnimation.

Comment 1

[Botond Ballo [:botond]]

Attachment: bug895904.patch

The attached patch hooks up NativePanZoomController.cancelAnimation(). It also fixes a bug in AsyncPanZoomController::CancelAnimation() where it was modiying the mState field which is supposed to be protected by the monitor, but neither it nor its caller was grabbing the monitor.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 779354 [details] [diff] [review]
bug895904.patch

Review of attachment 779354 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/gfx/NativePanZoomController.java
@@ +68,5 @@
>      public void abortPanning() {
>          // no-op in APZC, I think
>      }
>  
> +    public native void abortAnimation();

Move this down so that it's in the same block as the other public native void functions.

::: widget/android/AndroidJNI.cpp
@@ +833,5 @@
>  
>  NS_EXPORT void JNICALL
> +Java_org_mozilla_gecko_gfx_NativePanZoomController_abortAnimation(JNIEnv* env, jobject instance)
> +{
> +  AsyncPanZoomController* controller = nsWindow::GetPanZoomController();

Use a four-space indent throughout this function. That seems to be what the majority of code in this file uses.

Comment 3

[Botond Ballo [:botond]]

Try results: https://tbpl.mozilla.org/?tree=Try&rev=abd7552d522e

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6745e2006d56

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6745e2006d56