611403 - Installing crashme-new.xpi fails to actually install

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(not currently active) Ted Mielczarek]

STR:
1) Visit the URL in the URL field
2) Click the "crashme-new" link
3) Click "Allow"
4) Click "Install Crash Me Now Advanced!"
5) Notification appears to restart, click restart in the Add-Ons pane

Expected:
"Crash Me Now Advanced!" in the Add-Ons list

Actual:
crashme did not get installed.

I don't see any telling errors in the error console, it all looks like it's going smoothly until it fails to install.

This is a self-built Android Fennec on a Nexus One built on mozilla-central changeset 8b43f0a5ae9b, and mobile changeset 7f6cee27564.

Comment 1

[(not currently active) Ted Mielczarek]

I tried today's official nightly, still didn't work.

Comment 2

[Wesley Johnston (:wesj)]

I can not reproduce this with nightlies or with my own builds on my Droid X. Maybe its a problem with the Nexus One. Can you try again with extensions.logging.enabled=true and tell me what appears?

Comment 3

[(not currently active) Ted Mielczarek]

Aha! After restarting:
LOG addons.xpi: Processing install of crashme-new.xpi
Error: ERROR addons.xpi: Failure moving /data/data/org.mozilla.fennec/mozilla/xnxwnx50.default/extensions/staged/[e-mail] to /data/data/org.mozilla.fennec/mozilla/xnxwnx50.default/extensions
XPIProvider.jsm Line: 221

Error: ERROR addons.xpi: Failed to install staged add-on [e-mail] in app-profile: [Exception... "Component returned failure code: ... NS_ERROR_FILE_TARGET_DOES_NOT_EXIST [nsIFile.isDirectory]" location: XPIProvider.jsm :: anonymous :: line 200" ]
XPIProvider.jsm Line: 200

Comment 4

[(not currently active) Ted Mielczarek]

Note that crashme-new is an unpacked extension.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Initial Fix

For some reason the nsIDirectoryEnumerator sometimes gives us an item that doesn't exist. Not sure if the problem is higher up in our code or with Android. This fixes this (at least for crashme), but I'm going to look higher up the chain before I put it up for review.

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Real Initial fix

Whoops. Fix an obvious error I made moving from a messy patch to a clean one.

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 497909 [details] [diff] [review]
Real Initial fix

I'm concerned that we don't properly understand the cause of this bug. My best guess is that on android nsIDirectoryEnumerator is a live view of the filesystem when elsewhere it seems to be static from the time of creation. If that is so then this patch would probably make us miss installing files. Wes was going to verify whether that was the case or not so I'm not going to review this till I've heard back.

Assuming that is the case then we need to enumerate the entries into an array or something before mutating the directory.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Cache directory entries

Based on what I've been able to read, its not smart to delete files while using readdir. So I think we are probably better off caching the list like you suggested.

Comment 9

[Wesley Johnston (:wesj)]

Comment on attachment 500928 [details] [diff] [review]
Cache directory entries

This worked to install crashme, but I'm seeing some other errors now. Removing review request while I look into them...

Comment 10

[Wesley Johnston (:wesj)]

Attachment: Cache v2

Whoops. Just a dumb mistake. Fixed. This also fixes bug 617523 on this Nexus One.

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 500951 [details] [diff] [review]
Cache v2

Use cacheEntries.forEach(function(aEntry) { ... }, this) instead of the for-loop.

Comment 12

[Wesley Johnston (:wesj)]

Attachment: Patch v3

Updated to use Array.forEach.

Comment 13

[Dave Townsend [:mossop]]

Comment on attachment 501029 [details] [diff] [review]
Patch v3

Looks good

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mozilla-central/rev/267854eaa0ff

Comment 15

[ahoza]

Verified as fixed on Mozilla /5.0 (Android;Linux armv7l;rv:2.0b9pre) Gecko/20100107 Firefox/4.0b9pre Fennec /4.0b4pre 
with : Motorola Droid 2 and HTC Desire A8181