723495 - java.lang.IllegalArgumentException: View not attached to window manager at android.view.WindowManagerImpl.findViewLocked(WindowManagerImpl.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

It's #3 top crasher in 12.0a1 and 11.0a2 (introduced by Socorro 2.4.1).
It first appeared in 12.0a1/20120124040227 and 11.0a2/20120127042007.

The m-c (about 500 ADU) regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=758005504cab&tochange=cfaee7b043f7
The Aurora (about 1000 ADU) regression range might be:
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=c0df4333b5b7&tochange=47cb704586dc
There are no bugs in common!

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalArgumentException%3A%20View%20not%20attached%20to%20window%20manager%20at%20android.view.WindowManagerImpl.findViewLocked%28WindowManagerImpl.java%29

Comment 1

[Robert Kaiser]

(In reply to Scoobidiver from comment #0)
> It first appeared in 12.0a1/20120124040227 and 11.0a2/20120127042007.
> 
> The m-c (about 500 ADU) regression range might be:

Forget about that in those Java cases. We introduced new Java signature generation yesterday, so those probably have been present before but just didn't get this proper Java signature before yesterday.

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

not sure if this is a regression from bug 704225?  cc'ing kats.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I need a full java stack trace in order to know what the problem is here. The signature alone is not enough to diagnose this issue (and a lot of the other java crashes).

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Stack for : https://crash-stats.mozilla.com/report/index/2ffae5a5-c47f-4239-b713-c8b842120206

java.lang.IllegalArgumentException: View not attached to window manager
	at android.view.WindowManagerImpl.findViewLocked(WindowManagerImpl.java:381)
	at android.view.WindowManagerImpl.removeView(WindowManagerImpl.java:226)
	at android.view.Window$LocalWindowManager.removeView(Window.java:432)
	at android.app.Dialog.dismissDialog(Dialog.java:278)
	at android.app.Dialog.access$000(Dialog.java:71)
	at android.app.Dialog$1.run(Dialog.java:111)
	at android.app.Dialog.dismiss(Dialog.java:268)
	at org.mozilla.fennec.CrashReporter.finish(CrashReporter.java:125)
	at org.mozilla.fennec.CrashReporter$1.run(CrashReporter.java:116)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at org.mozilla.gecko.GeckoApp$32.run(GeckoApp.java:1777)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at android.app.ActivityThread.main(ActivityThread.java:3691)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:907)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:665)
	at dalvik.system.NativeStart.main(Native Method)

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Hmm, yes it does appear to be a regression of bug 704225 but looking at the code I have no idea how this could happen. Maybe just catching and logging that exception would be the way to go here.

Comment 6

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

Looks like we just want to wrap a try / catch around the progress.dimiss()

I can even write that patch...

Comment 7

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

Attachment: patch

This patch just wraps a try/catch around the problematic code and logs the exception.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 595253 [details] [diff] [review]
patch

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

I'm a bit leery of just randomly catching exceptions when we don't know what the root cause is. If this is happening because finish() is getting called twice somehow then the super.finish() call will still throw an exception the second time, and this patch will just move the exception down a few lines. Sure, we can move that into the try block too, but its try blocks all the way down. However, given we have more important things to worry about, r+

Comment 9

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

Here are some posts on why this can happen:

http://bend-ing.blogspot.com/2008/11/properly-handle-progress-dialog-in.html
http://stackoverflow.com/questions/2745061/java-lang-illegalargumentexception-view-not-attached-to-window-manager
http://stackoverflow.com/questions/4998616/illegalargumentexception-why

All the posts seem to indicate that try/catching the error is safe enough.

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/2320d34911e2

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Should there be a return statement under this if condition?

https://hg.mozilla.org/integration/mozilla-inbound/file/2320d34911e2/mobile/android/base/CrashReporter.java.in#l244

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Also the first link you pointed to could explain this behaviour, if you rotate the device while the progress dialog is up. The CrashReporter activity entry in the AndroidManifest doesn't handle orientation changes, so it would probably finish the dialog when that happens.

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2320d34911e2

Comment 14

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

(In reply to Kartikaya Gupta (:kats) from comment #11)
> Should there be a return statement under this if condition?
> 
> https://hg.mozilla.org/integration/mozilla-inbound/file/2320d34911e2/mobile/
> android/base/CrashReporter.java.in#l244

I think so. I'll open a new bug

Comment 15

[Scoobidiver (away)]

There are no crashes in 13.0a1/20120209.

Comment 16

[Scoobidiver (away)]

It's #3 top crasher in 12.0a2. Can you push it in Aurora and Beta?

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Scoobidiver from comment #16)
> It's #3 top crasher in 12.0a2. Can you push it in Aurora and Beta?

I think it's a good idea to at least get this on Aurora, since we still have MTD users there and this may reduce user pain.