Closed Bug 700559 Opened 8 years ago Closed 8 years ago

java.lang.RuntimeException: Not overscrolled at startSnap()

Categories

(Firefox for Android :: General, defect, P1, critical)

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox11 --- verified
firefox12 --- verified
firefox13 --- verified
fennec 11+ ---

People

(Reporter: blassey, Assigned: kats)

References

Details

(Keywords: crash, Whiteboard: [native-crash])

Attachments

(2 files)

with a build from changeset 48e52668c10a I get this crash when panning around:

I/Gecko   ( 1564): Got message: PanZoom:Resize
E/Fennec  ( 1564): ### Setting page size to (0,0)
E/Fennec  ( 1564): page size: (0,0) visible rect: (-195,12,480,681)screen size: (480,681)
E/GeckoApp( 1564): top level exception
E/GeckoApp( 1564): java.lang.RuntimeException: Not overscrolled at startSnap()
E/GeckoApp( 1564): 	at org.mozilla.fennec.ui.PanZoomController$Axis.startSnap(PanZoomController.java:469)
E/GeckoApp( 1564): 	at org.mozilla.fennec.ui.PanZoomController$FlingRunnable.run(PanZoomController.java:323)
E/GeckoApp( 1564): 	at android.os.Handler.handleCallback(Handler.java:587)
E/GeckoApp( 1564): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoApp( 1564): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 1564): 	at org.mozilla.gecko.GeckoApp$18.run(GeckoApp.java:1143)
E/GeckoApp( 1564): 	at android.os.Handler.handleCallback(Handler.java:587)
E/GeckoApp( 1564): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoApp( 1564): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 1564): 	at android.app.ActivityThread.main(ActivityThread.java:3835)
E/GeckoApp( 1564): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoApp( 1564): 	at java.lang.reflect.Method.invoke(Method.java:507)
E/GeckoApp( 1564): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:847)
E/GeckoApp( 1564): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:605)
E/GeckoApp( 1564): 	at dalvik.system.NativeStart.main(Native Method)
Keywords: crash
Whiteboard: [native-crash]
Assignee: nobody → pwalton
Priority: -- → P1
Severity: normal → critical
Code line change; just ran into this.

java.lang.RuntimeException: Not overscrolled at startSnap()
E/GeckoApp( 5537): 	at org.mozilla.gecko.ui.PanZoomController$Axis.startSnap(PanZoomController.java:466)
E/GeckoApp( 5537): 	at org.mozilla.gecko.ui.PanZoomController$FlingRunnable.run(PanZoomController.java:311)
Assignee: pwalton → kgupta
After much debugging, I figured out why this happens. My analysis below is based on the version of the code at https://hg.mozilla.org/projects/birch/file/d7fa4814218d/embedding/android/ui/PanZoomController.java

It's a race condition that occurs when the FlingRunnable.stop() method runs concurrently with a new call to fling(). When this happens, the mFlingState gets set back NOTHING (in stop()) between it getting set to FLING in onTouchEnd() and the check in fling(). The velocity is therefore not reset to zero, the displace() calls move the viewport into an overscroll state, and then calling startFling() puts it into a WAITING_TO_SNAP state. Note, however, that updatePosition() is never called here, which means that when the FlingRunnable.run() starts, and it repopulates the viewport position, the overscroll disappears and the excess is set back to zero. Now that it's in a WAITING_TO_SNAP state with no overscroll, the exception in startSnap gets triggered from FlingRunnable.run().
Amazing detective work.

Can we just always run the pan/zoom code on one thread?
I'd like to, yes. I think the cleanest approach would be to somehow schedule that timer on the main UI thread so it never interferes with the other event handling. I'll put together a patch for that.
Actually, I just realized that everything *is* happening on one thread, I totally missed the fact that the timer just turns around and posts a message on the UI thread, and that's where the FlingRunnable actually runs.

However, most of my analysis is still correct - it just happens when two fling events overlap and the second fling is entered while there's still a velocity (but no overscroll) left over from the first fling.

This makes the patch much simpler, but also means there's more dead code in this file than I first thought (will remove that separately).
Attachment #575217 - Flags: review?(pwalton)
Comment on attachment 575217 [details] [diff] [review]
Fix for the crash

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

r=me, thanks for digging into this!
Attachment #575217 - Flags: review?(pwalton) → review+
http://hg.mozilla.org/projects/birch/rev/a87cd9791a02
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
With today's builds I'm running into this issue still.

Samsung Nexus S (Android 2.3.6)
20111118104003
http://hg.mozilla.org/projects/birch/rev/b209ff13993a


E/GeckoApp( 3597): java.lang.RuntimeException: Not overscrolled at startSnap()
E/GeckoApp( 3597): 	at org.mozilla.gecko.ui.PanZoomController$Axis.startSnap(PanZoomController.java:473)
E/GeckoApp( 3597): 	at org.mozilla.gecko.ui.PanZoomController$FlingRunnable.run(PanZoomController.java:311)
E/GeckoApp( 3597): 	at android.os.Handler.handleCallback(Handler.java:587)
E/GeckoApp( 3597): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoApp( 3597): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 3597): 	at org.mozilla.gecko.GeckoApp$23.run(GeckoApp.java:1208)
E/GeckoApp( 3597): 	at android.os.Handler.handleCallback(Handler.java:587)
E/GeckoApp( 3597): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoApp( 3597): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 3597): 	at android.app.ActivityThread.main(ActivityThread.java:3683)
E/GeckoApp( 3597): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoApp( 3597): 	at java.lang.reflect.Method.invoke(Method.java:507)
E/GeckoApp( 3597): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839)
E/GeckoApp( 3597): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597)
E/GeckoApp( 3597): 	at dalvik.system.NativeStart.main(Native Method)

I can reliably reproduce this using http://www.madisonavenuepub.com/ at their splash screen, doing rotate and pinch-zooms on the content.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
While I couldn't reproduce the error, I realize thered are other (legitimate) scenarios in which this could happen, since other code could run between the call to fling() and the FlingRunnable actually executing. Instead of trying to eliminate all of those conditions and/or deal with them individually, I think it's better to just remove the exception since we can deal with the situation gracefully and not lose anything.
Attachment #575550 - Flags: review?(pwalton)
Comment on attachment 575550 [details] [diff] [review]
Remove the exception

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

This is fine.
Attachment #575550 - Flags: review?(pwalton) → review+
https://hg.mozilla.org/projects/birch/rev/74e9842f2d0f
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
tracking-fennec: --- → 11+
Verified fixed on:

Firefox 13.0a1 (2012-02-20)
20120220031231
http://hg.mozilla.org/mozilla-central/rev/561771f01881

Firefox 12.0a2 (2012-02-20)
20120220042008
http://hg.mozilla.org/releases/mozilla-aurora/rev/dfb7c3567c49

Firefox 11.0 (2012-02-19)
20120219151859
http://hg.mozilla.org/releases/mozilla-beta/rev/6df75f4d5ccc

--
Device: Motorola Droid PRO
OS: Android 2.3.3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.