Closed Bug 302206 Opened 16 years ago Closed 16 years ago

Need to restart Firefox twice after updating an extension

Categories

(Toolkit :: Add-ons Manager, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: elfguy, Assigned: robert.strong.bugs)

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050726 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050726 Firefox/1.0+

When updating an extension from within the extension manager, Firefox says it
will be installed on next restart. However it still says that after a restart.
It takes 2 restarts to have the extension working.

Reproducible: Always

Steps to Reproduce:
1. Update an extension
2. Restart Firefox
3. See EM
With an upgrade the metadata for the original item is removed and the item is
then set to needs-install. It appears that during an upgrade PendingOperations
no longer gets the needs-install for the item that is being upgraded though it
is written to the extensions.cache. Then when a restart is performed
PendingOperations is populated with the new data and the install finishes. I
should be able to come up with a patch shortly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Severity: major → normal
Attached patch patch (obsolete) — Splinter Review
Benjamin - I've gone over the PendingOperations changes and can't find a reason
not to use the original code. I have also tested new installs, upgrades,
changing the app path, etc. and it all worked. Am I missing something?
Assignee: nobody → rob_strong
Status: NEW → ASSIGNED
Attachment #190680 - Flags: review?(benjamin)
Comment on attachment 190680 [details] [diff] [review]
patch

The codepath that breaks is
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsExtensio
nManager.js.in#4410
which calls
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsExtensio
nManager.js.in#4348

Where "addItem(OP_NONE)" is called. AFAICT this is meant to remove all the
pending operations for that item ID, but perhaps I'm wrong. Anyway, it caused
infinite loops in the _finishOperations while-loop because there was nothing to
deal with OP_NONE in that loop. I think I encountered this when testing
upgrades from 1.0 to 1.1.
Attachment #190680 - Flags: review?(benjamin) → review-
I see where that may have happened previously but the code has changed in
regards to this since bug 264750 landed. I suspect when going from 1.0 to 1.0+
the incompatibility check calling enableItem or disableItem could have caused
the loop you saw. To summarize:

The loop was in _finishOperations is only called during startup.
_appEnableItem and _appDisableItem are now used for the incompatibility checks
during startup. Neither of these ever set the op to OP_NONE.
enableItem and disableItem which can set the op to OP_NONE are only called via
the ui after startup with one exception in _upgradeFromV10. In this instance it
is a newly created datasource so it should always set the op to OP_NEEDS_DISABLE.
Comment on attachment 190680 [details] [diff] [review]
patch

I'm still worried, but ok.
Attachment #190680 - Flags: review-
Attachment #190680 - Flags: review+
Attachment #190680 - Flags: approval1.8b4+
Attached patch more safe gaurding (obsolete) — Splinter Review
I changed the migration code so it just sets userDisabled and appDisabled
directly since this only runs during migration and that is all that needs to
happen at that point. Now the only call site for either of the two functions
that set OP_NONE are in extensions.js.
Attachment #190680 - Attachment is obsolete: true
Attachment #190706 - Flags: review?(benjamin)
Attachment #190706 - Flags: review?(benjamin)
Attachment #190706 - Flags: review+
Attachment #190706 - Flags: approval1.8b4+
Attachment #190706 - Attachment is obsolete: true
Attachment #190706 - Flags: review+
Sorry about that. I forgot to fix a comment.
Attachment #190708 - Flags: review?(benjamin)
Attachment #190708 - Flags: review?(benjamin)
Attachment #190708 - Flags: review+
Attachment #190708 - Flags: approval1.8b4+
Whiteboard: [checkin needed][a+]
Fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed][a+]
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.