Last Comment Bug 655254 - When an add-on moves to a different directory but doesn't change in any other way we should keep updated compatibility information
: When an add-on moves to a different directory but doesn't change in any other...
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: ARM Android
: P1 major with 1 vote (vote)
: mozilla7
Assigned To: Dave Townsend [:mossop]
:
Mentors:
Depends on: 744833
Blocks: 657019
  Show dependency treegraph
 
Reported: 2011-05-06 07:33 PDT by Cristian Nicolae (:xti)
Modified: 2012-05-11 08:49 PDT (History)
11 users (show)
dtownsend: in‑testsuite+
dtownsend: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
fixed
wontfix
7+


Attachments
Disabled addons (51.43 KB, image/png)
2011-05-06 07:33 PDT, Cristian Nicolae (:xti)
no flags Details
extensions.sqlite (384.00 KB, application/x-sqlite3)
2011-05-10 02:07 PDT, Cristian Nicolae (:xti)
no flags Details
extensions.sqlite-journal (224.55 KB, application/octet-stream)
2011-05-10 02:08 PDT, Cristian Nicolae (:xti)
no flags Details
patch rev 1 (16.15 KB, patch)
2011-05-27 13:49 PDT, Dave Townsend [:mossop]
no flags Details | Diff | Review
patch rev 2 (16.18 KB, patch)
2011-06-03 14:24 PDT, Dave Townsend [:mossop]
robert.strong.bugs: review+
Details | Diff | Review

Description Cristian Nicolae (:xti) 2011-05-06 07:33:51 PDT
Created attachment 530617 [details]
Disabled addons

Build id : Mozilla/5.0 (Android;Linux armv7l;rv:6.0a1)Gecko/20110506
Firefox/6.0a1 Fennec/6.0a1
Device: HTC Desire
OS: Android 2.2

Steps to reproduce:
1. Open Fennec App
2. Install at least 1 add-on
3. Quit Fennec
4. Move Fennec App to SD Card
5. After the app is moved reopen it
6. Go to Addons Manager

Expected result:
After step 6, addons installed at step 2 are still active and user is able at least to disable them.

Actual result:
Addons are disabled and buttons "Options" and "Enable" are grayed out. User is able only to uninstall those addons.

Note:
If after step 5, are repeated steps 2-3 and then the app is moved back to internal memory, all the installed addons are disabled.
Please see the attached screenshot.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-06 07:38:23 PDT
Dave - I am working under the assumption the when an add-on is installed, we remember the absolute install path. When we move the profile, the install path is no longer valid.

If this is true, is there a way to "rebase" the add-ons?
Comment 2 Dave Townsend [:mossop] 2011-05-06 08:37:14 PDT
The code should be detecting this. Can you enable extensions.logging.enabled and get a log of the text console of the first startup after the app is moved?
Comment 3 Cristian Nicolae (:xti) 2011-05-09 01:24:19 PDT
(In reply to comment #2)
> The code should be detecting this. Can you enable extensions.logging.enabled
> and get a log of the text console of the first startup after the app is
> moved?

I've installed 3 add-ons (Quit Fennec, Personas and Nightly Tester Tools) and here is the log just after I've moved the app to SD card:

LOG addons.xpi: Writing add-ons list
LOG addons.xpi: Updating add-ons states
LOG addons.xpi: Updating database with changes to installed add-ons
LOG addons.xpi: Add-on nightly@mobile.mozilla.org modified in app-profile
LOG addons.xpi: Add-on mask@desre.org modified in app-profile
Blocklist::_loadBlocklist: no XML File found
LOG addons.xpi: Add-on quitfennec@mbrubeck.limpet.net modified in app-profile
LOG addons.xpi: Opening database
LOG addons.xpi: checkForChanges
LOG addons.xpi: startup
Could not read chrome manifest file '/data/data/org.mozilla.fennec/chrome.manifest'.
Warning: Cannot load binary components from a jar.
Source File: /mnt/asec/org.mozilla.fennec-1/pkg.apk:components/components.manifest Line: 35
Comment 4 Cristian Nicolae (:xti) 2011-05-09 05:51:38 PDT
After I've updated the app with the latest build (20110509), there is one more warning about addons:
Warning: WARN addons.repository: Unknown type id when parsing addon: 3
Source File: resource://gre/modules/AddonRepository.jsm
Comment 5 Dave Townsend [:mossop] 2011-05-09 09:58:40 PDT
Strange, the log suggests that it spotted that the add-ons had moved. Can you attach a copy of extensions.sqlite from the profile before and after the move?
Comment 6 Dave Townsend [:mossop] 2011-05-09 11:39:28 PDT
Actually I can see this happening now. Looks like on the second startup on the sdcard things start working again?
Comment 7 Dave Townsend [:mossop] 2011-05-09 12:36:09 PDT
I think I know what is going on here. All of the add-ons I've found to test are marked compatible up to 4.0.* in their install.rdf but have been bumped to 6.0.* on AMO which is why they install ok. When an add-on has changed directory on startup we basically throw away the updated compatibility info and just use the facts from install.rdf.
Comment 8 Dave Townsend [:mossop] 2011-05-09 13:21:14 PDT
Ok, confirmed this is the case. I guess if the add-on keeps the same version number then we probably should keep the compatibility info in the database
Comment 9 Cristian Nicolae (:xti) 2011-05-10 02:07:53 PDT
Created attachment 531272 [details]
extensions.sqlite

I've attached the extensions.sqlite after I've moved the app to SD Card, but I do not have access to device /data/. The device should be rooted, but I cannot do that.
There is another file on SD Card "extensions.sqlite-journal". I will attach it too. Maybe it would be helpful.
Comment 10 Cristian Nicolae (:xti) 2011-05-10 02:08:55 PDT
Created attachment 531273 [details]
extensions.sqlite-journal
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-13 12:18:10 PDT
Dave - what kind of timeframe ETA do you have for this bug?
Comment 12 Dave Townsend [:mossop] 2011-05-27 13:49:44 PDT
Created attachment 535740 [details] [diff] [review]
patch rev 1

This fix works for the case we are concerned with. The install location has moved and so the extension's directory has changed but nothing else about it has. Fennec's code for moving the profile to the sdcard takes steps to ensure the file modification times remain the same so we rely on that. This includes a test and I've also build this for android and tested it manually to verify it works.
Comment 13 Robert Strong [:rstrong] (use needinfo to contact me) 2011-05-27 15:09:03 PDT
Comment on attachment 535740 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -2168,17 +2168,17 @@ var XPIProvider = {
>    * @param  aMigrateData
>    *         an object generated from a previous version of the database
>    *         holding information about what add-ons were previously userDisabled
>    *         and updated compatibility information if present
>    * @param  aActiveBundles
>    *         When performing recovery after startup this will be an array of
>    *         persistent descriptors of add-ons that are known to be active,
>    *         otherwise it will be null
>-   * @return true if a change requiring a restart was detected
>+   * @return true if a change requiring flushing the caches was detected
>    */
>   processFileChanges: function XPI_processFileChanges(aState, aManifests,
>                                                       aUpdateCompatibility,
>                                                       aOldAppVersion,
>                                                       aOldPlatformVersion,
>                                                       aMigrateData,
>                                                       aActiveBundles) {
>     let visibleAddons = {};
>@@ -2192,17 +2192,17 @@ var XPIProvider = {
>      *
>      * @param  aInstallLocation
>      *         The install location containing the add-on
>      * @param  aOldAddon
>      *         The AddonInternal as it appeared the last time the application
>      *         ran
>      * @param  aAddonState
>      *         The new state of the add-on
>-     * @return true if restarting the application is required to complete
>+     * @return a boolean indicating if flushing caches is required to complete
>      *         changing this add-on
nit: just noticed this. You use "@return true..." above and "@return a boolean..." here. I'd prefer consistency throughout the file.
Comment 14 Cristian Nicolae (:xti) 2011-06-01 09:24:42 PDT
This issue is reproducing on Beta 5:
Build id : Mozilla/5.0 (Android;Linux armv7l;rv:5.0)Gecko/20110527
Firefox/5.0 Fennec/5.0
Device: LG Optimus 2X
OS: Android 2.2
Comment 15 Dave Townsend [:mossop] 2011-06-03 14:24:19 PDT
Created attachment 537233 [details] [diff] [review]
patch rev 2

Corrected the comment
Comment 16 Dave Townsend [:mossop] 2011-06-10 14:18:46 PDT
Landed: http://hg.mozilla.org/mozilla-central/rev/fbeb460473f5
Comment 17 Henrik Skupin (:whimboo) 2011-06-23 06:42:53 PDT
Cristian, can you please verify this fix?
Comment 18 Cristian Nicolae (:xti) 2011-06-23 07:25:28 PDT
VERIFIED FIXED on:

Build id : Mozilla/5.0 (Android;Linux armv7l;rv:7.0a1)Gecko/20110623
Firefox/7.0a1 Fennec/7.0a1

Device: Motorola Droid 2
OS: Android 2.2
Comment 19 Dave Townsend [:mossop] 2011-08-01 20:46:38 PDT
*** Bug 675854 has been marked as a duplicate of this bug. ***
Comment 20 al_9x 2011-08-01 21:41:57 PDT
This does not work for unpacked extensions, which will be nearly all of them for older upgraded profiles (my case).  Copying such a profile changes the folder modified dates and the extensions remain disabled (Fx7).

Note You need to log in before you can comment on or make changes to this bug.