Last Comment Bug 895904 - NativePanZoomController.abortAnimation() needs to be hooked up
: NativePanZoomController.abortAnimation() needs to be hooked up
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: All Android
: -- normal (vote)
: mozilla25
Assigned To: Botond Ballo [:botond]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: apz-fennec
  Show dependency treegraph
 
Reported: 2013-07-19 08:07 PDT by Botond Ballo [:botond]
Modified: 2013-07-24 05:43 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
bug895904.patch (6.27 KB, patch)
2013-07-22 12:53 PDT, Botond Ballo [:botond]
bugmail: review+
Details | Diff | Splinter Review

Description Botond Ballo [:botond] 2013-07-19 08:07:07 PDT
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] 2013-07-22 12:53:18 PDT
Created attachment 779354 [details] [diff] [review]
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:kats@mozilla.com) 2013-07-23 06:53:18 PDT
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] 2013-07-23 07:25:33 PDT
Try results: https://tbpl.mozilla.org/?tree=Try&rev=abd7552d522e
Comment 4 Ryan VanderMeulen [:RyanVM] 2013-07-23 13:43:55 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/6745e2006d56
Comment 5 Ed Morley [:emorley] 2013-07-24 05:43:56 PDT
https://hg.mozilla.org/mozilla-central/rev/6745e2006d56

Note You need to log in before you can comment on or make changes to this bug.