NativePanZoomController.abortAnimation() needs to be hooked up

RESOLVED FIXED in mozilla25

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: botond, Assigned: botond)

Tracking

(Blocks: 1 bug)

Trunk
mozilla25
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
Blocks: 776030
(Assignee)

Comment 1

4 years ago
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.
Attachment #779354 - Flags: review?(bugmail.mozilla)
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.
Attachment #779354 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 3

4 years ago
Try results: https://tbpl.mozilla.org/?tree=Try&rev=abd7552d522e
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/6745e2006d56
Assignee: nobody → botond
Keywords: checkin-needed

Comment 5

4 years ago
https://hg.mozilla.org/mozilla-central/rev/6745e2006d56
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.