Last Comment Bug 765463 - java.lang.IllegalArgumentException: at java.nio.Buffer.limit(Buffer.java) or java.nio.Buffer.limitImpl(Buffer.java) or java.nio.Buffer.position(Buffer.java) or java.nio.Buffer.positionImpl(Buffer.java)
: java.lang.IllegalArgumentException: at java.nio.Buffer.limit(Buffer.java) or ...
Status: RESOLVED FIXED
[native-crash] [QA^]
: crash, regression, reproducible, topcrash
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 16 Branch
: ARM Android
: -- critical (vote)
: Firefox 16
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
Mentors:
: 765712 (view as bug list)
Depends on: 766643
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-16 01:04 PDT by Scoobidiver (away)
Modified: 2016-07-29 14:25 PDT (History)
8 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed


Attachments
patch (955 bytes, patch)
2012-06-24 17:45 PDT, Brad Lassey [:blassey] (use needinfo?)
bugmail: review-
Details | Diff | Splinter Review
Slightly tested patch (2.16 KB, patch)
2012-06-25 08:30 PDT, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details | Diff | Splinter Review
Patch (3.43 KB, patch)
2012-06-25 09:16 PDT, Kartikaya Gupta (email:kats@mozilla.com)
blassey.bugs: review-
Details | Diff | Splinter Review
Patch (v2) (4.91 KB, patch)
2012-06-26 22:44 PDT, Kartikaya Gupta (email:kats@mozilla.com)
blassey.bugs: review+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2012-06-16 01:04:48 PDT
It first appeared in 16.0a1/20120615144113 and there are currently 17 crashes.
The regression window is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=da8c6039c25e&tochange=4e3362864fbd
It's likely a regression from bug 755070.

java.lang.IllegalArgumentException
	at java.nio.Buffer.limit(Buffer.java:251)
	at org.mozilla.gecko.gfx.ScreenshotLayer$ScreenshotImage.copyBuffer(ScreenshotLayer.java:137)
	at org.mozilla.gecko.gfx.ScreenshotLayer$ScreenshotImage.setBitmap(ScreenshotLayer.java:144)
	at org.mozilla.gecko.gfx.ScreenshotLayer.setBitmap(ScreenshotLayer.java:53)
	at org.mozilla.gecko.gfx.LayerRenderer.setCheckerboardBitmap(LayerRenderer.java:138)
	at org.mozilla.gecko.ScreenshotHandler$1.run(GeckoAppShell.java:2338)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:123)
	at org.mozilla.gecko.GeckoBackgroundThread.run(GeckoBackgroundThread.java:31)

More reports at:
https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=ALL%3AALL&range_value=1&range_unit=weeks&query_search=signature&query_type=contains&query=java.nio.Buffer&do_query=1
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-06-17 05:05:57 PDT
I can reproduce this on my Galaxy Nexus in current trunk build when visiting https://www.ziggo.nl/producten/alles-in-1/
I have to have set the uagent (with Phony) to phone Android, though.
Comment 2 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-06-17 05:59:37 PDT
Hmm, and now I can't reproduce anymore.
Comment 3 Cristian Nicolae (:xti) 2012-06-18 06:27:19 PDT
This crash has occurred on the latest Nightly build while I was trying to use the Google Vkb. I will try to find some certain steps for it:

https://crash-stats.mozilla.com/report/index/bp-8a9cb080-f141-479b-8035-940832120618

--
Firefox 16.0a1 (2012-06-18)
Device: Galaxy Nexus
OS: Android 4.0.2
Comment 4 Aaron Train [:aaronmt] 2012-06-18 06:43:32 PDT
STR:

1. www.ups.com, pick your country
2. Tap into the top left package tracker, the virtual-keyboard is invoked

Leave device idle; track dozens of GeckoScreenshots in log-cat. Fennec eventually crashes
Comment 5 Cristian Nicolae (:xti) 2012-06-18 06:53:45 PDT
Steps to reproduce:
1. Open Fennec
2. Go to google.com
3. Tap on Images from the top menu
4. Tap on Google's vkb button from the search input field
5. Insert a couple of chars from it and wait

Expected result:
No crash occurs after step 5.

Actual result:
This crash occurs after step 5. Here are some reports:

https://crash-stats.mozilla.com/report/index/bp-084b2cb6-5bca-4ded-afe2-dfe932120618
https://crash-stats.mozilla.com/report/index/bp-20f8fd34-e2c7-49a3-842b-cf99c2120618
https://crash-stats.mozilla.com/report/index/bp-bde4fb7b-89b8-4b77-9a0a-dbf482120618
Comment 7 Cristian Nicolae (:xti) 2012-06-18 08:46:38 PDT
Simpler STR:

1. Go to google.com (classic version)
2. Put the app in background
3. Go to Android settings and wait

Note:
First, it will take some time until the first crash, but then, those crashes will be more and more frequently until there will be a crash after each 10s or less.
Comment 8 Chris Peterson [:cpeterson] 2012-06-18 22:09:37 PDT
blassey, this Fennec 16 topcrash looks like fallout from the new screenshot code (bug 755070).
Comment 9 Aaron Train [:aaronmt] 2012-06-19 18:40:30 PDT
*** Bug 765712 has been marked as a duplicate of this bug. ***
Comment 10 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-06-20 14:00:34 PDT
http://www.neowin.net also seems to cause this crash after a while.
Comment 11 alex_mayorga 2012-06-20 18:41:33 PDT
Dropping my crash here as I might have STR bp-2303d363-45f2-45ca-8fca-c24842120621
Comment 12 alex_mayorga 2012-06-24 15:37:53 PDT
bp-b43291c5-68d7-4d3d-bb80-fc4402120624

Crashing while login or ordering from www.atumesa.com from an Xperia pro.
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2012-06-24 17:45:48 PDT
Created attachment 636206 [details] [diff] [review]
patch

not sure why I did (rect.bottom - 1) here
Comment 14 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-24 23:18:52 PDT
Comment on attachment 636206 [details] [diff] [review]
patch

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

I don't think this is right. The code in AndroidBridge::TakeScreenshot calls notifyScreenShot with the parameters (dstX, dstY, dstX + dstW, dstY + dstH) which end up becoming the left, top, right, and bottom of the rect. If dstX = 0, dstY = 0, dstW = 10, and dstH = 10, then rect.right + rect.bottom * stride = 10 + 10 * 10 = 110, which exceeds the 100 pixels (dstW * dstH) that were actually painted. That's why I suggested using rect.bottom - 1, but I guess that's not right either for some cases.
Comment 15 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-25 08:30:09 PDT
Created attachment 636319 [details] [diff] [review]
Slightly tested patch

buffer.left and buffer.right need to be multiplied by 2 as well to account for 16bpp. This version should in theory never throw an exception because of the clamping. If things don't screenshot fully then there are errors elsewhere (probably in the slicing code, as mentioned on IRC).
Comment 16 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-25 08:47:11 PDT
Comment on attachment 636319 [details] [diff] [review]
Slightly tested patch

This prevents the crash using the STR AaronMT provided (go to ups.com and pick a country if needed, put focus in the tracking id textbox, and wait).

The crash is prevented by the clamping, since the rect is still (0, 0, 0, 0) and end is still calculated to be 1024.
Comment 17 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-25 08:56:27 PDT
Also I see scheduleCheckerboardScreenshotEvent getting called with these parameters when this happens:
sx = 18, sy = 135, sw = 1, sh = 15, dx = 9, dy = 239, dw = 0, dh = 26
Comment 18 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-25 09:16:04 PDT
Created attachment 636334 [details] [diff] [review]
Patch

This one also kills the useless screenshotting when the dest area has zero width or height.
Comment 19 Brad Lassey [:blassey] (use needinfo?) 2012-06-26 12:48:27 PDT
Comment on attachment 636334 [details] [diff] [review]
Patch

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

::: mobile/android/base/gfx/ScreenshotLayer.java
@@ +129,5 @@
>                  super.finalize();
>              }
>          }
>  
> +        void copyBuffer(ByteBuffer src, ByteBuffer dst, Rect rect, int bufferWidth) {

r- for this change, just pass the stride

@@ +134,5 @@
> +            int start = (rect.left + rect.top * bufferWidth) * 2; // 2 for 16bpp
> +            int end = (rect.right + (rect.bottom - 1) * bufferWidth) * 2; // 2 for 16bpp
> +            // clamp stuff just to be safe
> +            start = Math.max(0, Math.min(dst.limit(), Math.min(src.limit(), start)));
> +            end = Math.max(start, Math.min(dst.limit(), Math.min(src.limit(), end)));

I think you actually want to clamp to src.capacity() here, since you'll be setting the src buffer's limit below and we only need to make sure that that call is valid.
Comment 20 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-26 22:44:34 PDT
Created attachment 637002 [details] [diff] [review]
Patch (v2)
Comment 21 Marco Zehe (:MarcoZ) 2012-06-27 05:51:12 PDT
Since the June 27 build, accessibility's Explore By Touch also triggers this bug. STR:

1. With ICS, TalkBack and Explore By Touch enabled in Accessibility settings, go to http://www.marcozehe.de.
2. Start sliding your finger down on the right-hand side until you reach the "Quick navigation keys now in Firefox for Android nightly builds" heading.
3. Slide down just a little bit more.

Result: Crash: bp-638e5071-5049-4766-9511-425a92120627
Comment 22 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-27 07:54:06 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa4b9a0d764
Comment 23 Ed Morley [:emorley] 2012-06-28 01:10:18 PDT
https://hg.mozilla.org/mozilla-central/rev/7fa4b9a0d764

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