Last Comment Bug 695145 - xul & js modified files are not updated when reinstalling
: xul & js modified files are not updated when reinstalling
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P1 normal (vote)
: ---
Assigned To: Geoff Brown [:gbrown] (pto May 28-June 13)
:
Mentors:
Depends on: 686466
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-17 14:05 PDT by [:fabrice] Fabrice Desré
Modified: 2012-01-09 11:26 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
patch (763 bytes, patch)
2011-10-28 12:51 PDT, Brad Lassey [:blassey] (use needinfo?)
mh+mozilla: review+
Details | Diff | Review

Description [:fabrice] Fabrice Desré 2011-10-17 14:05:31 PDT
Updating a build on device with |adb install -r| to keep your data doesn't update xul/js modifications. 

This doesn't happen with a fennec build from inbound or m-c
Comment 1 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-20 13:59:25 PDT
I diff'ed the APK contents (m-c vs birch): Does any of this look wrong/suspicious?

947,971d946
< chrome/toolkit/content/mozapps/extensions/blocklist.js
< chrome/toolkit/content/mozapps/extensions/blocklist.xul
< chrome/toolkit/content/mozapps/extensions/selectAddons.css
< chrome/toolkit/content/mozapps/extensions/extensions-content.js
< chrome/toolkit/content/mozapps/extensions/newaddon.js
< chrome/toolkit/content/mozapps/extensions/list.js
< chrome/toolkit/content/mozapps/extensions/selectAddons.js
< chrome/toolkit/content/mozapps/extensions/extensions.js
< chrome/toolkit/content/mozapps/extensions/list.xul
< chrome/toolkit/content/mozapps/extensions/newaddon.xul
< chrome/toolkit/content/mozapps/extensions/updateinfo.xsl
< chrome/toolkit/content/mozapps/extensions/selectAddons.xul
< chrome/toolkit/content/mozapps/extensions/update.js
< chrome/toolkit/content/mozapps/extensions/extensions.xml
< chrome/toolkit/content/mozapps/extensions/extensions.css
< chrome/toolkit/content/mozapps/extensions/about.js
< chrome/toolkit/content/mozapps/extensions/blocklist.xml
< chrome/toolkit/content/mozapps/extensions/blocklist.css
< chrome/toolkit/content/mozapps/extensions/extensions.xul
< chrome/toolkit/content/mozapps/extensions/selectAddons.xml
< chrome/toolkit/content/mozapps/extensions/about.xul
< chrome/toolkit/content/mozapps/extensions/eula.xul
< chrome/toolkit/content/mozapps/extensions/setting.xml
< chrome/toolkit/content/mozapps/extensions/eula.js
< chrome/toolkit/content/mozapps/extensions/update.xul
1242d1216
< components/amContentHandler.js
1250d1223
< components/amWebInstallListener.js
1253d1225
< components/nsBlocklistService.js
1263d1234
< components/addonManager.js
1273d1243
< modules/AddonUpdateChecker.jsm
1277,1278d1246
< modules/AddonLogging.jsm
< modules/XPIProvider.jsm
1284d1251
< modules/AddonRepository.jsm
1288,1289d1254
< modules/LightweightThemeManager.jsm
< modules/SpellCheckDictionaryBootstrap.js
1331d1295
< modules/PluginProvider.jsm
1340d1303
< modules/AddonManager.jsm
1360a1324
> res/layout/gecko_app.xml
1361a1326
> res/layout/awesomebar_search.xml
1362a1328
> res/layout/gecko_menu.xml
1364a1331
> res/layout/show_tabs.xml
1365a1333,1335
> res/layout/awesomebar_row.xml
> res/layout/bookmark_list_row.xml
> res/layout/bookmarks.xml
1373a1344
> res/drawable/reload.png
1374a1346,1347
> res/drawable/start.png
> res/drawable/address_bar_button_left.9.png
1378a1352,1353
> res/drawable/address_bar_bg.9.png
> res/drawable/address_bar_button_right.9.png
1380a1356
> res/drawable/favicon.png
1381a1358,1359
> res/drawable/address_bar_button_middle.9.png
> res/drawable/quit.png
Comment 2 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-20 14:45:56 PDT
The native UI startupCache is substantially smaller than the XUL version, missing these files:

> jsloader/resource/gre/components/AboutRedirector.js
> jsloader/resource/gre/components/addonManager.js
> jsloader/resource/gre/components/AutoCompleteCache.js
> jsloader/resource/gre/components/LoginManager.js
> jsloader/resource/gre/components/nsFormHistory.js
> jsloader/resource/gre/components/nsPlacesAutoComplete.js
> jsloader/resource/gre/components/nsPlacesExpiration.js
> jsloader/resource/gre/modules/AddonLogging.jsm
> jsloader/resource/gre/modules/AddonManager.jsm
> jsloader/resource/gre/modules/AddonRepository.jsm
> jsloader/resource/gre/modules/DownloadUtils.jsm
> jsloader/resource/gre/modules/Geometry.jsm
> jsloader/resource/gre/modules/LightweightThemeManager.jsm
> jsloader/resource/gre/modules/OpenWebapps.jsm
> jsloader/resource/gre/modules/PlacesUtils.jsm
> jsloader/resource/gre/modules/PluginProvider.jsm
> jsloader/resource/gre/modules/XPIProvider.jsm
> jssubloader/185/resource/gre/chrome/chrome/content/AwesomePanel.js
> jssubloader/185/resource/gre/chrome/chrome/content/CapturePickerUI.js
> jssubloader/185/resource/gre/chrome/chrome/content/common-ui.js
> jssubloader/185/resource/gre/chrome/chrome/content/console.js
> jssubloader/185/resource/gre/chrome/chrome/content/downloads.js
> jssubloader/185/resource/gre/chrome/chrome/content/extensions.js
> jssubloader/185/resource/gre/chrome/chrome/content/OfflineApps.js
> jssubloader/185/resource/gre/chrome/chrome/content/sync.js
> jssubloader/185/resource/gre/chrome/chrome/content/TabsPopup.js
> jssubloader/185/resource/gre/chrome/chrome/content/WebappsUI.js
> xulcache/resource/gre/chrome/chrome/content/browser-scripts.js
> xulcache/resource/gre/chrome/chrome/content/browser-ui.js
> xulcache/resource/gre/chrome/chrome/content/input.js
> xulcache/resource/gre/chrome/chrome/content/Util.js
Comment 3 Michael Wu [:mwu] 2011-10-20 23:28:35 PDT
The startup cache requires something to tell it to invalidate the cache when things are updated. I suspect XPIProvider.jsm was doing that for you before it got removed. Look for startupcache-invalidate or InvalidateCache in XPIProvider.jsm.
Comment 4 Michael Wu [:mwu] 2011-10-20 23:31:53 PDT
Also, unified diff is much more readable than the default.
Comment 5 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-24 18:58:09 PDT
(In reply to Michael Wu [:mwu] from comment #3)
> The startup cache requires something to tell it to invalidate the cache when
> things are updated. I suspect XPIProvider.jsm was doing that for you before
> it got removed. Look for startupcache-invalidate or InvalidateCache in
> XPIProvider.jsm.

From the code, I see that XPIProvider.jsm does send a startupcache-invalidate notification, and StartupCache observes this and reacts by deleting and re-creating the startup cache. However, my testing (against XUL Fennec) indicates that this code is not invoked at or following installation time (adb install -r): The startup cache does not receive startupcache-invalidate, but somehow, changes to js files are picked up. There must be another mechanism at play, but I do not see it.
Comment 6 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-25 17:03:25 PDT
Analysis of this bug is complicated by the use of a timer in the startup cache: If I understand it correctly, the startup cache file is only updated after 60 seconds of inactivity (no writes to the cache for that period).
Comment 7 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-25 18:19:07 PDT
I think the way to resolve this bug is to delete the startup cache when Fennec is updated (it will be re-created on demand). I cannot think of a way to do this in the APK, since the startup cache is created in the profile (ie, non-constant directory path). Alternately, we can delete the startup cache via the existing startupcache-invalidate mechanism, but I don't know how to trigger it only following an update (how can I detect the update at run time?). Hints, suggestions welcomed!
Comment 8 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-26 13:54:20 PDT
From irc: There's another way that the startup cache can be deleted. The startup cache directory is deleted by RemoveComponentRegistries in nsAppRunner, conditionally. This looks like the mechanism I was looking for.
Comment 9 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-26 14:01:07 PDT
I have confirmed that RemoveComponentRegistries is called and deletes the startupCache in XUL-Fennec on first run following installation or update (install -r).
Comment 10 Mike Hommey [:glandium] 2011-10-27 01:26:30 PDT
I found the problem: the version and buildid info is now "hardcoded" in C++, and doing an incremental build without touching toolkit/xre/nsAndroidStartup.cpp won't trigger a version change, while the buildid is supposed to have changed. I believe this will be fixed by the non-hackish fix for bug 686466 (at least, i'll make sure it does)
Comment 11 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-10-27 14:46:41 PDT
Just to recap:

 - old versions of js and xul files are sometimes seen after an update because they are being retrieved from the startupCache, which is persisted in the profile 
 - the startupCache has no checks for outdated content; the startupCache should be deleted following any change that might affect its contents
 - one method of deleting the startupCache is to send a startupcache-invalidate event; XPIProvider.jsm does this when certain add-on changes are detected; when received by the StartupCache instance, the startup cache file is deleted and re-created
 - another method is used in nsAppRunner: when certain changes are detected, nsAppRunner deletes the startupCache directory; this is triggered under several circumstances, but notably when the Version stored in the profile's compatibility.ini (by a previous Fennec execution) does not match the version and build ID info known to the app. This version and build ID info known to the app would normally be loaded from application.ini (updated with timestamps by the build process), but on birch, the version and build ID are compiled from preprocessor definitions in nsAndroidStartup.cpp and therefore do not change until / unless nsAndroidStartup.cpp is re-compiled.

It looks like it will be best to wait for glandium's changes for 686466. In the meantime, work-arounds include:
 - use adb uninstall / adb install instead of adb install -r
 - touch nsAndroidStartup.cpp before each build
 - manually delete the startupCache file
 - manually manipulate compatibility.ini
Comment 12 Brad Lassey [:blassey] (use needinfo?) 2011-10-28 12:51:40 PDT
Created attachment 570332 [details] [diff] [review]
patch
Comment 13 Mike Hommey [:glandium] 2011-10-28 15:20:52 PDT
Comment on attachment 570332 [details] [diff] [review]
patch

This should be enough as a workaround on birch until bug 686466 lands.
Comment 14 Brad Lassey [:blassey] (use needinfo?) 2011-10-28 15:27:26 PDT
pushed https://hg.mozilla.org/projects/birch/rev/a9fb813f816e
Comment 15 Matt Brubeck (:mbrubeck) 2011-12-22 13:27:17 PST
I think I'm still seeing this even after this patch and bug 686466 landed.  When I make a change to a JS file only, and then rebuild the mobile directory, then repackage and reinstall, I (sometimes? always?) see the old version still used...
Comment 16 Geoff Brown [:gbrown] (pto May 28-June 13) 2011-12-22 14:07:18 PST
(In reply to Matt Brubeck (:mbrubeck) from comment #15)

In cases where you see the old version still used, check if the buildid (the 6th parameter, a timestamp that looks like "20111222135307") in <objdir>/build/application.ini.h reflects your most recent build time and also if your <objdir>/toolkit/xre/nsAndroidStartup.o was re-built.

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