Last Comment Bug 709817 - The webpage position hangs if a scroll up action is performed several times
: The webpage position hangs if a scroll up action is performed several times
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 normal (vote)
: Firefox 12
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
Mentors:
: 713678 (view as bug list)
Depends on: 712037
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-12 09:02 PST by Cristian Nicolae (:xti)
Modified: 2012-03-08 02:40 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified
11+


Attachments
(1/3) Remove dead code (9.79 KB, patch)
2011-12-20 08:01 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
(2/3) Ensure all PZC access is on UI thread (3.23 KB, patch)
2011-12-20 08:02 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
(3/3) Don't clobber mState on aborting animation (2.55 KB, patch)
2011-12-20 08:05 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
Logs 1 (59.94 KB, text/plain)
2012-01-11 02:11 PST, Camelia Urian
no flags Details
Logs 2 (64.62 KB, text/plain)
2012-01-11 02:11 PST, Camelia Urian
no flags Details

Description Cristian Nicolae (:xti) 2011-12-12 09:02:12 PST
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111212
Firefox/11.0a1 Fennec/11.0a1
Devices: Motorola Droid PRO
OS: Android 2.3.3

Steps to reproduce:
1. Open Fennec App
2. Browse to www.mozilla.com
3. Perform a zoom in action
4. Scroll up the page for several times

Expected result:
At step 4, the webpage moves down while the scroll action is performed, but it will auto-scroll down until the top of the webpage will hit the URL Bar after the finger is released of the screen.

Actual result:
After step 4, the webpage is hanged down as you can see in the following video: http://www.youtube.com/watch?v=iTHRmeXE4NQ&list=UUQbfDJvsv1kOAmp09GPjSag&feature=plcp
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-19 14:47:16 PST
With my logging patches in bug 712037 applied, I reproduced this and got the following output:

D/GeckoPanZoomController( 4717): onTouchEnd in PANNING_LOCKED
D/GeckoLayerController( 4717): scrollBy: v=RectF(0.0, -203.27779, 799.9999, 972.7222) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
[snip more intermediate fling animation events]
D/GeckoLayerController( 4717): scrollBy: v=RectF(0.0, -254.70802, 799.9999, 921.292) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
D/GeckoPanZoomController( 4717): generating valid viewport using v=RectF(0.0, -254.70802, 799.9999, 921.292) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
D/GeckoPanZoomController( 4717): generated valid viewport as v=RectF(0.0, 0.0, 799.9999, 1176.0) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
D/GeckoPanZoomController( 4717): end bounce at v=RectF(0.0, 0.0, 799.9999, 1176.0) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
D/GeckoLayerController( 4717): setViewportMetrics: v=RectF(0.0, -254.70802, 799.9999, 921.292) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
[snip more intermediate bounce animation events]
D/GeckoLayerController( 4717): setViewportMetrics: v=RectF(0.0, -117.02815, 799.9999, 1058.9718) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
I/InputReader(  290): dispatchTouch::touch event's action is 0, pending(waiting finished signal)=1
I/InputDispatcher(  290): Delivering touch to current input target: action: 0, channel '426dcc50 org.mozilla.fennec_kats/org.mozilla.fennec_kats.App (server)'
I/InputDispatcher(  290): Delivering touch to current input target: action: 0, channel 'TouchIntercepter (server)'
I/Gecko   ( 4717): AndroidGeckoEvent: 0x411b7e70 : 19
D/GeckoPanZoomController( 4717): onTouchStart in state FLING
D/GeckoPanZoomController( 4717): Finishing animation at v=RectF(0.0, -117.02815, 799.9999, 1058.9718) p=(1859.0,2934.0) z=1.896568 o=0.0,0.0
I/GeckoSoftwareLayerClient( 4717): Adjusting viewport
I/Gecko   ( 4717): AndroidGeckoEvent: 0x411b8da0 : 19
I/InputReader(  290): dispatchTouch::touch event's action is 1, pending(waiting finished signal)=1
I/InputDispatcher(  290): Delivering touch to current input target: action: 1, channel '426dcc50 org.mozilla.fennec_kats/org.mozilla.fennec_kats.App (server)'
I/InputDispatcher(  290): Delivering touch to current input target: action: 1, channel 'TouchIntercepter (server)'
D/GeckoPanZoomController( 4717): onTouchEnd in NOTHING
E/GeckoPanZoomController( 4717): Received impossible touch end while in NOTHING

This shows the problem is due to unsynchronized access on the mState variable; the onTouchStart method stops the animation timer, but there is still a bounce runnable in progress on another thread. That runnable invokes finishAnimation() some time after the onTouchStart changes the mState to TOUCHING, and the finishAnimation() code changes the mState back to NOTHING. Then when we get the touch-end event, it falls into the "impossible" case and doesn't do the bounce to restore the viewport. So the viewport ends up stuck in overscroll.
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-20 08:01:14 PST
Created attachment 583165 [details] [diff] [review]
(1/3) Remove dead code
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-20 08:02:42 PST
Created attachment 583167 [details] [diff] [review]
(2/3) Ensure all PZC access is on UI thread

This isn't actually the root cause of this bug but I noticed it while debugging and figured it would be good to fix it as well.
Comment 4 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-20 08:05:22 PST
Created attachment 583168 [details] [diff] [review]
(3/3) Don't clobber mState on aborting animation

This patch is the one that actually fixes the issue. I verified by reproducing the circumstances under which the bug happens (animation finishes between onTouchStart and onTouchEnd) as shown by this logging output and verifying that there's still a bounce afterwards that removes any overscroll.

D/GeckoLayerController(29365): setViewportMetrics: v=RectF(-126.15358, -170.1961, 673.84656, 1005.80383) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoLayerController(29365): setViewportMetrics: v=RectF(-109.23613, -147.37247, 690.764, 1028.6276) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoLayerController(29365): setViewportMetrics: v=RectF(-93.21132, -125.75311, 706.7888, 1050.2468) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoPanZoomController(29365): onTouchStart in state FLING
D/GeckoPanZoomController(29365): Finishing animation at v=RectF(-93.21132, -125.75311, 706.7888, 1050.2468) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoPanZoomController(29365): onTouchEnd in TOUCHING
D/GeckoPanZoomController(29365): generating valid viewport using v=RectF(-93.21132, -125.75311, 706.7888, 1050.2468) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoPanZoomController(29365): generated valid viewport as v=RectF(0.0, 0.0, 800.0001, 1176.0) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
D/GeckoPanZoomController(29365): end bounce at v=RectF(0.0, 0.0, 800.0001, 1176.0) p=(1659.0,2704.0) z=1.6930485 o=0.0,0.0
Comment 5 Patrick Walton (:pcwalton) 2011-12-20 12:10:20 PST
Comment on attachment 583165 [details] [diff] [review]
(1/3) Remove dead code

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

r=me
Comment 6 Patrick Walton (:pcwalton) 2011-12-20 12:11:19 PST
Comment on attachment 583167 [details] [diff] [review]
(2/3) Ensure all PZC access is on UI thread

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

Looks good.
Comment 7 Patrick Walton (:pcwalton) 2011-12-20 12:13:42 PST
Comment on attachment 583168 [details] [diff] [review]
(3/3) Don't clobber mState on aborting animation

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

LGTM
Comment 10 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-03 10:02:24 PST
*** Bug 713678 has been marked as a duplicate of this bug. ***
Comment 11 Alex Keybl [:akeybl] 2012-01-03 13:26:15 PST
Comment on attachment 583165 [details] [diff] [review]
(1/3) Remove dead code

[Triage Comment]
Mobile only, approving for Aurora.
Comment 13 Camelia Urian 2012-01-10 08:01:20 PST
Environment:
Aurora 11.0a2 (2012-01-10) Samsung Nexus S (Android 2.3.6)
Nightly 12.0a1 (2012-01-10) Samsung Nexus S (Android 2.3.6)

This issue is still reproducing as originally described on both Nightly and Aurora.
Reopening bug
Comment 14 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-10 12:47:46 PST
Camelia, can you please provide logcat output for this issue? I am unable to reproduce it on the devices I have.
Comment 15 Camelia Urian 2012-01-11 02:11:14 PST
Created attachment 587632 [details]
Logs 1

Environment:
Nightly 12.0a1 (2012-01-10) Samsung Nexus S (Android 2.3.6)

Attached logs.
Comment 16 Camelia Urian 2012-01-11 02:11:54 PST
Created attachment 587633 [details]
Logs 2
Comment 17 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-12 10:23:15 PST
Both of these logs show normal behaviour of the pan/zoom code based on the events we're getting. However, the events we're getting might themselves be messed up. On the first log I see a fair amount of output like this:

D/GeckoPanZoomController(  792): onScaleBegin in PANNING
D/GeckoPanZoomController(  792): onScaleEnd in PINCHING

which implies the ScaleGestureDetector is detecing a scale starting and ending, but no scale actually happening. It happens frequently enough to be suspicious. That log ends with:

D/GeckoPanZoomController(  792): onScaleBegin in PANNING_LOCKED
D/GeckoPanZoomController(  792): onScale in state PINCHING
D/GeckoLayerController(  792): scrollBy: v=RectF(-299.6429, -384.73145, 180.35672, 314.27258) p=(1046.669,4738.9063) z=3.2701726 o=0.0,0.0
D/GeckoLayerController(  792): scaleWithFocus: v=RectF(-307.99185, -397.39465, 172.00778, 301.60938) p=(958.56433,4340.0034) z=2.994902 o=0.0,0.0; zf=2.994902

which seems normal, if you were doing a pinch right at the end. However based on the fact that the app pauses shortly after that, I believe we're getting incorrect events, and that the ScaleGestureDetector is telling us there's a pinch happening when in fact there are no fingers on the screen.

Log #2 shows similar behaviour, but ends with a whole series of pinch/scale events, and again I think we're getting bad data from the ScaleGestureDetector and possibly from the underlying event system. I'll play around with some more scenarios that might trigger this and see if I can get a better handle on what's going on.
Comment 18 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-13 04:08:04 PST
pcwalton is rewriting the ScaleGestureDetector, which may fix this if that's where the problem is. If it's the events we're getting that are messed up, then we'll have to try something else, so I'll leave this bug open.
Comment 19 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-24 08:00:05 PST
The ScaleGestureDetector rewrite went in this morning (https://hg.mozilla.org/mozilla-central/rev/ca4d85ac6bf5). If this issue is still reproducible with that change please file a new bug; the code has changed enough that if it is still happening, the root cause will be different.
Comment 20 Paul Feher 2012-03-08 02:40:46 PST
Verified fixed on:
Nightly Fennec 13.0a1 (2012-03-07)
Device: HTC Desire Z
OS: Android 2.3.3

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