1354973 - Crash in 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)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Max Liu [:maliu]]

This bug was filed from the Socorro interface and is 
report bp-262e6399-5496-4556-86d8-12a952170409.
=============================================================

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:3881)
	at android.view.ViewGroup.addView(ViewGroup.java:3734)
	at android.support.v4.view.ViewPager.addView(Unknown Source)
	at android.view.ViewGroup.addView(ViewGroup.java:3679)
	at android.view.ViewGroup.addView(ViewGroup.java:3655)
	at org.mozilla.gecko.home.activitystream.topsites.TopSitesPagerAdapter.instantiateItem(TopSitesPagerAdapter.java:71)


Looks like we store views in pages list for reuse, not actually create a new instance every time.

Comment 1

[Max Liu [:maliu]]

Attachment: Bug 1354973 - Remove view before add to new ViewGroup,

Review commit: https://reviewboard.mozilla.org/r/128396/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128396/

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8856439 [details]
Bug 1354973 - Remove view before add to new ViewGroup,

https://reviewboard.mozilla.org/r/128396/#review131974

Thanks for fixing this!

::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesPagerAdapter.java:72
(Diff revision 1)
> +        ViewParent viewParent = page.getParent();
> +        if (viewParent != null && viewParent instanceof ViewGroup) {
> +            ViewGroup viewGroup = (ViewGroup) viewParent;
> +            viewGroup.removeView(page);
> +        }

nit: final :)

Comment 3

[Max Liu [:maliu]]

Comment on attachment 8856439 [details]
Bug 1354973 - Remove view before add to new ViewGroup,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128396/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b66674b3b80
Remove view before add to new ViewGroup, r=sebastian

Comment 5

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9b66674b3b80

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Goes back to Fx51 and bug 1293790 IIUC. Please request Aurora approval on this if I've got that right :)

Comment 7

[Max Liu [:maliu]]

Comment on attachment 8856439 [details]
Bug 1354973 - Remove view before add to new ViewGroup,

Approval Request Comment
[Feature/Bug causing the regression]: ActivityStream
[User impact if declined]: Random timing crash when TopSite refreshed
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]:Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: Trivial solution on the same thread and small function block
[String changes made/needed]: No

Comment 8

[Gerry Chang [:gchang]]

Comment on attachment 8856439 [details]
Bug 1354973 - Remove view before add to new ViewGroup,

Fix a crash. Aurora54+.

Comment 9

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/04f47021f97c