582837 - Failure to update appDisabled state for add-ons with targetPlatforms when upgrading the application

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Michael Kraft [:morac]]

I've found that if I upgrade to a new trunk load (or make the browser think it's upgrading) and there is an existing extensions.sqlite file then the following error will show up in the console.

 ERROR addons.xpi Error processing file changes [Exception... Illegal value  nsresult 0x80070057 (NS_ERROR_ILLEGAL_VALUE)  location JS frame  resource//gremodules/XPIProvider.jsm  anonymous  line 777  data no]

I don't know if it hurts anything and it doesn't show up unless there is an "upgrade".

Steps I can use to recreate this is to either run Firefox 3.6.7 and then the trunk load with the same profile or to modify the "extensions.lastAppVersion" preference in the trunk load, both which trigger update checking.

I'm not sure if this occurs if there are no add-ons installed since on my system two add-ons are installed via the registry.

Comment 1

[Dave Townsend [:mossop]]

Exactly what builds were you testing with and what are the add-ons you have installed?

Comment 2

[Michael Kraft [:morac]]

The build version is Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b3pre) Gecko/20100728 Minefield/4.0b3pre, but I've seen it in prior builds as well.  I've seen this on two different machines (both running Windows XP).  The add-ons in common between the two machines are:

1. AI Roboform Toolbar for Firefox 6.9.98   (Windows current user registry)
1. Java Quick Starter 1.0  (Windows local machine registry)
2. Microsoft .Net Framework Assistant 0.0.0  (Windows local machine registry)
3. Default Theme

All of these are registry added add-ons and shouldn't be checking for updates, though they would be checked for compatibility updates.

I put some debugging statements in the copyRowProperties function in XPIProvider.jsm and aProperties is ["os", "abi"] and aTarget is {}.  This means it's failing on the call from the _getTargetPlatforms function.  At the time of the exception, "aProp" is "os" which means the error is occurring on row.getResultByName("os").  I tried to dump out aRow.getResultByIndex(0), but that put out the same error so there are no returned rows.

In _getTargetPlatforms, I dumped out the aAddon._internal_id value so I could see what add-on was triggering the error.  I got a result of "7" which matches up with Roboform in the extensions.sqlite database.  Roboform (addon_internal_id = 7) has an entry in the targetPlatform table so I don't know why the row information would be empty. 

I then put a try catch around line 777 in XPIProvider.jsm and the result was an "illegal value" for every add-on that is in the targetPlatform table.  I even tried dumping out aRow.numEntries and that threw an exception.  It's almost as if the StatementRow aRow isn't actually an instance of the mozIStorageRow interface at that point.

Comment 3

[Dave Townsend [:mossop]]

Someone else reported something similar relating to add-ons with targetPlatform entries previously so there is clearly something wrong there, will investigate today.

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

When reading the target platform data synchronously during startup we have to use copyProperties rather than copyRowProperties which is meant for access to the results of asynchronous statements. This failure stops us updating appDisabled for add-ons during an upgrade so including it in the existing upgrade test is pretty straightforward.

Comment 5

[Dave Townsend [:mossop]]

Fixed: http://hg.mozilla.org/mozilla-central/rev/0a2999d5ad5e

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Michael, could you please re-test with the latest trunk build and check if it is fixed for you? Thanks.

Comment 7

[Michael Kraft [:morac]]

I'm not seeing the errors in the 20100802 nightly so it looks to be fixed.