695145 - xul & js modified files are not updated when reinstalling

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[[:fabrice] Fabrice Desré]

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]]

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]]

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]]

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]]

Also, unified diff is much more readable than the default.

Comment 5

[Geoff Brown [:gbrown]]

(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]]

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]]

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]]

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]]

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]]

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]]

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?)]

Attachment: patch

Comment 13

[Mike Hommey [:glandium]]

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?)]

pushed https://hg.mozilla.org/projects/birch/rev/a9fb813f816e

Comment 15

[Matt Brubeck (:mbrubeck)]

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]]

(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.