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

RESOLVED FIXED in Firefox 44

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: u421692, Assigned: jchen)

Tracking

({regression})

Trunk
Firefox 44
ARM
Android
regression
Points:
---

Firefox Tracking Flags

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

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8670279 [details]
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

Updated

2 years ago
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?

Comment 1

2 years ago
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.

Comment 2

2 years ago
Jim, did you touch this recently? Could you have a look?
Flags: needinfo?(nchen)
(Assignee)

Comment 3

2 years ago
Yeah I can take this.
Assignee: margaret.leibovic → nchen
Status: NEW → ASSIGNED
Flags: needinfo?(nchen)
(Assignee)

Comment 4

2 years ago
Created attachment 8671059 [details] [diff] [review]
Force restart after installing addon (v1)

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)

Updated

2 years ago
Blocks: 1197974
No longer blocks: 1170113
(Assignee)

Comment 5

2 years ago
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)

Updated

2 years ago
tracking-fennec: ? → 43+

Comment 6

2 years ago
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+

Comment 7

2 years ago
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)
(Assignee)

Comment 8

2 years ago
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.

Comment 9

2 years ago
(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.
(Assignee)

Comment 10

2 years ago
Created attachment 8671949 [details] [diff] [review]
Allow quitting when asked explicitly (v1)

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)
(Assignee)

Updated

2 years ago
Attachment #8671059 - Attachment is obsolete: true
Removing NI
Flags: needinfo?(mark.finkle)
Attachment #8671949 - Flags: review?(snorp) → review+

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/decf4611a88d
https://hg.mozilla.org/mozilla-central/rev/decf4611a88d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: affected → fixed
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)
status-firefox44: fixed → verified
You need to log in before you can comment on or make changes to this bug.