java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child''s parent first. at android.view.ViewGroup.addViewInner(ViewGroup.java) at com.android.internal.policy.impl.PhoneWindow.openPanel on ICS

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
--
critical
RESOLVED FIXED
5 years ago
10 months ago

People

(Reporter: Scoobidiver (away), Assigned: sriram)

Tracking

(4 keywords)

15 Branch
Firefox 17
ARM
Android
crash, regression, reproducible, topcrash
Points:
---

Firefox Tracking Flags

(firefox14 unaffected, firefox15+ fixed, firefox16+ fixed, fennec15+)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
It first appeared in 15.0a1/20120602 and has been hit by 11 ICS users so far. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73783bf75c4c&tochange=5199196b65ec

java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child's parent first.
	at android.view.ViewGroup.addViewInner(ViewGroup.java:3337)
	at android.view.ViewGroup.addView(ViewGroup.java:3208)
	at android.view.ViewGroup.addView(ViewGroup.java:3188)
	at com.android.internal.policy.impl.PhoneWindow.openPanel(PhoneWindow.java:582)
	at com.android.internal.policy.impl.PhoneWindow.onKeyUpPanel(PhoneWindow.java:817)
	at com.android.internal.policy.impl.PhoneWindow.onKeyUp(PhoneWindow.java:1486)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchKeyEvent(PhoneWindow.java:1813)
	at android.view.ViewRootImpl.deliverKeyEventPostIme(ViewRootImpl.java:3327)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2597)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4424)
	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:784)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalStateException%3A+The+specified+child+already+has+a+parent.+You+must+call+removeView%28%29+on+the+child%27%27s+parent+first.+at+android.view.ViewGroup.addViewInner%28ViewGroup.java%29
(Reporter)

Updated

5 years ago
Crash Signature: [@ java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child''s parent first. at android.view.ViewGroup.addViewInner(ViewGroup.java)] → [@ java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child''s parent first. at android.view.ViewGroup.addViewInner(ViewGroup.java) ]
Easy to reproduce via Monkey, with 1000 pseudo-random events sent to recent builds
(Reporter)

Updated

5 years ago
Keywords: reproducible
sriram, does this crash regression look familiar? Could this be another problem with static variables not being cleaned up?
(Assignee)

Comment 3

5 years ago
I am not too sure if we are doing something wrong. If you look into the stack trace of bug 739152 (which is also similar), the crash happens when somewhere a view tries to be added back to a different parent, through Gecko.

at org.mozilla.gecko.GeckoApp$31.run(GeckoApp.java:1387)

However here, this particular stacktrace doesn't have any trace of Gecko. This feels like it happens randomly at Android level, with no Fennec's intervention.
(Assignee)

Comment 4

5 years ago
http://forum.xda-developers.com/showthread.php?t=849676&page=130 -- says it happens (in cyanogen mod) when hardware (!!) keyboard is opened at the wrong time.

http://stackoverflow.com/questions/10454265/various-exceptions-caused-by-the-android-fragments-compatibility-library

And I remember a new library was added for swiping between awesomebar screens. Adding :wesj.
status-firefox14: --- → affected
(Reporter)

Comment 5

5 years ago
Crashes in 14.0 Beta with this crash signature are unrelated to this bug: different stack, not a top crasher (see the regression range in comment 0), not specific to ICS.
status-firefox14: affected → unaffected

Comment 6

5 years ago
FWIW, I just ran into this crash while surfing and trying to go back to the previous page. I was sliding my finger around the bottom between the home and back buttons, trying to aim at the back button, when it happened. Report:
bp-1b2ed479-e000-42a1-845b-bfd362120628
(Reporter)

Comment 7

5 years ago
It's #4 top crasher in 15.0a2 and #9 in 16.0a1.

It might be a regression from bug 739412.

There are a few crashes on Gingerbread with Linux kernel 3.1.10_IcyGlitch.

Here is the breakdown by device on:
* Aurora:
Samsung GT-I9100 	8
HTC One X 	7
HTC One V 	7
Samsung GT-P6210 	3
Samsung GT-P1000 	3
Unknown P300(Funbook) 	3
Samsung Galaxy Nexus 	3
Unknown AIRPAD 	2
Samsung GT-N7000 	1
Malata TeamDRH ICS for GTablet 	1
HTC Desire 	1
* Nightly:
samsung Galaxy Nexus 	4
Samsung GT-N7000 	4
Samsung GT-I9100 	2
telechips A3HD 	2
samsung GT-P1000 	2
samsung GT-I9100 	1
Samsung GT-P1000 	1
Samsung GT-I9000 	1
HTC ADR6425LVW 	1
HTC HD2 	1
(Reporter)

Updated

5 years ago
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
tracking-firefox15: ? → +
tracking-firefox16: ? → +

Comment 8

5 years ago
Hey Sriram/Mark - how can we help QA try to reproduce here? If there are no actionable leads for QA, can we make any speculative fixes early in the 15 beta cycle?
Assignee: nobody → sriram
Keywords: qawanted
(Reporter)

Comment 9

5 years ago
Comment 1 has STR.
tracking-fennec: ? → 15+
Sriram - Please take a look
(Assignee)

Comment 11

5 years ago
https://crash-stats.mozilla.com/report/index/9919af70-d3a7-42f7-98a2-45ec62120720 -- This is a different crash unrelated to this bug -- but shows up in the same crash stats.

https://crash-stats.mozilla.com/report/index/ca1b48a7-8e9f-4e91-b6cd-db2f52120720 -- This is a crash reported in this bug.

If you look into both the stack traces, 
#1 has something in GeckoApp try to add a view that already is attached to another parent. This is our fault -- which we can fix.
#2 has nothing done by GeckoApp. The stacktrace doesn't really have to be from us. This could be by Android trying to add something (like I mentioned, a h/w keyboard or something). So, we don't own this crash. -- which is what the bug reports.

Please file a bug for the other crash.
(Assignee)

Comment 12

5 years ago
There are so many crashes unrelated to this bug. Basically in some parts of the code, we try to add the same view to two different parents. If we solve that, those crashes will be solved.

This crash mentioned here -- it's not our fault.

Also, I would be interested to know, if we see similar crash reports in Google Play market, as they reports crashes for the deployed app. Please update the bug, if the crashes there match the same stack trace mentioned here.
(Reporter)

Comment 13

5 years ago
(In reply to Sriram Ramasubramanian [:sriram] from comment #11)
> #1 has something in GeckoApp try to add a view that already is attached to
> another parent. This is our fault -- which we can fix.
It's already fixed in Fx 15.
(Assignee)

Comment 14

5 years ago
Is it good to close this bug then? As it's not our fault..
(Reporter)

Comment 15

5 years ago
(In reply to Sriram Ramasubramanian [:sriram] from comment #12)
> This crash mentioned here -- it's not our fault.
Why is Fx 14 unaffected?

Comment 16

5 years ago
(In reply to Sriram Ramasubramanian [:sriram] from comment #14)
> Is it good to close this bug then? As it's not our fault..

Well, our users don't care whose fault it is, they only care that Firefox is crashing. Is there some way to circumvent the problem?
I concur with kairo and scoobidiver.  Even if it isn't our fault, people will blame us for the issue as this crash doesn't happen in other browsers.
(Assignee)

Comment 18

5 years ago
I agree that users don't care whose fault it is.
But here is the problem:
1. We cannot identify such a crash -- as it can happen anytime.
   And where exactly could be trap the exceptions?
2. We don't have a proper STR to add exception handlers in that path.

We can't do anything but wait for the user to restart.
(Reporter)

Comment 19

5 years ago
I checked some crashes with this signature in 14.0.1 and none of them has a stack trace containing com.android.internal.policy.impl.PhoneWindow.openPanel, so a change in Fx 15.0 makes it appear.
It looks similar to bug 775271 that also first appeared in 15.0 and occurs on ICS.
(Assignee)

Comment 20

5 years ago
Here is a possibility (which I got while trying MonkeyRunner):
When the device is connected to a h/w keyboard, and menu key is pressed in h/w keyboard, we crash for custom menu -- starting 15.0.
(Reporter)

Comment 21

5 years ago
I think it's a regression from bug 739412 that is in the regression range (see comment 0).
(Assignee)

Comment 22

5 years ago
Created attachment 645470 [details] [diff] [review]
Patch

As I had mentioned, this particular crash happens on honeycomb+ devices having a soft menu button, and the menu is tried to be opened using a h/w menu button -- say from a bluetooth keyboard.

This patch monitors for the MENU key at activity level and opens the custom menu, if the device had a custom menu attached to soft menu. If not, it uses the standard android way.

Note, onKeyDown() for Activity is called only when nothing else in the activity needs it. So, we won't have problems with the content.

The patch is all over the places just to decouple it between Activities and the actual responder -- BrowserToolbar.
Attachment #645470 - Flags: review?(mbrubeck)
Comment on attachment 645470 [details] [diff] [review]
Patch

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

::: mobile/android/base/GeckoApp.java
@@ +716,5 @@
> +    @Override
> +    public boolean onKeyDown(int keyCode, KeyEvent event) {
> +        // Custom Menu should be opened when hardware menu key is pressed.
> +        if (Build.VERSION.SDK_INT >= 11 && keyCode == KeyEvent.KEYCODE_MENU)
> +            return onMenuKeyPressed();

I don't think the SDK_INT check is needed here; there's already a check in BrowserApp.onMenuKeyPressed, which seems like the right place to do it.

Actually, why not just move all this code to BrowserApp?  If WebApp wants to respond to the menu key in some future version, it can just provide its own onKeyDown.  For now, it doesn't seem like putting this in the base class buys us anything.
Attachment #645470 - Flags: review?(mbrubeck) → review-
(Assignee)

Comment 24

5 years ago
(In reply to Matt Brubeck (:mbrubeck) from comment #23)
> Comment on attachment 645470 [details] [diff] [review]
> Patch
> 
> Review of attachment 645470 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/GeckoApp.java
> @@ +716,5 @@
> > +    @Override
> > +    public boolean onKeyDown(int keyCode, KeyEvent event) {
> > +        // Custom Menu should be opened when hardware menu key is pressed.
> > +        if (Build.VERSION.SDK_INT >= 11 && keyCode == KeyEvent.KEYCODE_MENU)
> > +            return onMenuKeyPressed();
> 
> I don't think the SDK_INT check is needed here; there's already a check in
> BrowserApp.onMenuKeyPressed, which seems like the right place to do it.
> 
> Actually, why not just move all this code to BrowserApp?  If WebApp wants to
> respond to the menu key in some future version, it can just provide its own
> onKeyDown.  For now, it doesn't seem like putting this in the base class
> buys us anything.

I can monitor this at BrowserApp level. However, that didn't feel the right approach to me.
1. The custom menu operations should then be moved into BrowserApp.
   If they stay in GeckoApp -- everything related to Custom Menu will be in GeckoApp, this alone will stick out in BrowserApp -- which is wrong.
2. Build check is just for parity with other methods like onCreatePanelView() and onCreatePanelMenu(). It's just to make sure, we are looking at that part of code -- just for 11+.
(In reply to Sriram Ramasubramanian [:sriram] from comment #24)
> 1. The custom menu operations should then be moved into BrowserApp.
>    If they stay in GeckoApp -- everything related to Custom Menu will be in
> GeckoApp, this alone will stick out in BrowserApp -- which is wrong.

But it seems to me you are already adding this logic to BrowserApp, here:

>+    public boolean onMenuKeyPressed() {
>+        if (Build.VERSION.SDK_INT >= 11)
>+            return mBrowserToolbar.onMenuKeyPressed();

Putting the same code directly into BrowserApp.onKeyDown seems to be simpler, and just as clean.  Either way, BrowserApp now contains some of the custom menu logic (which is fine, since it is responsible for interaction with BrowserToolbar).
Comment on attachment 645470 [details] [diff] [review]
Patch

Okay, Sriram convinced me that it is useful to keep all the custom menu logic in GeckoApp, since WebApp may need it soon.  r=mbrubeck with one nit:

>+    @Override
>+    public boolean onMenuKeyPressed() {
>+        if (Build.VERSION.SDK_INT >= 11)
>+            return mBrowserToolbar.onMenuKeyPressed();
>+
>+        return false;
>+    }

We can remove the SDK_INT check from this BrowserApp method, since GeckoApp takes care of that logic for us.
Attachment #645470 - Flags: review- → review+
(Assignee)

Comment 27

5 years ago
Created attachment 645604 [details] [diff] [review]
Patch

This patch cleans up the code.
Ideally when s/w or h/w menu button is pressed, we should be opening the menu by openOptionsMenu(). So, the onClickListener() of our s/w menu, and the h/w menu key from external keyboard should just call openOptionsMenu().

Extra condition checks (that were earlier in BrowserToolbar) is moved to BrowserApp, as BrowserApp is the one who should do the checks. BrowserApp knows BrowserToolbar might have a popup menu, hence calls BrowserToolbar's implementation. If BrowserToolbar doesn't have a popup menu, the default implementation is called.
Attachment #645604 - Flags: review?(mbrubeck)
Attachment #645604 - Flags: review?(mbrubeck) → review+
(Assignee)

Comment 28

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/a2923f4b32dd
(Assignee)

Comment 29

5 years ago
Comment on attachment 645604 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Custom menu.
User impact if declined: Phones/Tablets with s/w menu button won't respond for menu from external keyboard.
Testing completed (on m-c, etc.): Landed in m-i on 07/24 
Risk to taking this patch (and alternatives if risky): None. This just makes sure to open menu, on press of h/w menu key.
String or UUID changes made by this patch: None.
Attachment #645604 - Flags: approval-mozilla-beta?
Attachment #645604 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
Attachment #645470 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a2923f4b32dd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Comment on attachment 645604 [details] [diff] [review]
Patch

No risk patch, mobile-only, fixes native crash - approving.
Attachment #645604 - Flags: approval-mozilla-beta?
Attachment #645604 - Flags: approval-mozilla-beta+
Attachment #645604 - Flags: approval-mozilla-aurora?
Attachment #645604 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 32

5 years ago
Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/97a20755be4f
(Assignee)

Comment 33

5 years ago
Pushed to beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/e956c519281c
(Reporter)

Updated

5 years ago
status-firefox15: affected → fixed
status-firefox16: affected → fixed
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.