849495 - java.lang.IllegalArgumentException: x must be >= 0 at android.graphics.Bitmap.checkXYSign(Bitmap.java) at org.mozilla.gecko.LightweightTheme.setLightweightTheme(LightweightTheme.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Scoobidiver (away)]

It has been hit by two users several times at startup in 20.0b4. It's likely a regression from bug 816457.

Here is a crash report: bp-7d09acb4-ded0-4c0f-85d4-819f32130309.

java.lang.IllegalArgumentException: x must be >= 0
	at android.graphics.Bitmap.checkXYSign(Bitmap.java:225)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:495)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:471)
	at org.mozilla.gecko.LightweightTheme.setLightweightTheme(LightweightTheme.java:136)
	at org.mozilla.gecko.LightweightTheme.access$000(LightweightTheme.java:43)
	at org.mozilla.gecko.LightweightTheme$1.run(LightweightTheme.java:100)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4507)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:980)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:747)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalArgumentException%3A+x+must+be+%3E%3D+0+at+android.graphics.Bitmap.checkXYSign%28Bitmap.java%29

Comment 1

[Sriram Ramasubramanian [:sriram]]

Moore was right. Screen size and resolution double in size every two years!
Nexus 10 has a very large display with XXHDPI. The personas (with an "s") that are smaller than the screen-size will cause a crash while trying to crop them.
https://www.getpersonas.com/en-US/persona/493078 <-- this would crash in Nexus 10.

While trying to fix it, I am confused. I can solve the crash (with effective checks). But, we cannot right align a bitmap, and show a color on the left. This will affect tiny buttons like Back, Tabs too. (If the image falls short and can't come under Tabs button, we should be showing a color).

What's expected here? We don't install images bigger than the screen-size? We try to squeeze in the image somehow?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #1)

> What's expected here? We don't install images bigger than the screen-size?
> We try to squeeze in the image somehow?

What would a stretched image look like? Why not use the dominant color for the left side?

Comment 3

[Sriram Ramasubramanian [:sriram]]

The size of the image is small. http://cl.ly/image/2t1E34310v3B <-- this gives an example of how much the image occupies in the entire UI.

Stretching is problem.
1. When the image is stretched for about:home, the same transformation should be applied for every small element. That doesn't sound feasible.
2. Even if we do (1), on patterns, there won't be any noticeable difference. But on personas carrying images like https://www.getpersonas.com/en-US/persona/492572 , the image will be too much distorted.

Dominant color to the left.
1. The image should be right aligned. The above screenshot does it. However, when an image is right aligned, it cannot be fed into a Shader of the Paint and drawn. If we cannot pass it through the shader, the image cannot blend into the background of dominant color.
2. In case of tabs button, this get even tricker! a. The image is half way through. b. There should be brown on the left. c. The resultant bitmap should be applied a LinearGradient (performance intensive). d. This LinearGradient (for alpha) Bitmap should be applied over a solid color.

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

I found a better way. If the bitmap size is less than max-device-width, I create a canvas of size max-device-width X bitmap-height, paint it with dominant color and draw the original (smaller image) bitmap by top-right aligning it. There by, the bitmap is alteast as wide as the device that everything works fine.

Comment 5

[Sriram Ramasubramanian [:sriram]]

The result will be like this: http://cl.ly/image/000F3g3G2K1w

Comment 6

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/62a6fba29def

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 724149 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): LWThemes.
User impact if declined: Personas will crash on Nexus 10 (and other big screen devices) when the size of the image is less than the size of the device.
Testing completed (on m-c, etc.): Landed in m-i on 03/13
Risk to taking this patch (and alternatives if risky): Very less. This makes sure the image is wide enough for the device.
String or UUID changes made by this patch: None.

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/62a6fba29def

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 724149 [details] [diff] [review]
Patch

low risk crash/regression fix - let's uplift.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/32f2a5ca2e02
https://hg.mozilla.org/releases/mozilla-beta/rev/784c1f1815a9