Closed Bug 1338055 Opened 8 years ago Closed 8 years ago

Crash in java.lang.IllegalArgumentException: Prompt:Show was not registered at org.mozilla.gecko.EventDispatcher.unregisterListener(EventDispatcher.java)

Categories

(Firefox for Android Graveyard :: General, defect)

Unspecified
Android
defect
Not set
critical

Tracking

(firefox52 unaffected, firefox-esr52 unaffected, firefox53 disabled, firefox54 disabled, firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- disabled
firefox54 --- disabled
firefox55 --- fixed

People

(Reporter: jchen, Assigned: droeh)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

This bug was filed from the Socorro interface and is report bp-5456300e-ebdb-45c1-839e-17cb82170208. ============================================================= Stack shows this happens when custom tabs activity is destroyed.
Flags: needinfo?(droeh)
Right, I'm looking into this now.
Flags: needinfo?(droeh)
Attached patch Proposed patch (deleted) — Splinter Review
This just sets mPromptService to null after destroying it so that it will not get destroyed a second time. Should stop the crash from occurring, but it is odd that it's happening to begin with; I don't think we should be able to hit GeckoApp.onDestroy() twice without calling GeckoApp.initialize() in between, which seems to be what is happening here.
Assignee: nobody → droeh
Attachment #8844452 - Flags: review?(nchen)
Attachment #8844452 - Flags: review?(nchen) → review?(s.kaspari)
I think there should be a duplicate somewhere. I remember finding it from crash-stats.
Attachment #8844452 - Flags: review?(s.kaspari) → review+
(In reply to Sebastian Kaspari (:sebastian) from comment #3) > I think there should be a duplicate somewhere. I remember finding it from > crash-stats. The other one was actually about "Doorhanger:Add" (bug 1325021).
Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b3692a0e5a43 Prevent crash from destroying PromptService multiple times. r=sebastian
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Still crashing with today's Nightly (2017-03-11), e.g. https://crash-stats.mozilla.com/report/index/7c59d86f-a658-45e1-b6cb-fe7b02170311. STR: 1. Open Firefox. 2. Open a custom tab. 3. Switch back to the main Firefox activity. 4. Swipe away the custom tab in the task switcher.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thanks for the STR, that helped me figure out what is actually going on here: When we are in Fennec and we swipe to kill a custom tab in the app switcher, we call onDestroy() on the GeckoApp belonging to the custom tab and destroy() on the PromptService belonging to the custom tab, but when we go to unregister the events we use GeckoApp.getEventDispatcher(); this relies on GeckoAppShell.getGeckoInterface() to get the GeckoApp, which unfortunately returns the GeckoApp belonging to Fennec. (This is because we call setGeckoInterface() in GeckoApp.onResume(), and the custom tab does not get resumed before being destroyed.) I'm looking right now to see if there is a nice way around this, but in either case it will be fixed for free when we switch to a GeckoView-based custom tab client.
OK, simple but slightly odd fix: update the 'active' GeckoInterface in GeckoApp.onDestroy(). This way we get the correct GeckoApp from GeckoAppShell.getGeckoInterface() during destruction, and when we switch back to any other GeckoApp-activity we set the GeckoInterface again in onResume() anyways. I also moved super.onDestroy() in BrowserApp.onDestroy() so that a similar issue with mDoorHangerPopup and similar can be avoided. As far as I can tell, I don't think this should break anything, but it's possible there's something subtle I'm overlooking -- setting the active GeckoInterface in onDestroy() definitely feels odd.
Attachment #8847653 - Flags: review?(nchen)
Comment on attachment 8847653 [details] [diff] [review] Update active GeckoInterface in GeckoApp.onDestroy() Review of attachment 8847653 [details] [diff] [review]: ----------------------------------------------------------------- Sounds like you should just keep an EventDispatcher reference inside PromptService. I don't think we can guarantee that `CustomTabsActivity.onDestroy` is always called while BrowserApp is not already the active activity. Otherwise, if `onDestroy` is called while `BrowserApp` is active, `BrowserApp.onResume` won't be called because it's already active. Also, this leaks CustomTabsActivity beyond its lifetime, which is not ideal. If nothing else, you should save the previous GI and restore it afterwards.
Attachment #8847653 - Flags: review?(nchen) → feedback+
Right, we already store Context in PromptService, so I'm just using that to get the instance EventDispatcher in this patch. I think there are probably other problems with the same root cause (getting the wrong GeckoInterface from GeckoAppShell) -- in particular there definitely will be similar cases in bug 1347584. I kind of hate to have to spread Context/EventDispatcher references all over the place, so I'll see if any nicer approach presents itself while I'm working on that bug.
Attachment #8847653 - Attachment is obsolete: true
Attachment #8849131 - Flags: review?(nchen)
Comment on attachment 8849131 [details] [diff] [review] Use stored context to get instance EventDispatcher in PromptService Review of attachment 8849131 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/java/org/mozilla/gecko/prompts/PromptService.java @@ +20,5 @@ > private static final String LOGTAG = "GeckoPromptService"; > > private final Context mContext; > > public PromptService(Context context) { Just make the parameter GeckoApp, and store it as mGeckoApp, instead of mContext.
Attachment #8849131 - Flags: review?(nchen) → review+
Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/63b4a8e63215 Store a reference to the instance EventDispatcher in PromptService. r=jchen
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: