636663 - Android Java Crash java.lang.IllegalArgumentException

Status: RESOLVED DUPLICATE of bug 663447

(Firefox for Android Graveyard :: General, defect)

Description

[Stuart Parmenter]

java.lang.IllegalArgumentException: View not attached to window manager
at android.view.WindowManagerImpl.findViewLocked(WindowManagerImpl.java:391)
at android.view.WindowManagerImpl.removeView(WindowManagerImpl.java:236)
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.os.Handler.handleCallback(Handler.java:587)
at android.os.Handler.dispatchMessage(Handler.java:92)
at android.os.Looper.loop(Looper.java:143)
at android.app.ActivityThread.main(ActivityThread.java:5068)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:521)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:858)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
at dalvik.system.NativeStart.main(Native Method)

Comment 1

[Martijn Wargers (dead)]

Stuart, how did you get this crash in the Android OS? Does it have something to do with Fennec?

Comment 2

[Doug Turner (:dougt)]

stuart, str?  were you rotating the screen when this happened?

Comment 3

[Doug Turner (:dougt)]

So, the window manager owns a view.  When we change the view, it is best to be on the main thread or things become racy.  You can change the view by simply calling dismiss on a view.  This happens in the crash reporting code.

finish() calls .dismiss on the progress dialog:

http://mxr.mozilla.org/mozilla-central/source/embedding/android/CrashReporter.java.in#94

We call finish after posting the crash report on a separate thread here:

http://mxr.mozilla.org/mozilla-central/source/embedding/android/CrashReporter.java.in#301

So, if the orientation changes, you will see this throw.

It is easy enough to wrap the dimiss() with a try/catch.  However, do the reports suggest that there are other ways of crashing?  Looking through gecko, i don't see other android system dialogs that are dismissed.

mwu, blassey?

Comment 4

[Doug Turner (:dougt)]

The other thing I noticed is that at startup I see 4 calls on AndroidBridge that are not on the main thread:

Init()
SetSurfaceView()
SetMainThread()
AttachThread()

Can any of these race on anything view related?  I don't think so, but not sure.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 6

[Doug Turner (:dougt)]

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

in order to prevent multiple doFinish() calls, you should move:

mHandler = new Handler()

into doFinish() like so:

void doFinish() {
  if (mHandler == null) {
    mHandler = new Handler()
    mHandler.post(new Runnable(){
       public void run() {
         finish();
       }});
   }
}

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

No, the handler needs ti be created on thee main thread. That's the whole point of this. Besides there shouldn't be anything dangerous about multiple finish () calls, nor do I think thur code can even do that

Comment 8

[Doug Turner (:dougt)]

could you comment why the handler is created where it is?  if so, i am happy with that.

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

added comment dougt asked for on irc

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/0b6f92a2cc74

Comment 11

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

Stuart, can you provide steps to reproduce please?

Comment 12

[Stuart Parmenter]

I don't have STR -- this was reported from the Android market java crash stats

Comment 13

[Anna (Waverley)]

Can someone please verify this bug ?

Comment 14

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

Not fixed. We still see this crash in beta 5 (see bug 663447). Making a dup of bug 663447 so we can keep tracking it.