Closed
Bug 1238763
Opened 5 years ago
Closed 5 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)
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.
Comment 1•5 years ago
|
||
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
Assignee | ||
Comment 2•5 years ago
|
||
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...
Assignee | ||
Comment 3•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/30435/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/30435/
Attachment #8706671 -
Flags: review?(michael.l.comella)
Comment 4•5 years ago
|
||
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() {...});
Assignee | ||
Updated•5 years ago
|
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)
Assignee | ||
Comment 5•5 years ago
|
||
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/
Assignee | ||
Comment 6•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/30569/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/30569/
Attachment #8707071 -
Flags: review?(liuche)
Comment 7•5 years ago
|
||
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 8•5 years ago
|
||
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+
Assignee | ||
Comment 9•5 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3e0daa666d7aab1d8fdaf4e9dad1a25fd3356deb Bug 1238763 - Pre: remove unneeded imports r=liuche https://hg.mozilla.org/integration/fx-team/rev/431f531e8bff4d1e5a0b315f4aa7ac42f307ef21 Bug 1238763 - Don't run onPromptFinished on the UI thread. r=liuche
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3e0daa666d7a https://hg.mozilla.org/mozilla-central/rev/431f531e8bff
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Reporter | ||
Comment 11•5 years ago
|
||
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
Assignee | ||
Comment 12•5 years ago
|
||
(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!
Depends on: 1243307
Assignee | ||
Comment 13•5 years ago
|
||
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.)
Updated•27 days ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•