Closed Bug 1238763 Opened 6 years ago Closed 6 years ago

crash in java.lang.IllegalThreadStateException: Expected anything but 1 ("main"), but running there. at org.mozilla.gecko.util.ThreadUtils.assertOnThreadComparison(ThreadUtils.java)

Categories

(Firefox for Android Graveyard :: General, defect)

46 Branch
All
Android
defect
Not set
critical

Tracking

(firefox46 fixed)

VERIFIED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: kats, Assigned: ahunt)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-aa8465e3-0fac-47f6-8d3f-d2a672160111.
=============================================================

STR: add a bookmark, then click on the options in the toast and add the bookmark to home screen.
This is why we add assertions and make them fail before release :)

        at org.mozilla.gecko.util.ThreadUtils.assertNotOnUiThread(ThreadUtils.java:127)
	at org.mozilla.gecko.db.LocalURLMetadata.getForURLs(LocalURLMetadata.java:149)
	at org.mozilla.gecko.GeckoAppShell.createShortcut(GeckoAppShell.java:820)
	at org.mozilla.gecko.BrowserApp$20.onPromptFinished(BrowserApp.java:1119)
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Hardware: ARM → All
So it turns out our prompt callbacks are run on the UI thread - I think it's best to run all callbacks in an AsyncTask (patch will be attached shortly). The alternative would be to use an AsyncTask just for our createShortcut call, but that feels more awkward to me...
https://reviewboard.mozilla.org/r/30435/#review27207

::: mobile/android/base/java/org/mozilla/gecko/prompts/Prompt.java:506
(Diff revision 1)
> +            new AsyncTask<Void, Void, Void>() {

You might just want to use our:
ThreadUtils.postToBackgroundThread(new Runnable() {...});
Attachment #8706671 - Attachment description: MozReview Request: Bug 1238763 - Don't run onPromptFinished on the UI thread. r=mcomella → MozReview Request: Bug 1238763 - Pre: remove unneeded imports r=liuche
Attachment #8706671 - Flags: review?(michael.l.comella) → review?(liuche)
Comment on attachment 8706671 [details]
MozReview Request: Bug 1238763 - Pre: remove unneeded imports r=liuche

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30435/diff/1-2/
Comment on attachment 8706671 [details]
MozReview Request: Bug 1238763 - Pre: remove unneeded imports r=liuche

https://reviewboard.mozilla.org/r/30435/#review27377
Attachment #8706671 - Flags: review?(liuche) → review+
Comment on attachment 8707071 [details]
MozReview Request: Bug 1238763 - Part 1: Don't run onPromptFinished on the UI thread. r=liuche

https://reviewboard.mozilla.org/r/30569/#review27379

lgtm! You probably don't need the Part 1, unless there's more.
Attachment #8707071 - Flags: review?(liuche) → review+
https://hg.mozilla.org/mozilla-central/rev/3e0daa666d7a
https://hg.mozilla.org/mozilla-central/rev/431f531e8bff
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
This doesn't crash now but I also don't get any confirmation after clicking on the "add to home screen" option. It added the icon but didn't provide any feedback inside fennec itself. Don't know if that's intentional or not.
Status: RESOLVED → VERIFIED
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> This doesn't crash now but I also don't get any confirmation after clicking
> on the "add to home screen" option. It added the icon but didn't provide any
> feedback inside fennec itself. Don't know if that's intentional or not.

That's expected, and something we want to fix in Bug 1240560!
I'll be reverting this fix (and providing a different fix) in Bug 1238763 - for consistency with Android conventions, and for simplicity, the callback should probably be on the UI thread, and the callee can then be responsible for pushing code to the background thread as needed.

(This will avoid transferring from UI thread to background thread and back to the UI thread in those cases where the callback needs to do UI actions.)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.