308172 - Obscure error checking for updates to an extension

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Brendan Eich [:brendan]]

<brendan> running b1, tried to find updates to bugmenot
<brendan> found one
<brendan> clicked install
<brendan> failed
<brendan> Error: installLocation has no properties
<brendan> Source File: file:///usr/local/deerpark/components/nsExtensionManager.js
<brendan> Line: 3203
<brendan> that was the last js console item
<brendan> before it were four occurrences of:
<brendan> No chrome package registered for chrome://bugmenot/skin/bugmenot.png .
<brendan> before that was:
<brendan> Datasource: Update Ended
<brendan> and happier looking stuff before that
<brendan> wanted to check if you knew of such a bug on file yet?
<bsmedberg> no, I don't think so

Comment 1

[Brendan Eich [:brendan]]

Asa, does this sound like a bug you know of?

/be

Comment 2

[Asa Dotzler [:asa]]

I'd seen something like that before I think but thought it was fixed at bug
296566.  Rob, is this something we already have on file? 

Comment 3

[Robert Strong (they/them - no direct email)]

No, I haven't seen this issue or a bug for it though there was a similar issue
previously that has since been fixed.

Brendan - was the extension disabled prior to the update? With this being on
Linux there may be a file permissions issue with the way the extension was packaged.

Comment 4

[Brendan Eich [:brendan]]

I'm sure the extension was disabled.

/be

Comment 5

[Robert Strong (they/them - no direct email)]

There are a couple of issues I have found while researching this and I'm going
to address bug 308638 in this bug a well.

The upgrade code from the EM rewrite updates the metadata before the install
which leads to bug 308638 as well as the "No chrome package registered for" errors.
When upgrading an extension during an app upgrade the chrome.manifest is not
generated though all other required actions are performed.

Comment 6

[Robert Strong (they/them - no direct email)]

I highly suspect the chrome.manifest is not generated due to the zipreadercache
keeping a reference to the jar for the old extension. I took care of this issue
for themes since they are installed and uninstalled without restarting but it
wasn't necessary for extensions previously due to that they always require a
restart. In this instance when upgrading from 1.0.x the ext is installed, a
compatibility check is performed, and an upgrade may occur which will then lead
to the removal of the old extension and another install for the upgraded extension.

Comment 7

[Robert Strong (they/them - no direct email)]

Attachment: patch

With the new way we do compatibility checks I had to reorder the steps so
upgrades occur after disabling in order not to have update manifests overwrite
the needs-upgrade operation type. This lets the actual ext. upgrade occur
during the next restart which is the old behavior. The other option is to do
what I did for themes which is extract the contents.rdf files for generating
the chrome.manifests so that the RDF system doesn't retain its hold on the jar
courtesy of the zipreadercache. This would be too big of a patch for 1.5 if I
were to take that route.

This also no longer updates the important extension metadata until the actual
install occurs which fixes bug 308638 and the "No chrome package registered
for" errors in comment #0 which were caused by the item no longer having
appDisabled="true" and the EM ui trying to display the ext. icon for an
extension that wasn't really enabled.

I also fixed the onerror handler for the nsIXMLHttpRequest... it was throwing
in the case of timeouts.

Comment 8

[Robert Strong (they/them - no direct email)]

Checked in on trunk

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.156; previous revision: 1.155

Comment 9

[Robert Strong (they/them - no direct email)]

Requesting 1.8b5 - this fixes a couple of significant problems
update hang when the update url times out
ext metadata added to ext before an upgrade actually occurs (bug 308638)
ext's without a chrome.manifest don't get the chrome.manifest generated when
they are upgraded during an incompatibility check on app upgrade

Comment 10

[Mike Schroepfer]

Comment on attachment 196341 [details] [diff] [review]
patch

Approved for 1.8b5 per bug meeting

Comment 11

[Robert Strong (they/them - no direct email)]

Checked in on MOZILLA_1_8_Branch

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.144.2.12; previous revision: 1.144.2.11
done