Closed Bug 1211919 Opened 6 years ago Closed 6 years ago

Firefox becomes unresponsive if trying to install an addon that requires restart

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox41 unaffected, firefox42 unaffected, firefox43 affected, firefox44 verified, fennec43+)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox41 --- unaffected
firefox42 --- unaffected
firefox43 --- affected
firefox44 --- verified
fennec 43+ ---

People

(Reporter: u421692, Assigned: jchen)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached file addon_log
1. Open https://addons.mozilla.org
2. Install an addon that requires to restart Firefox (e.g. Stylish, LastPass Password Manager)
3. Tap on restart button when the "restart" doorhanger is displayed 

Expected result:
Firefox restarts and addon is installed

Actual result:
Nothing happens, user can not use Firefox, only after restarting Firefox from Task Manager

Please see attached logs; same thing happens when trying to uninstall the addon
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?
We're running into an error trying to quit the app here:
http://hg.mozilla.org/mozilla-central/annotate/89732fcdb0ba/mobile/android/chrome/content/browser.js#l6050

The logic here hasn't changed in a long time, so I suspect something is going wrong in that `quit` implementation.
Jim, did you touch this recently? Could you have a look?
Flags: needinfo?(nchen)
Yeah I can take this.
Assignee: margaret.leibovic → nchen
Status: NEW → ASSIGNED
Flags: needinfo?(nchen)
Right now we pass the "attempt quit" flag when restarting due to
installing an addon. However, after bug 1197974, merely attempting to
quit is not enough and we have to pass the "force quit" flag.
Attachment #8671059 - Flags: review?(snorp)
Blocks: 1197974
No longer blocks: addon-signing
Comment on attachment 8671059 [details] [diff] [review]
Force restart after installing addon (v1)

Actually snorp is on PTO
Attachment #8671059 - Flags: review?(snorp) → review?(margaret.leibovic)
tracking-fennec: ? → 43+
Comment on attachment 8671059 [details] [diff] [review]
Force restart after installing addon (v1)

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

Sounds reasonable to me.
Attachment #8671059 - Flags: review?(margaret.leibovic) → review+
Does this mean we should also update this BlockListPrompt usage?
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/BlocklistPrompt.js#35

I'm not even sure how to trigger that prompt. This was added in the days of XUL Fennec... mfinkle, do you know if this even works?

Also, it would probably be worth documenting that eAttemptQuit just doesn't work on Android, if that's the case now.
Flags: needinfo?(mark.finkle)
Might be worth fixing eAttemptQuit if lots of code are using it. The easiest workaround would be making eAttemptQuit the same as eForceQuit for Android.
(In reply to Jim Chen [:jchen] [:darchons] from comment #8)
> Might be worth fixing eAttemptQuit if lots of code are using it. The easiest
> workaround would be making eAttemptQuit the same as eForceQuit for Android.

A quick grep makes me realize that it's used in a bunch of toolkit code, at least some of which we probably use. So yeah, fixing the eAttemptQuit implementation for Android would probably make more sense.
This patch fixes the eAttemptQuit flag, and makes the other patch unnecessary.
Asking snorp to review since he's coming back next week.

Right now we don't allow quitting Fennec when the last nsWindow closes
(e.g. when the last GeckoView is destroyed) because we want to keep the
Gecko thread running throughout the app process lifetime. However, when
we are asked to quit explicitly through nsIAppStartup::Quit, we should
release the hold on nsAppStartup and allow quitting to continue.
Attachment #8671949 - Flags: review?(snorp)
Attachment #8671059 - Attachment is obsolete: true
Removing NI
Flags: needinfo?(mark.finkle)
Attachment #8671949 - Flags: review?(snorp) → review+
https://hg.mozilla.org/mozilla-central/rev/decf4611a88d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
After trying to install and add-on that requires to restart Firefox, Firefox restarts and the add-on is installed, so:
Verified as fixed using:
Device: LG G4 (Android 5.1)
Build: Firefox for Android 44.0a1 (2015-10-18)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.