Closed Bug 394717 Opened 13 years ago Closed 13 years ago
_ERROR _ILLEGAL _VALUE calling ns IExtension Manager .update when profile updates
I'm not sure what's causing it, but I only see it immediately after I try to start Firefox with a profile that was based on a slightly older version. When the profile tries to update extensions, it hangs. I don't know exactly how to test this one, but it's definitely happening.
Note that it doesn't really hang; you can still cancel out of it. But once you hit the error, the update is essentially dead. This was with a custom Venkman build installed in the profile - see http://weblogs.mozillazine.org/weirdal/archives/018483.html for details on how to package Venkman.
So are the steps to reproduce: 1. Install a new version of firefox (new build or whole version change?) 2. Wait for background update to happen
JS stack trace: Error()@:0 ([object Array],0,2,[object XPCWrappedNative_NoHelper])@file:///home/ajvincent/trunk/xu-debug/dist/bin/components/nsExtensionManager.js:4973 update([object Array],0,2,[object Object])@:0 ()@chrome://mozapps/content/extensions/update.js:154 anonymous([object Event])@chrome://global/content/bindings/wizard.xml:410 _fireEvent([object XULElement],"pageshow")@chrome://global/content/bindings/wizard.xml:411 set_currentPage([object XULElement])@chrome://global/content/bindings/wizard.xml:94 ()@chrome://mozapps/content/extensions/update.js:54 onload([object Event])@chrome://mozapps/content/extensions/update.xul:1 And what do you know: em.update(, 0, nsIExtensionManager.UPDATE_SYNC_COMPATIBILITY, this); That second argument triggers NS_ERROR_ILLEGAL_VALUE. This makes me wonder - what's the correct thing to do here?
Is there any reason not to remove the check for itemCount equaling 0 and updating the tests and idl accordingly?
Robert: seeing as I'm the one that added it, all I can say is "it seemed like a good idea at the time..."
I agree and being one of the reviewers it seemed fine at the time to me as well. ;) Would you be willing to put together a patch and request review from me?
I think having 0 items passed is probably sensible enough to not throw, however if you're allowing that can we be very explicit in the comments about whether the listener's onUpdateStarted and onUpdateEnded will ever be called
Dave, you raise an interesting point. From my cursory reading of the code, onUpdateStarted gets called, but not onUpdateEnded. However, there was a call to it, as a couple extensions tried to update. Not sure what they were, and that worries me. I have a patch without the docs changes. I'll submit it tomorrow after I've had a bit of time to think about it.
It looks to me that when itemCount is 0 we get all addons into the items array and we always use items and items.length in the call to checkForUpdates. I also observed onUpdateStarted being called at the start, all of the onAddonUpdateStarted / onAddonUpdateEnded pairs, and it finished with onUpdateEnded being called. Seems reasonable to me. Where in the code are you concerned about this?
What if there really are zero extensions? (Note: Firefox probably doesn't qualify, as it has theme extensions.)
Ahhh... now I see the concern. I think adding onUpdateEnded when we have a listener in checkForUpdates after onUpdateStarted when aItemCount is 0 should suffice.
Assignee: nobody → ajvincent
Status: NEW → ASSIGNED
Attachment #279693 - Flags: review?(robert.bugzilla)
Comment on attachment 279693 [details] [diff] [review] patch, v1 Thanks Alex... r=me
Attachment #279693 - Flags: review?(robert.bugzilla) → review+
Checked in to trunk Checking in mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl; /cvsroot/mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl,v <-- nsIExtensionManager.idl new revision: 1.54; previous revision: 1.53 done Checking in mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v <-- nsExtensionManager.js.in new revision: 1.251; previous revision: 1.250 done RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug394717.js,v done Checking in mozilla/toolkit/mozapps/extensions/test/unit/test_bug394717.js; /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug394717.js,v <-- test_bug394717.js initial revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
You need to log in before you can comment on or make changes to this bug.